From 10538913cc723a79433164006ab3ad6622bff591 Mon Sep 17 00:00:00 2001 From: Dietmar Borgards Date: Sun, 25 Oct 2026 10:51:58 +0100 Subject: [PATCH] fix(isotp): forward the owned actor shutdown timeout before detaching --- src/CanKit.Pro.IsoTp/IsoTpChannel.cs | 10 +++--- .../IsoTp/IsoTpChannelIntegrationTests.cs | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/CanKit.Pro.IsoTp/IsoTpChannel.cs b/src/CanKit.Pro.IsoTp/IsoTpChannel.cs index 32d5ae4e..e0362952 100644 --- a/src/CanKit.Pro.IsoTp/IsoTpChannel.cs +++ b/src/CanKit.Pro.IsoTp/IsoTpChannel.cs @@ -192,7 +192,7 @@ internal sealed class IsoTpChannel : IIsoTpChannel /// /// Builds a channel on . A null -- the only /// value production passes -- makes the channel create and own its own loop; an injected one - /// stays the caller's to dispose. + /// stays the caller's to dispose unless the test explicitly transfers ownership. /// /// /// The actor parameter is a seam for tests, not a feature: substituting a loop built on a @@ -205,13 +205,13 @@ internal sealed class IsoTpChannel : IIsoTpChannel /// wire (#112), which are facts about the host and deliberately not on the actor's clock. /// internal IsoTpChannel(ICanBusService service, IsoTpEndpoint endpoint, - IsoTpChannelOptions options, bool ownsService, IProtocolActor? actor = null) + IsoTpChannelOptions options, bool ownsService, IProtocolActor? actor = null, bool ownsActor = false) { _service = service ?? throw new ArgumentNullException(nameof(service)); _endpoint = endpoint; _options = options ?? throw new ArgumentNullException(nameof(options)); _ownsService = ownsService; - _ownsActor = actor is null; + _ownsActor = actor is null || ownsActor; // Encode once: EncodeStMin throws on negative values; surfacing at Open keeps the RX // path free of codec throws that ProtocolActor would only raise as BackgroundException. _localStMinRaw = IsoTpFrameCodec.EncodeStMin(_options.LocalStMin); @@ -772,8 +772,10 @@ void FailInFlightSend() // actor is not ours to dispose -- the caller may still be running other channels on it -- // but the handler is, so it comes off either way rather than outliving this channel on a // loop that keeps going. - _actor.BackgroundExceptionOccurred -= OnActorBackgroundException; + // Keep forwarding through the owned actor's shutdown: Dispose itself can report a + // callback that exceeded its join timeout (FR-RAW-023, #449). if (_ownsActor) _actor.Dispose(); + _actor.BackgroundExceptionOccurred -= OnActorBackgroundException; _readerCts.Dispose(); // _sendGate is deliberately not disposed, and not waited for either (#339): the wait diff --git a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs index f19bf7ae..d2bcf449 100644 --- a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs @@ -3445,6 +3445,39 @@ public async Task Flow_Parameters_Come_From_The_First_Flow_Control_Only() cfs.Should().Be(4); } + // FR-RAW-023, #449: the owned actor reports a shutdown timeout during Dispose itself. + // The callback remains held until finally, so the timeout cannot race its release. + [Fact] + public async Task The_Shutdown_Timeout_Of_The_Channels_Own_Actor_Reaches_Its_Subscribers() + { + using var bus = ControllableBus.EchoCapable(NewSession()); + using var service = new CanBusService(bus); + using var release = new ManualResetEventSlim(); + using var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, null, + shutdownTimeout: TimeSpan.FromMilliseconds(50)); + using var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x700, 0x708), + new IsoTpChannelOptions(), ownsService: false, actor, ownsActor: true); + var timedOut = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + channel.BackgroundExceptionOccurred += (_, ex) => { if (ex is TimeoutException) timedOut.TrySetResult(ex); }; + var held = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + try + { + actor.Post(() => + { + held.TrySetResult(true); + release.Wait(); + }); + await held.Task.WaitAsync(ShortTimeout); + channel.Dispose(); + + (await timedOut.Task.WaitAsync(ShortTimeout)).Should().BeOfType(); + } + finally + { + release.Set(); + } + } + // #56 -- DiscardPendingPdus called from a BackgroundExceptionOccurred handler, which runs on // the actor: the post-and-wait would wait for the loop it is on. It runs inline there. [Fact]