Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/CanKit.Pro.IsoTp/IsoTpChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ internal sealed class IsoTpChannel : IIsoTpChannel
/// <summary>
/// Builds a channel on <paramref name="service"/>. A null <paramref name="actor"/> -- 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.
/// </summary>
/// <remarks>
/// The actor parameter is a seam for tests, not a feature: substituting a loop built on a
Expand All @@ -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.
/// </remarks>
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);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Exception>(TaskCreationOptions.RunContinuationsAsynchronously);
channel.BackgroundExceptionOccurred += (_, ex) => { if (ex is TimeoutException) timedOut.TrySetResult(ex); };
var held = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
try
{
actor.Post(() =>
{
held.TrySetResult(true);
release.Wait();
});
await held.Task.WaitAsync(ShortTimeout);
channel.Dispose();

(await timedOut.Task.WaitAsync(ShortTimeout)).Should().BeOfType<TimeoutException>();
}
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]
Expand Down
Loading