From ce73c9079be5def46d65fe26bee3a86379e09d25 Mon Sep 17 00:00:00 2001 From: Dietmar Borgards Date: Tue, 27 Oct 2026 04:46:58 +0100 Subject: [PATCH 1/2] test: keep ordering holds until explicit release --- .../CANopen/CanOpenForeignPdoObserveTests.cs | 51 ++++++------ .../IsoTp/IsoTpChannelIntegrationTests.cs | 79 +++++++++++-------- 2 files changed, 74 insertions(+), 56 deletions(-) diff --git a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenForeignPdoObserveTests.cs b/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenForeignPdoObserveTests.cs index 7767d37e..c80d835f 100644 --- a/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenForeignPdoObserveTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenForeignPdoObserveTests.cs @@ -778,14 +778,14 @@ public async Task Overlapping_observations_of_one_peer_both_read_the_live_mappin var file = PeerFile(); tool.BindPeerDeviceDescription(Peer, file); using var entered = new SemaphoreSlim(0, 1); - using var release = new SemaphoreSlim(0, 1); + using var release = new ManualResetEventSlim(); var held = 0; using var server = new ScriptedUploadServer(responder, Peer, (index, sub) => { if (index == 0x1800 && sub == 1 && System.Threading.Interlocked.Exchange(ref held, 1) == 0) { entered.Release(); - release.Wait(ShortTimeout); + release.Wait(); } if (index is >= 0x1400 and <= 0x1403 or >= 0x1800 and <= 0x1803) return index == 0x1800 && sub == 1 ? U32(Tpdo1) : null; @@ -795,26 +795,33 @@ public async Task Overlapping_observations_of_one_peer_both_read_the_live_mappin var firstSink = new ListSink(); var secondSink = new ListSink(); - var first = tool.ObserveForeignPdoAsync(Peer, Tpdo1, new byte[] { 0x5A }, file, firstSink); - entered.Wait(ShortTimeout).Should().BeTrue(); - // The call runs synchronously up to its first wait, which is the per-peer queue while the - // first observation holds it; without that queue it would take the peer's busy SDO - // channel right here, be refused, and complete from the file. So when it returns, the - // overlap under test has happened -- no sleep is needed to let it queue. - var second = tool.ObserveForeignPdoAsync(Peer, Tpdo1, new byte[] { 0xA5 }, file, secondSink); - second.IsCompleted.Should().BeFalse("the second observation waits for the first"); - release.Release(); - - var firstResult = await first.WithTimeoutAsync(ShortTimeout); - var secondResult = await second.WithTimeoutAsync(ShortTimeout); - firstResult.Observations.Should().ContainSingle(); - firstResult.Observations[0].Origin.Should().Be(ForeignPdoMappingOrigin.LiveMapping); - firstSink.Signals.Should().ContainSingle(); - firstSink.Signals[0].Value.Should().Equal(0x5A); - secondResult.Observations.Should().ContainSingle(); - secondResult.Observations[0].Origin.Should().Be(ForeignPdoMappingOrigin.LiveMapping); - secondSink.Signals.Should().ContainSingle(); - secondSink.Signals[0].Value.Should().Equal(0xA5); + try + { + var first = tool.ObserveForeignPdoAsync(Peer, Tpdo1, new byte[] { 0x5A }, file, firstSink); + entered.Wait(ShortTimeout).Should().BeTrue(); + // The call runs synchronously up to its first wait, which is the per-peer queue while the + // first observation holds it; without that queue it would take the peer's busy SDO + // channel right here, be refused, and complete from the file. So when it returns, the + // overlap under test has happened -- no sleep is needed to let it queue. + var second = tool.ObserveForeignPdoAsync(Peer, Tpdo1, new byte[] { 0xA5 }, file, secondSink); + second.IsCompleted.Should().BeFalse("the second observation waits for the first"); + release.Set(); + + var firstResult = await first.WithTimeoutAsync(ShortTimeout); + var secondResult = await second.WithTimeoutAsync(ShortTimeout); + firstResult.Observations.Should().ContainSingle(); + firstResult.Observations[0].Origin.Should().Be(ForeignPdoMappingOrigin.LiveMapping); + firstSink.Signals.Should().ContainSingle(); + firstSink.Signals[0].Value.Should().Equal(0x5A); + secondResult.Observations.Should().ContainSingle(); + secondResult.Observations[0].Origin.Should().Be(ForeignPdoMappingOrigin.LiveMapping); + secondSink.Signals.Should().ContainSingle(); + secondSink.Signals[0].Value.Should().Equal(0xA5); + } + finally + { + release.Set(); + } } [Fact] diff --git a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs index 89a82562..8075a223 100644 --- a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs @@ -2657,44 +2657,55 @@ private static async Task CallerCancelsASendQueuedOnTheGateAsync(bool disposeAsL using var bus = OpenClassic(session, 0); using var service = new CanBusService(bus); using var actor = new ProtocolActor(); - var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x123, 0x321), FastOptions(), - ownsService: false, actor); - using var release = new ManualResetEventSlim(); - using var callerCts = new CancellationTokenSource(); - var busy = actor.PostAsync(() => release.Wait(TimeSpan.FromSeconds(30))); + using (var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x123, 0x321), FastOptions(), + ownsService: false, actor)) + { + using var release = new ManualResetEventSlim(); + using var callerCts = new CancellationTokenSource(); + var busy = actor.PostAsync(() => release.Wait()); - var send1 = channel.SendAsync(new byte[] { 0x01 }); - var send2 = channel.SendAsync(new byte[] { 0x02 }, callerCts.Token); - send2.IsCompleted.Should().BeFalse(); - // Ends once Dispose is under way, and so after its first line, which marks the channel - // disposed: from then on the actor fails send1 instead of beginning it. - var receive = channel.ReceiveAsync(); - var late = new LateDisposal(disposeAsLateAsPossible, send1); + try + { + var send1 = channel.SendAsync(new byte[] { 0x01 }); + var send2 = channel.SendAsync(new byte[] { 0x02 }, callerCts.Token); + send2.IsCompleted.Should().BeFalse(); + // Ends once Dispose is under way, and so after its first line, which marks the channel + // disposed: from then on the actor fails send1 instead of beginning it. + var receive = channel.ReceiveAsync(); + var late = new LateDisposal(disposeAsLateAsPossible, send1); + + try + { + callerCts.Cancel(); + var queued = await Assert.ThrowsAsync(() => send2.WaitAsync(ShortTimeout)); + queued.CancellationToken.Should().Be(callerCts.Token); + } + finally + { + var disposal = Task.Run(async () => + { + await late.MayStartAsync().ConfigureAwait(false); + channel.Dispose(); + }); + // Released only once the disposal is seen to have begun (#400): released earlier, + // send1 can begin and complete before Dispose reaches it, and then it owes no + // ObjectDisposedException. + await late.WaitForAsync(receive); + late.ActorReleased(); + release.Set(); + await busy; + await disposal.WaitAsync(ShortTimeout); + } - try - { - callerCts.Cancel(); - var queued = await Assert.ThrowsAsync(() => send2.WaitAsync(ShortTimeout)); - queued.CancellationToken.Should().Be(callerCts.Token); - } - finally - { - var disposal = Task.Run(async () => + await Assert.ThrowsAsync(() => send1.WaitAsync(ShortTimeout)); + } + finally { - await late.MayStartAsync().ConfigureAwait(false); - channel.Dispose(); - }); - // Released only once the disposal is seen to have begun (#400): released earlier, - // send1 can begin and complete before Dispose reaches it, and then it owes no - // ObjectDisposedException. - await late.WaitForAsync(receive); - late.ActorReleased(); - release.Set(); - await busy; - await disposal.WaitAsync(ShortTimeout); + // Also releases the actor when setup or an assertion fails before the disposal probe. + release.Set(); + await busy.WaitAsync(ShortTimeout); + } } - - await Assert.ThrowsAsync(() => send1.WaitAsync(ShortTimeout)); } /// Starts a disposal as late as the test lets it: not before the test waits on it, From 1d9dc7c6bc06234ba74be3b5e16fa2462ba30e62 Mon Sep 17 00:00:00 2001 From: Dietmar Borgards Date: Tue, 27 Oct 2026 05:08:09 +0100 Subject: [PATCH 2/2] test(isotp): simplify queued-send disposal cleanup --- .../IsoTp/IsoTpChannelIntegrationTests.cs | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs index 8075a223..e5d8ec42 100644 --- a/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/IsoTp/IsoTpChannelIntegrationTests.cs @@ -2674,28 +2674,25 @@ private static async Task CallerCancelsASendQueuedOnTheGateAsync(bool disposeAsL var receive = channel.ReceiveAsync(); var late = new LateDisposal(disposeAsLateAsPossible, send1); - try - { - callerCts.Cancel(); - var queued = await Assert.ThrowsAsync(() => send2.WaitAsync(ShortTimeout)); - queued.CancellationToken.Should().Be(callerCts.Token); - } - finally + callerCts.Cancel(); + var queued = await Assert.ThrowsAsync(() => send2.WaitAsync(ShortTimeout)); + queued.CancellationToken.Should().Be(callerCts.Token); + + // Observe Dispose as part of the test sequence. The using block and outer finally + // own failure cleanup, releasing the actor before the channel is torn down. + var disposal = Task.Run(async () => { - var disposal = Task.Run(async () => - { - await late.MayStartAsync().ConfigureAwait(false); - channel.Dispose(); - }); - // Released only once the disposal is seen to have begun (#400): released earlier, - // send1 can begin and complete before Dispose reaches it, and then it owes no - // ObjectDisposedException. - await late.WaitForAsync(receive); - late.ActorReleased(); - release.Set(); - await busy; - await disposal.WaitAsync(ShortTimeout); - } + await late.MayStartAsync().ConfigureAwait(false); + channel.Dispose(); + }); + // Released only once the disposal is seen to have begun (#400): released earlier, + // send1 can begin and complete before Dispose reaches it, and then it owes no + // ObjectDisposedException. + await late.WaitForAsync(receive); + late.ActorReleased(); + release.Set(); + await busy; + await disposal.WaitAsync(ShortTimeout); await Assert.ThrowsAsync(() => send1.WaitAsync(ShortTimeout)); }