fix(canopen): stop a block download's sub-block batch when the transfer ends

src/CanKit.Pro.CANopen/CanOpenNode.SdoBlock.cs

@@ -585,7 +585,10 @@ private void SendNextBlockDownloadSubBlock(SdoBlockClientSession session)
         }
         session.ResumeSeqno = seqno;
         session.Phase = SdoBlockClientPhase.AwaitSubBlockAck;
-        _ = SendOrderedControlFrames(TrackSdoBlockClientSend(session), frames.ToArray());
+        // Every way the transfer ends completes its task (abort, peer abort, cancel, timeout,
+        // dispose), and that is readable from the sending task; the session tables are not.
+        _ = SendOrderedControlFrames(TrackSdoBlockClientSend(session),
+            () => session.Tcs.Task.IsCompleted, frames.ToArray());
     }
 
     private void SendBlockDownloadEnd(SdoBlockClientSession session)

src/CanKit.Pro.CANopen/CanOpenNode.cs

@@ -2369,6 +2369,17 @@ private Task SendOrderedControlFrames(params (uint CobId, byte[] Payload)[] fram
     /// </summary>
     private Task SendOrderedControlFrames(Action<CanOpenTransportException?>? onSendCompleted,
         params (uint CobId, byte[] Payload)[] frames)
+        => SendOrderedControlFrames(onSendCompleted, shouldStop: null, frames);
+
+    /// <summary>
+    /// As above. <paramref name="shouldStop"/>, when given, is asked before each frame and ends the
+    /// batch without sending the rest: a block transfer sends a whole sub-block as one batch, and
+    /// when the transfer ends meanwhile (the server aborted, the caller cancelled) the segments
+    /// still to go would reach a server that has no session for them any more, and read as
+    /// command specifiers. It runs on the sending task, so it must read thread-safe state only.
+    /// </summary>
+    private Task SendOrderedControlFrames(Action<CanOpenTransportException?>? onSendCompleted,
+        Func<bool>? shouldStop, params (uint CobId, byte[] Payload)[] frames)
     {
         return Task.Run(async () =>
         {
@@ -2377,6 +2388,7 @@ private Task SendOrderedControlFrames(Action<CanOpenTransportException?>? onSend
             {
                 foreach (var (cobId, payload) in frames)
                 {
+                    if (shouldStop?.Invoke() == true) return;
                     try
                     {
                         var frame = CanFrame.Classic(unchecked((int)cobId), payload, isExtendedFrame: false);

tests/CanKit.Pro.Tests/TestCases/CANopen/CanOpenSdoClientSendFailureTests.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using AwesomeAssertions;
@@ -50,6 +51,63 @@ await FluentActions.Awaiting(() => act().WithTimeoutAsync(ShortTimeout))
         reported.Should().Contain(ex => ex is CanOpenTransportException);
     }
 
+    public static TheoryData<string> BlockDownloadEndings => new() { "the server aborts", "the caller cancels" };
+
+    // A block download sends a whole sub-block as one ordered batch. When the transfer ends while
+    // the batch is still going -- the server aborted, or the caller cancelled -- the segments still
+    // to go must not be sent: the server has no session for them any more and reads each as a
+    // command specifier, a seqno of 0x20..0x3F being a classic download initiate.
+    [Theory]
+    [MemberData(nameof(BlockDownloadEndings))]
+    public async Task A_Block_Download_That_Ends_Mid_SubBlock_Sends_No_More_Segments(string ending)
+    {
+        using var bus = ControllableBus.DeferredEchoCapable($"canopen-sdo-block-end-{Guid.NewGuid():N}");
+        var toServer = new List<byte[]>();
+        bus.OnTransmitting = frame =>
+        {
+            if ((uint)frame.ID == CanOpenCobId.SdoRx(0x11)) lock (toServer) toServer.Add(frame.Data.ToArray());
+        };
+        using var client = CanOpen.OpenNode(bus, 0x7F, new CanOpenNodeOptions { SdoTimeout = TimeSpan.FromSeconds(30) });
+        await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // the boot-up
+        bus.DeferredEchoes.ReleaseAll();
+
+        var payload = Enumerable.Range(0, 70).Select(i => (byte)(0x30 + i)).ToArray(); // ten segments
+        using var cts = new CancellationTokenSource();
+        var download = client.SdoDownloadAsync(0x11, 0x1000, 0x00, payload, SdoTransferMode.Block, cts.Token);
+        await bus.DeferredEchoes.WaitForEnqueuedAsync(2, ShortTimeout); // the initiate
+        bus.DeferredEchoes.ReleaseNext();
+        bus.RaiseObserved(CanFrame.Classic(unchecked((int)CanOpenCobId.SdoTx(0x11)),
+            SdoBlockFrames.BuildBlockDownloadInitResponse(0x1000, 0x00, serverCrcSupported: false, blockSize: 10)),
+            isEcho: false);
+        await bus.DeferredEchoes.WaitForEnqueuedAsync(3, ShortTimeout); // segment 1, its confirmation held
+
+        if (ending == "the server aborts")
+        {
+            bus.RaiseObserved(CanFrame.Classic(unchecked((int)CanOpenCobId.SdoTx(0x11)),
+                SdoFrames.BuildAbort(0x1000, 0x00, (uint)SdoAbortCode.General)), isEcho: false);
+            await FluentActions.Awaiting(() => download.WithTimeoutAsync(ShortTimeout))
+                .Should().ThrowAsync<SdoAbortException>();
+        }
+        else
+        {
+            cts.Cancel();
+            await FluentActions.Awaiting(() => download.WithTimeoutAsync(ShortTimeout))
+                .Should().ThrowAsync<OperationCanceledException>();
+        }
+
+        // The transfer is over; let the batch carry on. A batch that ignores that sends nine more.
+        bus.DeferredEchoes.ReleaseAll();
+        // No signal says "nothing more is coming", so this is a negative window: it can only pass
+        // falsely on a slow host, never fail falsely.
+        await Task.Delay(TimeSpan.FromMilliseconds(300));
+        bus.DeferredEchoes.ReleaseAll();
+
+        byte[][] sent;
+        lock (toServer) sent = toServer.ToArray();
+        sent.Length.Should().Be(ending == "the server aborts" ? 2 : 3,
+            "the initiate and segment 1 -- and, after a cancel, the abort -- and nothing after");
+    }
+
     public static TheoryData<string> Uploads => new() { "upload", "block upload" };
 
     [Theory]