Conversation
…own event handler ICanOpenNode now also implements IAsyncDisposable. DisposeAsync waits for the reader and the event pump without holding a thread. Dispose and DisposeAsync no longer wait for the event pump when they are called from a subscriber, which runs on it: Dispose used to stall for its two-second join timeout waiting for itself (#251, the CANopen part). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Lifecycle API: Event semantics after dispose: The event pump honors Also fixes #270: Reviewed by Cursor Bugbot for commit 70db3e8. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7059d99cdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7059d99. Configure here.
…as well, not only from the event pump DisposeAsync called from inside one of the node's own callbacks now takes the blocking path, which knows which task it must not wait for (the reader on its failure report, the event pump on an event); the actor's reentrant Dispose stays on the thread it recognises. The join no longer swallows exceptions it cannot see: the two tasks do not fault. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4991838ffc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oncurrent DisposeAsync wait for the disposal Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9792880fc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…; cover the cleanup with open sessions Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47145c42e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… ICanOpenNode ICanOpenNode stays IDisposable: adding IAsyncDisposable to a published interface breaks every third-party implementer and would need a major release. The nodes this library creates implement IAsyncDisposable, and CanOpenNodeExtensions.DisposeAsync(ICanOpenNode) reaches it (a node that is not this library's is disposed on the thread pool). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c636a17ceb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e transfers; bound the wait of a losing DisposeAsync Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22dca28d04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…it for a running disposal for as long as it can take Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db290f5b90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…criber disposes the node Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5292c1adcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e node, stop the pump when its join times out, wait for a running disposal without a bound Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9c4eae1fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…groundExceptionOccurred rounds end at a disposing subscriber Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd88c1251a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r the background-exception round Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c20bef4356
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: decf158fea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ery guarantee as 'starts no further delivery' Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65830c9b4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ocument the actor-timeout limit of the transfer cleanup Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…s the receive faults The reader posts the loss to the inbox before it raises the report, so the receive can complete first. With the report delayed by 200 ms the old assertion fails every time and this one does not (#270). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This branch has not been deployed
2 participants
What does this change?
The CANopen part of #251 (
Disposeblocks on work that needs the object's own actor):Disposeused to wait up to its two-second join timeout for the event pump, the reader or the actor to finish, which is the thread a subscriber disposing the node runs on. A subscriber of the node (an event,ApplicationReset,BackgroundExceptionOccurred) may now dispose it: the task that subscriber runs on is not waited for. From the actor the cleanup runs in place, so the open transfers have ended when the call returns.DisposeAsync, additive and without touchingICanOpenNode: the nodes this library creates implementIAsyncDisposable, andCanOpenNodeExtensions.DisposeAsync(this ICanOpenNode)reaches it without a cast (a node that is not this library's is disposed on the thread pool). It waits for the reader and event pump without holding a thread. A second call while one is running returns when the disposal has finished, also when it threw.await usingneeds a cast toIAsyncDisposable, since the interface does not declare it.DisposeAsynctakes the blocking path, which knows which task it must not wait for.Why not
ICanOpenNode : IAsyncDisposable: that is source-breaking for anyone who implements the interface and would be a major release (Codex made the point on this PR, and the maintainer chose to leave the interface alone). If a 2.0 is planned, the interface member can replace the extension then.UDS and J1939 have the same pattern (
UdsClientImpl.Dispose,PeriodicSchedule.Dispose); they get their own pull requests, one per package. #251 stays open until they have landed.Also in this PR:
IsoTpChannelIntegrationTests.A_Failing_Subscription_Ends_The_Inbox_With_Its_Failure(#261) read the failure report as soon as the receive faulted, although the reader raises it after posting the loss. It failed on most runs of this branch, whose new tests occupy thread-pool threads; the test now awaits the report (#270).Closes #270.
Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no release!in the title, plus aBREAKING CHANGE:footer explaining the migration)Checklist
dotnet build CanKit.Pro.sln -c Releasesucceeds (with-p:CI=true)dotnet test CanKit.Pro.sln -c Releasepasses (net10.0; net48 only runs in CI)receivedfile)Mutation checks: each context check removed (pump, reader, actor, in
Disposeand inDisposeAsync), the in-place cleanup on the actor replaced by a post, a concurrent call returning at once, and the completion task not set when a disposal throws each fail the matching test. The cleanup coverage test (open sessions and consumers) is a coverage test and proves no mutation.🤖 Generated with Claude Code