Repository navigation
fix(isotp): dispose the owned actor before unsubscribing in the constructor catch - #448
Conversation
…ructor catch The IsoTpChannel constructor catch detached the BackgroundExceptionOccurred handler before disposing an owned actor. Dispose the actor first and unsubscribe afterwards, the order the J1939/J1939-TP constructors use since #446. Closes #447 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AJKME1N1ZvnY3yYcPwNoF
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit e7bbd6d. 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
What does this change?
The
IsoTpChannelconstructor catch detached theBackgroundExceptionOccurredhandler before disposing an owned actor. It now disposes the owned actor first and unsubscribes afterwards, the order the J1939/J1939-TP constructors use since #446, so a shutdown-timeout report from a failing constructor is not dropped.No regression test, deliberately, and the reasoning of #446 holds here: a throwing constructor never returns the channel, so nothing can subscribe to its
BackgroundExceptionOccurred, andOnActorBackgroundExceptionforwards to that empty subscriber list (RaiseBackgroundException).IProtocolActorcan be injected, but an injected actor is not disposed here (_ownsActor), and the ownedProtocolActoris sealed with a plain event, so the order is observable only by reflecting on its delegate field from inside itsDispose-- an invasive seam. No mutation was run for that reason.Not changed, noticed:
IsoTpChannel.Disposestill unsubscribes before disposing the owned actor (with a comment giving its reason), unlike the J1939Disposepaths. That is outside #447 and left alone.Type of change
feat- new behaviour (minor release)fix/perf- bug or performance fix (patch release)docs/test/refactor/chore/ci- no releaseChecklist
-p:CI=true(0 warnings, 0 errors), IsoTp tests (286,net10.0) anddotnet format --verify-no-changespass locally. Pack not run (no csproj/props change).Closes #447
Generated by Claude Code