Skip to content
Merged
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
16 changes: 12 additions & 4 deletions src/CanKit.Pro.IsoTp/IsoTpChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,20 @@ internal sealed class IsoTpChannel : IIsoTpChannel
/// so substituting the actor puts all of them on the one clock the test controls. The
/// <c>Stopwatch</c> readings that remain here are arrival and transmit *instants* on the
/// wire (#112), which are facts about the host and deliberately not on the actor's clock.
/// <para>
/// <paramref name="ownsActor"/> hands an injected <paramref name="actor"/> to the channel to
/// dispose, as the one it creates for itself: a seam for a test that needs the channel's own
/// disposal of its actor on a loop with a short shutdown bound (#449).
/// </para>
/// </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 @@ -771,9 +776,12 @@ void FailInFlightSend()
// SendAsync can leave its await and enter WaitForBusTxIdleAsync / Release. An injected
// 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;
// loop that keeps going. For an owned actor only after its Dispose: that is where the
// actor reports a callback that outlived its shutdown bound, as a TimeoutException through
// its BackgroundExceptionOccurred, and unsubscribed first the channel's subscribers never
// heard of it (#449, as #437 for the J1939-TP channel).
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 @@ -1337,6 +1337,47 @@ public async Task A_Pending_Receive_Ends_On_Dispose_Although_The_Actor_Is_Busy()
}
}

// #449: the channel's own actor, disposed while a callback still holds its loop, gives up on
// the join after its shutdown bound and reports that as a TimeoutException through its own
// BackgroundExceptionOccurred. The channel forwards it only while its handler is still
// attached, so the handler comes off after that Dispose, not before (as #437 for the J1939-TP
// channel). The callback is held until the finally, so the bound is always exceeded: nothing
// is asserted against the clock.
[Fact]
public async Task The_Shutdown_Timeout_Of_The_Channels_Own_Actor_Reaches_Its_Subscribers()
{
var session = NewSession();
using var bus = OpenClassic(session, 0);
using var service = new CanBusService(bus);
using var release = new ManualResetEventSlim(); // declared first, so it outlives the channel
var actor = new ProtocolActor(ActorExecutionMode.DedicatedThread, null, null, shutdownTimeout: TimeSpan.FromMilliseconds(50));
using var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x123, 0x321), FastOptions(),
ownsService: false, actor, ownsActor: true); // disposes the actor it owns
// Reported on the thread pool (#356, T-2), so possibly after Dispose has returned: awaited
// on a signal. A report the channel never forwards leaves the signal unset and the wait fails.
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(TimeSpan.FromSeconds(30));
});
await held.Task.WaitAsync(ShortTimeout);
channel.Dispose(); // the actor's join gives up on the held callback and reports it

(await timedOut.Task.WaitAsync(ShortTimeout)).Should().BeOfType<TimeoutException>(
"the channel owns that actor, and its shutdown timeout is a background failure of the channel's");
}
finally
{
release.Set();
}
}

// Review on #275: the inbox's writer stays open now, so a producer callback that passed the
// disposed guard before the disposal and writes after it must not put anything in it. The
// callback is simulated by calling the producers directly after the disposal.
Expand Down
Loading