From f3e54d65cb791374b16112b6365b33aae3882464 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 08:48:08 -0600 Subject: [PATCH 1/4] fix(sdcard): bound the SD download path so a wedged card cannot hang forever (closes #399) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An SD file download against a device whose SD subsystem is wedged hung indefinitely and ignored cancellation: nothing on the parked path observed the token, so a consumer-side stall watchdog could see the hang but never end it. Three bounds, from the outside in: - SerialStreamTransport: WriteTimeout was set to the connect timeout and, unlike ReadTimeout, never lowered after open — so it kept that value for the life of the port. SerialPort.Write takes no CancellationToken, so a device that stops draining its receive buffer parks a write with nothing able to interrupt it. Both directions are now bounded after open. - DownloadSdCardFileAsync enforces its own overall deadline instead of trusting the parked call to notice a token: the transfer runs on a worker task raced against the deadline AND the caller's token, so both now end the call even when the transfer cannot observe either. On expiry the transfer is abandoned rather than awaited (documented on the method). Retries draw from the remaining budget rather than getting a fresh 30 minutes each. - SdCardFileReceiver checks its token every iteration. A serial read returns 0 bytes on its own timeout without ever having looked at the token, and the loop reported that as "transport stream closed" — a timeout — even for a transfer the caller had already cancelled. Tests cover the deadline, the caller-cancellation rescue, a synchronous park before the first await, and a slow-but-healthy transfer that must not be falsely aborted; each was verified to fail without the corresponding fix. Co-Authored-By: Claude Opus 5 --- .../Transport/SerialStreamTransportTests.cs | 20 ++ .../Device/SdCard/SdCardFileReceiverTests.cs | 52 ++++ .../Device/SdCard/SdCardOperationsTests.cs | 288 ++++++++++++++++++ .../Transport/SerialStreamTransport.cs | 38 ++- .../Device/DaqifiStreamingDevice.cs | 226 +++++++++++--- .../Device/SdCard/SdCardFileReceiver.cs | 8 + 6 files changed, 592 insertions(+), 40 deletions(-) diff --git a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs index 489e36c8..725b47a9 100644 --- a/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs +++ b/src/Daqifi.Core.Tests/Communication/Transport/SerialStreamTransportTests.cs @@ -53,6 +53,26 @@ public void SerialStreamTransport_Stream_WhenNotConnected_ThrowsTransportNotConn Assert.Contains("COM1", ex.Message); } + [Fact] + public void SerialStreamTransport_ApplyOperationalTimeouts_BoundsBothDirections() + { + // #399: only ReadTimeout used to be lowered after open, so WriteTimeout kept the connect + // timeout for the life of the port. SerialPort.Write takes no CancellationToken, so a + // device that stops draining its receive buffer parks the write for that whole duration + // with nothing able to interrupt it. Both directions must end up bounded and short. + using var port = new SerialPort("COM1") + { + ReadTimeout = 30000, + WriteTimeout = 30000 + }; + + SerialStreamTransport.ApplyOperationalTimeouts(port); + + Assert.Equal(500, port.ReadTimeout); + Assert.Equal(2000, port.WriteTimeout); + Assert.NotEqual(SerialPort.InfiniteTimeout, port.WriteTimeout); + } + [Fact] public void SerialStreamTransport_SetSerialPortForTesting_TakesOwnershipAndDisposesPreviousPort() { diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardFileReceiverTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardFileReceiverTests.cs index 7130afe0..bf5d050d 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardFileReceiverTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardFileReceiverTests.cs @@ -102,6 +102,27 @@ await Assert.ThrowsAnyAsync( cancellationToken: cts.Token)); } + [Fact] + public async Task ReceiveAsync_WhenCancelledAndStreamIgnoresTheToken_ReportsCancellationNotTimeout() + { + // #399: System.IO.Ports' SerialStream ignores the token once a read is in flight, and on a + // device that is sending nothing it just returns 0 bytes when its (500 ms) ReadTimeout + // elapses. The loop translated that into "transport stream closed" — a timeout — even when + // the caller had already cancelled, which is precisely the case where a consumer's stall + // watchdog wants to see its own cancellation come back. Cancellation has to win. + using var sourceStream = new TokenIgnoringSilentStream(); + using var destinationStream = new MemoryStream(); + var receiver = new SdCardFileReceiver(sourceStream); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => receiver.ReceiveAsync( + destinationStream, "test.bin", + timeout: TimeSpan.FromMinutes(5), + cancellationToken: cts.Token)); + } + [Fact] public async Task ReceiveAsync_ProgressReporting_BytesReceivedIncreases() { @@ -376,6 +397,37 @@ public override void Flush() { } public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); } + /// + /// A silent device: every read returns 0 bytes (a serial read timeout) and the cancellation + /// token is accepted and then ignored, exactly as System.IO.Ports' SerialStream behaves for a + /// read already in flight. + /// + private sealed class TokenIgnoringSilentStream : Stream + { + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override int Read(byte[] buffer, int offset, int count) => 0; + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + // Deliberately no ThrowIfCancellationRequested: the token is accepted, never acted on. + return Task.FromResult(0); + } + + public override void Flush() { } + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + /// /// A stream that blocks on read until cancellation is requested. /// Used to test cancellation behavior. diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs index 41d5a951..8f7bbd9b 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs @@ -5,6 +5,7 @@ using Daqifi.Core.Firmware; using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Net; @@ -1787,6 +1788,129 @@ public async Task DownloadSdCardFileAsync_MarkerOnlyThenSuccess_RetriesAndSuccee Assert.Equal(2, getCommands); } + [Fact] + public async Task DownloadSdCardFileAsync_WhenTransferParksIgnoringItsToken_ThrowsTimeoutException() + { + // #399: against a wedged SD subsystem the transfer parks somewhere that never looks + // at the token it was handed, so the only thing that can end the call is a deadline + // the download enforces itself. + var device = new ParkedDownloadDevice(ParkMode.Asynchronous, TimeSpan.FromMilliseconds(300)); + device.Connect(); + using var destinationStream = new MemoryStream(); + + try + { + var elapsed = Stopwatch.StartNew(); + var opTask = device.DownloadSdCardFileAsync("data.bin", destinationStream); + + // Guard the assertion rather than the test run: if the deadline never fires this + // fails instead of hanging CI forever. + var winner = await Task.WhenAny(opTask, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same((Task)opTask, winner); + + var ex = await Assert.ThrowsAsync(() => opTask); + elapsed.Stop(); + + Assert.Contains("data.bin", ex.Message); + + // It waited for the budget instead of failing early — the deadline is what ended + // it, not some unrelated fault. + Assert.True( + elapsed.Elapsed >= TimeSpan.FromMilliseconds(300), + $"Download failed after only {elapsed.ElapsedMilliseconds}ms, before the budget elapsed."); + } + finally + { + device.Release(); + } + } + + [Fact] + public async Task DownloadSdCardFileAsync_WhenTransferParksSynchronously_StillTimesOut() + { + // The suspected park in #399 (a blocking SerialPort.Write on a device that stopped + // draining) is in the transfer's SYNCHRONOUS prefix — before the first await. Unless + // that prefix runs on the worker task, it blocks the caller before any deadline can + // even be armed, and the call never returns a Task at all. Hence the Task.Run: it + // keeps the test thread free so a regression fails here instead of wedging the run. + var device = new ParkedDownloadDevice(ParkMode.Synchronous, TimeSpan.FromMilliseconds(300)); + device.Connect(); + using var destinationStream = new MemoryStream(); + + try + { + var opTask = Task.Run(() => device.DownloadSdCardFileAsync("data.bin", destinationStream)); + + var winner = await Task.WhenAny(opTask, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same((Task)opTask, winner); + + await Assert.ThrowsAsync(() => opTask); + } + finally + { + device.Release(); + } + } + + [Fact] + public async Task DownloadSdCardFileAsync_WhenTransferParksIgnoringItsToken_CallerCancellationStillEndsTheCall() + { + // The consumer-side stall watchdog described in #399: it cancels its token at 90s and + // nothing happens, because the parked path never polls it. The download must observe + // the caller's token itself so cancelling actually ends the await — well before the + // (deliberately distant) deadline below. + var device = new ParkedDownloadDevice(ParkMode.Asynchronous, TimeSpan.FromSeconds(60)); + device.Connect(); + using var destinationStream = new MemoryStream(); + using var cts = new CancellationTokenSource(); + + try + { + var opTask = device.DownloadSdCardFileAsync("data.bin", destinationStream, null, cts.Token); + + // Only cancel once the transfer is genuinely parked, so the test proves the token + // reaches a call already in flight rather than the up-front guard. + Assert.True(device.Parked.Wait(TimeSpan.FromSeconds(30)), "Transfer never reached the park."); + cts.Cancel(); + + var winner = await Task.WhenAny(opTask, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same((Task)opTask, winner); + + await Assert.ThrowsAnyAsync(() => opTask); + } + finally + { + device.Release(); + } + } + + [Fact] + public async Task DownloadSdCardFileAsync_SlowButHealthyTransfer_IsNotAborted() + { + // The bounding must only end transfers that are stuck. A slow-but-progressing one — + // a large file, a busy device — has to run to completion untouched. + var fileData = new byte[240]; + new Random(399).NextBytes(fileData); + + var device = new SlowDownloadDevice( + fileData, + chunkSize: 16, + chunkDelay: TimeSpan.FromMilliseconds(40), + budget: TimeSpan.FromSeconds(10)); + device.Connect(); + using var destinationStream = new MemoryStream(); + + // Act + var result = await device.DownloadSdCardFileAsync("data.bin", destinationStream); + + // Assert — every byte arrived, and the transfer really did take its time getting here. + Assert.Equal(fileData, destinationStream.ToArray()); + Assert.Equal(fileData.Length, result.FileSize); + Assert.True( + result.Duration >= TimeSpan.FromMilliseconds(200), + $"Transfer completed in {result.Duration.TotalMilliseconds}ms — too fast to prove a slow transfer survives."); + } + #endregion /// @@ -2090,6 +2214,170 @@ protected override async Task ExecuteRawCaptureAsync( } } + /// + /// How simulates a wedged transfer. + /// + private enum ParkMode + { + /// Parks in an awaited task that never observes the token (a read that never returns). + Asynchronous, + + /// Blocks the calling thread outright, before the first await (a blocking write that never drains). + Synchronous + } + + /// + /// A device whose raw-capture path parks and never observes the cancellation token it was + /// handed — the #399 failure mode, where neither the caller's token nor a consumer-side + /// watchdog can reach whatever the transfer is stuck in. unblocks the + /// abandoned worker at the end of the test so it cannot outlive it. + /// + private sealed class ParkedDownloadDevice : DaqifiStreamingDevice + { + private readonly TaskCompletionSource _asyncPark = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly ManualResetEventSlim _syncPark = new(false); + private readonly ParkMode _mode; + private readonly TimeSpan _budget; + + public ParkedDownloadDevice(ParkMode mode, TimeSpan budget) + : base("TestDevice") + { + _mode = mode; + _budget = budget; + } + + /// Set once the transfer has actually reached the park. + public ManualResetEventSlim Parked { get; } = new(false); + + public override bool IsUsbConnection => true; + + internal override TimeSpan SdCardDownloadTimeout => _budget; + + public override void Send(IOutboundMessage message) + { + // Swallowed: this device never gets as far as exchanging commands. + } + + protected override async Task ExecuteRawCaptureAsync( + Func rawAction, + CancellationToken cancellationToken = default) + { + Parked.Set(); + + // Deliberately ignores cancellationToken — that is the whole point. + if (_mode == ParkMode.Synchronous) + { + _syncPark.Wait(); + return; + } + + await _asyncPark.Task.ConfigureAwait(false); + } + + /// Lets the abandoned transfer finish so no thread is left parked after the test. + public void Release() + { + _asyncPark.TrySetResult(); + _syncPark.Set(); + } + } + + /// + /// A device that serves a healthy file slowly — small chunks with a delay between them — + /// so tests can show that bounding a stuck download does not abort a progressing one. + /// + private sealed class SlowDownloadDevice : DaqifiStreamingDevice + { + private readonly SlowChunkStream _stream; + private readonly TimeSpan _budget; + + public SlowDownloadDevice(byte[] fileData, int chunkSize, TimeSpan chunkDelay, TimeSpan budget) + : base("TestDevice") + { + _stream = new SlowChunkStream(fileData, chunkSize, chunkDelay); + _budget = budget; + } + + public override bool IsUsbConnection => true; + + internal override TimeSpan SdCardDownloadTimeout => _budget; + + public override void Send(IOutboundMessage message) + { + // Not asserted on by the slow-transfer tests. + } + + protected override async Task ExecuteRawCaptureAsync( + Func rawAction, + CancellationToken cancellationToken = default) + { + await rawAction(_stream, cancellationToken).ConfigureAwait(false); + } + } + + /// + /// Serves file data plus the EOF marker in small chunks, pausing between them, to imitate + /// a large transfer trickling in over USB. + /// + private sealed class SlowChunkStream : Stream + { + private static readonly byte[] EofMarker = Encoding.ASCII.GetBytes("__END_OF_FILE__"); + + private readonly byte[] _data; + private readonly int _chunkSize; + private readonly TimeSpan _chunkDelay; + private int _position; + + public SlowChunkStream(byte[] fileData, int chunkSize, TimeSpan chunkDelay) + { + _data = new byte[fileData.Length + EofMarker.Length]; + Array.Copy(fileData, 0, _data, 0, fileData.Length); + Array.Copy(EofMarker, 0, _data, fileData.Length, EofMarker.Length); + _chunkSize = chunkSize; + _chunkDelay = chunkDelay; + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => _data.Length; + public override long Position + { + get => _position; + set => throw new NotSupportedException(); + } + + public override int Read(byte[] buffer, int offset, int count) + { + Thread.Sleep(_chunkDelay); + var available = _data.Length - _position; + if (available <= 0) return 0; + + var toRead = Math.Min(Math.Min(count, _chunkSize), available); + Array.Copy(_data, _position, buffer, offset, toRead); + _position += toRead; + return toRead; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + await Task.Delay(_chunkDelay, cancellationToken).ConfigureAwait(false); + + var available = _data.Length - _position; + if (available <= 0) return 0; + + var toRead = Math.Min(Math.Min(count, _chunkSize), available); + Array.Copy(_data, _position, buffer, offset, toRead); + _position += toRead; + return toRead; + } + + public override void Flush() { } + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + /// /// A testable device that reports IsUsbConnection = false to verify /// that SD card operations reject non-USB connections. diff --git a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs index bdb868b2..838a8c74 100644 --- a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs +++ b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs @@ -19,6 +19,23 @@ public class SerialStreamTransport : IStreamTransport private SerialPort? _serialPort; private bool _disposed; + /// + /// Blocking-read timeout applied once the port is open. The connection timeout is only + /// needed for retry/backoff logic, not for reads during normal operation; a short value + /// keeps consumer threads stoppable (StopSafely). + /// + private const int OperationalReadTimeoutMs = 500; + + /// + /// Blocking-write timeout applied once the port is open. Everything written over this + /// transport is a short SCPI command line, so a healthy device drains a write in + /// milliseconds. Leaving the connect timeout in place (and, on a wedged device, blocking + /// for it) is the failure mode issue #399 is about: + /// accepts no , so an unbounded write cannot be cancelled by + /// any caller. Bounding it turns "parked forever" into a fault the caller can observe. + /// + private const int OperationalWriteTimeoutMs = 2000; + /// /// Initializes a new instance of the SerialStreamTransport class. /// @@ -171,11 +188,9 @@ await ConnectRetryExecutor.ExecuteAsync( _serialPort.Open(); - // After a successful open, lower the ReadTimeout to a short operational - // value. The connection timeout is only needed for retry/backoff logic, - // not for blocking reads during normal operation. A short ReadTimeout - // ensures consumer threads can be stopped promptly (StopSafely). - _serialPort.ReadTimeout = 500; + // After a successful open, swap the connect timeouts for the (shorter) + // operational ones — both directions, not just reads (#399). + ApplyOperationalTimeouts(_serialPort); return Task.CompletedTask; }, @@ -187,6 +202,19 @@ await ConnectRetryExecutor.ExecuteAsync( onStatusChanged: OnStatusChanged); } + /// + /// Replaces the connect-phase timeouts with the operational ones on an opened port. + /// Internal (not inlined at the call site) so the values can be asserted without a real + /// port: / are + /// settable while the port is closed and are carried into the handle on open. + /// + /// The port to configure. + internal static void ApplyOperationalTimeouts(SerialPort port) + { + port.ReadTimeout = OperationalReadTimeoutMs; + port.WriteTimeout = OperationalWriteTimeoutMs; + } + /// /// Closes the serial connection asynchronously. /// diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index ebcd6dcf..7105f6c0 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -2092,6 +2092,22 @@ public Task FormatSdCardAsync(CancellationToken cancellationToken = default) /// retry attempts, indicating its SD subsystem is not ready rather than the file being /// legitimately empty. /// + /// + /// Thrown when the download does not finish within . + /// The deadline is enforced by this method itself, so it still applies when the transfer + /// is parked in a call that cannot observe a cancellation token (#399). + /// + /// Thrown when is canceled. + /// + /// On a timeout — or a cancellation the parked transfer cannot itself observe — the + /// in-flight transfer is abandoned rather than awaited: it may be blocked in native + /// serial I/O that no token can interrupt, and waiting for it is the hang this method + /// exists to bound. Two consequences for callers: the abandoned transfer can still write + /// to after this method has thrown, so the stream + /// must not be reused for anything else; and the device is left mid-SD:GET, so + /// reconnecting (or power-cycling, if its SD subsystem is genuinely wedged) is the + /// reliable way to resume normal operation. + /// public async Task DownloadSdCardFileAsync( string fileName, Stream destinationStream, @@ -2129,49 +2145,56 @@ public async Task DownloadSdCardFileAsync( var stopwatch = Stopwatch.StartNew(); long fileSize = 0; + var budget = SdCardDownloadTimeout; try { - await ExecuteRawCaptureAsync(async (stream, ct) => + await RunWithHardDeadlineAsync(async token => { - // Prepare SD card interface - PrepareSdInterface(); - - // Small delay to let the interface switch settle - await Task.Delay(50, ct).ConfigureAwait(false); - - // Send the SCPI command to request the file - Send(ScpiMessageProducer.GetSdFile(fileName)); - - // Receive the file data. A marker-only (0-byte) transfer means the device's - // SD subsystem wasn't ready when it opened the file - the same kind of - // transient condition GetSdCardFilesAsync's LIST retry already absorbs - so - // retry the GET a bounded number of times before giving up (see #264). - var receiver = new SdCardFileReceiver(stream); - long bytesReceived; - var attempt = 0; - while (true) + await ExecuteRawCaptureAsync(async (stream, ct) => { - try - { - bytesReceived = await receiver.ReceiveAsync( - destinationStream, - fileName, - progress, - timeout: TimeSpan.FromMinutes(30), - cancellationToken: ct).ConfigureAwait(false); - break; - } - catch (SdCardEmptyTransferException) when (attempt < SD_LIST_MAX_RETRIES) + // Prepare SD card interface + PrepareSdInterface(); + + // Small delay to let the interface switch settle + await Task.Delay(50, ct).ConfigureAwait(false); + + // Send the SCPI command to request the file + Send(ScpiMessageProducer.GetSdFile(fileName)); + + // Receive the file data. A marker-only (0-byte) transfer means the device's + // SD subsystem wasn't ready when it opened the file - the same kind of + // transient condition GetSdCardFilesAsync's LIST retry already absorbs - so + // retry the GET a bounded number of times before giving up (see #264). + var receiver = new SdCardFileReceiver(stream); + long bytesReceived; + var attempt = 0; + while (true) { - attempt++; - await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); - Send(ScpiMessageProducer.GetSdFile(fileName)); + try + { + // Each attempt gets what is left of the overall budget, never a + // fresh full one: retries must not be able to push the total past + // the deadline the caller was promised. + bytesReceived = await receiver.ReceiveAsync( + destinationStream, + fileName, + progress, + timeout: RemainingBudget(budget, stopwatch), + cancellationToken: ct).ConfigureAwait(false); + break; + } + catch (SdCardEmptyTransferException) when (attempt < SD_LIST_MAX_RETRIES) + { + attempt++; + await Task.Delay(SD_INTERFACE_SETTLE_DELAY_MS, ct).ConfigureAwait(false); + Send(ScpiMessageProducer.GetSdFile(fileName)); + } } - } - fileSize = bytesReceived; - }, cancellationToken).ConfigureAwait(false); + fileSize = bytesReceived; + }, token).ConfigureAwait(false); + }, budget, fileName, cancellationToken).ConfigureAwait(false); } finally { @@ -2232,6 +2255,139 @@ public async Task DownloadSdCardFileAsync( } } + /// + /// Overall wall-clock budget for one + /// + /// call, covering every GET attempt. This is the per-transfer limit the receiver has + /// always applied; what #399 changed is that the download now enforces it itself instead + /// of trusting whatever the transfer is parked in to notice a cancellation token. + /// + /// Virtual only as a test seam — a 30-minute budget is not unit-testable. + internal virtual TimeSpan SdCardDownloadTimeout => TimeSpan.FromMinutes(30); + + /// + /// The part of not yet consumed, floored at zero (a negative + /// timeout is not a legal delay). + /// + private static TimeSpan RemainingBudget(TimeSpan budget, Stopwatch stopwatch) + { + var remaining = budget - stopwatch.Elapsed; + return remaining > TimeSpan.Zero ? remaining : TimeSpan.Zero; + } + + /// + /// The instant the download is given up on regardless of what it is doing. It sits just + /// past the cooperative so that a transfer which IS observing + /// its token still fails through the receiver's own timeout — which reports how many + /// bytes arrived — and the hard deadline only decides the case where it is not. + /// + private static TimeSpan HardDeadlineFor(TimeSpan budget) + { + var graceMs = Math.Clamp(budget.TotalMilliseconds * 0.1, 100, 5000); + return budget + TimeSpan.FromMilliseconds(graceMs); + } + + /// + /// Runs an SD download on a worker task and races it against a hard deadline, so neither + /// the deadline nor the caller's cancellation depends on the transfer being somewhere it + /// can observe a token (#399). On expiry the worker is abandoned rather than awaited. + /// + /// The transfer. Receives a token cancelled by caller cancellation or the deadline, whichever comes first. + /// The cooperative budget; the hard deadline is of it. + /// Used only in the message. + /// The caller's token, observed by the race itself and not only by the worker. + private static async Task RunWithHardDeadlineAsync( + Func operation, + TimeSpan budget, + string fileName, + CancellationToken cancellationToken) + { + var hardDeadline = HardDeadlineFor(budget); + + // hardDeadlineCts runs on its own timer, independent of the Task.Delay race below, so + // it still reaches the worker if the worker only returns long after the race was + // decided. linkedCts is what the worker observes: caller cancellation OR the deadline. + var hardDeadlineCts = new CancellationTokenSource(hardDeadline); + var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, hardDeadlineCts.Token); + + // Stops the racing delay the moment the outcome is decided — without it, a download + // that finishes in a second would leave a 30-minute timer registered behind it. + var raceCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + + // LongRunning (a dedicated thread, not a pooled one): the transfer's synchronous + // prefix — the consumer stop-and-join, PrepareSdInterface's blocking writes — otherwise + // runs on the CALLING thread up to the first await, which on a UI thread means a + // wedged device freezes the window, and which would put that prefix outside the very + // deadline it needs to be inside. A pooled Task.Run would also tie up a worker for the + // transfer's full blocking duration. Pass CancellationToken.None to StartNew itself: + // the worker's own token still cancels its waits, and "cancelled before start" must + // not surface as an operation fault. (Mirrors WifiBridgeActivator, #294/#295/#326.) + var workerTask = Task.Factory.StartNew( + () => operation(linkedCts.Token), + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default).Unwrap(); + + try + { + var winner = await Task.WhenAny( + workerTask, + Task.Delay(hardDeadline, raceCts.Token)).ConfigureAwait(false); + + // Only abandon when the worker is genuinely still running: WhenAny can hand back + // the delay even though the worker completed at that same boundary, and awaiting + // it below honors that result instead of discarding it. + if (winner != workerTask && !workerTask.IsCompleted) + { + // Cancel explicitly instead of relying on the deadline timer having fired: the + // delay above and hardDeadlineCts are two separate timers of the same duration, + // so the delay can win by a hair and leave a late-returning worker running one + // more state-changing step after the caller already threw. Idempotent. + hardDeadlineCts.Cancel(); + + // The worker may be parked in native serial I/O that no token can interrupt, so + // it is ABANDONED, not awaited — waiting for it is the hang being bounded here. + // Observe its eventual fault so it cannot resurface as an UnobservedTaskException, + // and dispose the sources only once it is done with them (disposing early would + // turn its pending waits into ObjectDisposedException instead of cancellation). + _ = workerTask.ContinueWith( + t => + { + _ = t.Exception; + linkedCts.Dispose(); + hardDeadlineCts.Dispose(); + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + + // Prefer surfacing caller cancellation over a generic timeout when both raced. + cancellationToken.ThrowIfCancellationRequested(); + + throw new TimeoutException( + $"SD card download of '{fileName}' did not complete within " + + $"{hardDeadline.TotalSeconds:0.#}s and was abandoned. The device's SD " + + "subsystem is not responding; reconnect (or power-cycle) before retrying."); + } + + // Propagate success or the transfer's own exception unchanged. + await workerTask.ConfigureAwait(false); + } + finally + { + raceCts.Cancel(); + raceCts.Dispose(); + + // The abandon path hands disposal to its continuation instead; dispose here only + // when the worker actually finished (the common, non-hung case). + if (workerTask.IsCompleted) + { + linkedCts.Dispose(); + hardDeadlineCts.Dispose(); + } + } + } + /// /// Checks whether any line in the response contains a SCPI error indicator. /// These errors (e.g., "**ERROR: -200") can occur transiently when the device diff --git a/src/Daqifi.Core/Device/SdCard/SdCardFileReceiver.cs b/src/Daqifi.Core/Device/SdCard/SdCardFileReceiver.cs index 4c0084de..3326c48c 100644 --- a/src/Daqifi.Core/Device/SdCard/SdCardFileReceiver.cs +++ b/src/Daqifi.Core/Device/SdCard/SdCardFileReceiver.cs @@ -99,6 +99,14 @@ public async Task ReceiveAsync( $"Received {totalBytesReceived} bytes before timeout."); } + // A read can complete without ever having observed the token: System.IO.Ports' + // SerialStream ignores the CancellationToken once a read is in flight, so a + // cancellation requested mid-read is only visible here. Checking it every + // iteration is what makes the accepted token actually honored rather than + // merely accepted (#399) — including on the zero-byte path below, which would + // otherwise report a cancelled transfer as a timeout. + token.ThrowIfCancellationRequested(); + if (bytesRead == 0) { // Stream ended without EOF marker — treat as timeout/error From f880a493efb7b39ef7fc526ddd357d7ed54c53c5 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 08:52:19 -0600 Subject: [PATCH 2/4] docs(sdcard): correct the write-timeout rationale and document the new deadline contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-fix WriteTimeout was not unbounded — it inherited the caller's ConnectionRetryOptions.ConnectionTimeout (3-10s by preset, settable to anything). Say that accurately rather than implying an infinite block, and surface the TimeoutException/abandonment contract on ISdCardOperations where consumers actually read it. Co-Authored-By: Claude Opus 5 --- .../Communication/Transport/SerialStreamTransport.cs | 11 +++++++---- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs | 12 +++++++----- src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs | 10 ++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs index 838a8c74..d381044d 100644 --- a/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs +++ b/src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs @@ -29,10 +29,13 @@ public class SerialStreamTransport : IStreamTransport /// /// Blocking-write timeout applied once the port is open. Everything written over this /// transport is a short SCPI command line, so a healthy device drains a write in - /// milliseconds. Leaving the connect timeout in place (and, on a wedged device, blocking - /// for it) is the failure mode issue #399 is about: - /// accepts no , so an unbounded write cannot be cancelled by - /// any caller. Bounding it turns "parked forever" into a fault the caller can observe. + /// milliseconds. Only used to be lowered after open, + /// leaving writes bounded by the caller's + /// for the life of the port — a value chosen for retry/backoff, not for how long a command + /// write may legitimately take, and one a consumer can set arbitrarily high. Since + /// accepts no , + /// nothing else can shorten that wait on a device that has stopped draining its receive + /// buffer (#399). /// private const int OperationalWriteTimeoutMs = 2000; diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index 7105f6c0..ee93352d 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -2102,11 +2102,13 @@ public Task FormatSdCardAsync(CancellationToken cancellationToken = default) /// On a timeout — or a cancellation the parked transfer cannot itself observe — the /// in-flight transfer is abandoned rather than awaited: it may be blocked in native /// serial I/O that no token can interrupt, and waiting for it is the hang this method - /// exists to bound. Two consequences for callers: the abandoned transfer can still write - /// to after this method has thrown, so the stream - /// must not be reused for anything else; and the device is left mid-SD:GET, so - /// reconnecting (or power-cycling, if its SD subsystem is genuinely wedged) is the - /// reliable way to resume normal operation. + /// exists to bound. The abandoned transfer's token is cancelled first, so it unwinds at + /// its next token check — but that check is only reached once whatever it is blocked in + /// returns, which may be never. Two consequences for callers: it can still write to + /// after this method has thrown, so the stream must + /// not be reused for anything else; and the device is left mid-SD:GET with the + /// protobuf consumer stopped, so reconnecting (or power-cycling, if its SD subsystem is + /// genuinely wedged) is the reliable way to resume normal operation. /// public async Task DownloadSdCardFileAsync( string fileName, diff --git a/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs b/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs index 55f97822..1bbcd537 100644 --- a/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs +++ b/src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs @@ -178,6 +178,12 @@ Task CheckSdCardSpaceAsync( /// Metadata about the downloaded file. /// Thrown when the device is not connected or is not using a USB/serial transport. /// Thrown when the filename is null, empty, or contains invalid characters. + /// + /// Thrown when the transfer does not finish within the implementation's download deadline. + /// The deadline is enforced by the download itself, so it holds even when the transfer is + /// parked in a call that cannot observe a cancellation token; the in-flight transfer is + /// then abandoned rather than awaited (#399). + /// Task DownloadSdCardFileAsync( string fileName, Stream destinationStream, @@ -193,6 +199,10 @@ Task DownloadSdCardFileAsync( /// Metadata about the downloaded file, including the local . /// Thrown when the device is not connected or is not using a USB/serial transport. /// Thrown when the filename is null, empty, or contains invalid characters. + /// + /// Thrown when the transfer does not finish within the implementation's download deadline + /// (see the overload above); the temporary file is removed. + /// Task DownloadSdCardFileAsync( string fileName, IProgress? progress = null, From 1acbf5716a25a7d85172d862537160c4d14b79a7 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 09:35:26 -0600 Subject: [PATCH 3/4] fix(sdcard): admit one SD download at a time so abandoned transfers can't stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo review of #401: because a timed-out download is abandoned rather than stopped, nothing stopped a caller from starting another one. Against a device that stays wedged — an "import all" loop over 30 files — each attempt would leave another permanently blocked thread, and worse, put a second reader on a transport stream the abandoned transfer still owns. That is exactly the framing corruption RestartMessageConsumerAfterSwap refuses to risk. A per-device gate now admits one download at a time and is released only when the worker genuinely finishes, however long after the caller gave up. A retry while a transfer is still parked fails fast with InvalidOperationException; once the abandoned worker unwinds, downloads are accepted again. Co-Authored-By: Claude Opus 5 --- .../Device/SdCard/SdCardOperationsTests.cs | 72 +++++++++++++++++++ .../Device/DaqifiStreamingDevice.cs | 70 +++++++++++++++++- 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs index 8f7bbd9b..0ff0a846 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs @@ -1884,6 +1884,78 @@ public async Task DownloadSdCardFileAsync_WhenTransferParksIgnoringItsToken_Call } } + [Fact] + public async Task DownloadSdCardFileAsync_WhileAPreviousTransferIsStillAbandoned_FailsFast() + { + // An abandoned transfer still owns the transport, so a retry must be refused rather + // than start a second reader on the same stream (and stack another blocked thread). + // A consumer looping over a wedged card's files gets one timeout, then cheap failures. + var device = new ParkedDownloadDevice(ParkMode.Asynchronous, TimeSpan.FromMilliseconds(300)); + device.Connect(); + using var firstDestination = new MemoryStream(); + using var secondDestination = new MemoryStream(); + + try + { + await Assert.ThrowsAsync( + () => device.DownloadSdCardFileAsync("data.bin", firstDestination)); + + // The first transfer is still parked, so the gate is still held. + var elapsed = Stopwatch.StartNew(); + var ex = await Assert.ThrowsAsync( + () => device.DownloadSdCardFileAsync("other.bin", secondDestination)); + elapsed.Stop(); + + Assert.Contains("abandoned", ex.Message, StringComparison.OrdinalIgnoreCase); + + // Fail fast, not another full deadline. + Assert.True( + elapsed.Elapsed < TimeSpan.FromMilliseconds(250), + $"Second download took {elapsed.ElapsedMilliseconds}ms — it should be refused immediately."); + } + finally + { + device.Release(); + } + } + + [Fact] + public async Task DownloadSdCardFileAsync_AfterAnAbandonedTransferUnwinds_IsAllowedAgain() + { + // The gate is a quarantine, not a one-shot latch: once the abandoned worker finally + // returns, the transport is free and downloads are accepted again. + var device = new ParkedDownloadDevice(ParkMode.Asynchronous, TimeSpan.FromMilliseconds(300)); + device.Connect(); + using var destination = new MemoryStream(); + + await Assert.ThrowsAsync( + () => device.DownloadSdCardFileAsync("data.bin", destination)); + + // Let the abandoned worker unwind, then wait for the gate to come back. + device.Release(); + + InvalidOperationException? lastRefusal = null; + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline) + { + try + { + // The park device completes its raw capture without ever calling the transfer + // body, so an accepted download reports a zero-byte result rather than throwing. + await device.DownloadSdCardFileAsync("data.bin", destination); + lastRefusal = null; + break; + } + catch (InvalidOperationException ex) + { + lastRefusal = ex; + await Task.Delay(25); + } + } + + Assert.Null(lastRefusal); + } + [Fact] public async Task DownloadSdCardFileAsync_SlowButHealthyTransfer_IsNotAborted() { diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index ee93352d..c5761852 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -72,6 +72,23 @@ public class DaqifiStreamingDevice : DaqifiDevice, IStreamingDevice, INetworkCon private bool _isLoggingToSdCard; private IReadOnlyList _sdCardFiles = Array.Empty(); + /// + /// Admits one SD download at a time. A download that hits its deadline is ABANDONED, not + /// stopped — its worker can still be parked in native I/O holding the transport stream — + /// so the gate is released only when that worker actually finishes, however long that + /// takes. Without it, a caller retrying against a device that stays wedged (an "import + /// all" loop, say) would start a second reader on the same stream, which is the framing + /// corruption already refuses to risk when restarting the + /// protobuf consumer, and would stack another permanently blocked thread each time (#399). + /// + /// + /// Deliberately not disposed: we only ever call and + /// , never , + /// so there is no handle to release — and an abandoned worker may release this long after + /// the device is disposed, which would otherwise fault a continuation nobody observes. + /// + private readonly SemaphoreSlim _sdDownloadGate = new(1, 1); + /// /// Reconstructs host timestamps from the device's rolling 32-bit tick counter during a /// streaming session. Scoped to this device instance, so a single fixed key suffices. @@ -2109,6 +2126,13 @@ public Task FormatSdCardAsync(CancellationToken cancellationToken = default) /// not be reused for anything else; and the device is left mid-SD:GET with the /// protobuf consumer stopped, so reconnecting (or power-cycling, if its SD subsystem is /// genuinely wedged) is the reliable way to resume normal operation. + /// + /// Until an abandoned transfer unwinds it still owns the transport, so a further download + /// on the same device fails fast with rather than + /// putting a second reader on the same stream. A caller looping over many files against a + /// wedged card therefore gets one timeout and then immediate, cheap failures — not a + /// growing pile of blocked threads. + /// /// public async Task DownloadSdCardFileAsync( string fileName, @@ -2298,12 +2322,48 @@ private static TimeSpan HardDeadlineFor(TimeSpan budget) /// The cooperative budget; the hard deadline is of it. /// Used only in the message. /// The caller's token, observed by the race itself and not only by the worker. - private static async Task RunWithHardDeadlineAsync( + /// + /// Thrown when a previous download still owns — it is either + /// genuinely in flight or was abandoned and is still parked on the transport. + /// + private async Task RunWithHardDeadlineAsync( Func operation, TimeSpan budget, string fileName, CancellationToken cancellationToken) { + // Fail fast rather than becoming a second reader on a stream an abandoned transfer + // still holds. Wait(0) never blocks: this either takes the gate or reports the state. + if (!_sdDownloadGate.Wait(0)) + { + throw new InvalidOperationException( + "A previous SD card download is still in flight, or was abandoned after timing out and " + + "is still parked on the transport. Reconnect the device before retrying."); + } + + // Released exactly once, by whichever path is last to be done with the worker: the + // finally below in the normal case, or the abandon-path continuation when the worker + // finally unwinds. Interlocked because a worker that completes right at the deadline + // boundary can reach both. + var gateReleased = 0; + void ReleaseGate() + { + if (Interlocked.Exchange(ref gateReleased, 1) != 0) + { + return; + } + + try + { + _sdDownloadGate.Release(); + } + catch (ObjectDisposedException) + { + // The device was disposed while a transfer was still abandoned. Benign + // teardown, and this can run from a discarded continuation — never throw. + } + } + var hardDeadline = HardDeadlineFor(budget); // hardDeadlineCts runs on its own timer, independent of the Task.Delay race below, so @@ -2358,6 +2418,9 @@ private static async Task RunWithHardDeadlineAsync( _ = t.Exception; linkedCts.Dispose(); hardDeadlineCts.Dispose(); + + // Only now is the transport genuinely free for another download. + ReleaseGate(); }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, @@ -2380,12 +2443,13 @@ private static async Task RunWithHardDeadlineAsync( raceCts.Cancel(); raceCts.Dispose(); - // The abandon path hands disposal to its continuation instead; dispose here only - // when the worker actually finished (the common, non-hung case). + // The abandon path hands disposal and the gate to its continuation instead; do it + // here only when the worker actually finished (the common, non-hung case). if (workerTask.IsCompleted) { linkedCts.Dispose(); hardDeadlineCts.Dispose(); + ReleaseGate(); } } } From b2a8da5b23700800eab2a0dcf758474e6cce56c4 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 29 Jul 2026 09:50:17 -0600 Subject: [PATCH 4/4] fix(sdcard): let cancellation outrank the download gate, and de-flake the fast-fail test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo review round 2 of #401. Cancellation vs gate: a caller who cancelled was told "a previous download is still in flight" — an answer about someone else's transfer, and a contract violation, since the method documents OperationCanceledException. The token is now checked before the gate is taken and again before the refusal is thrown, matching the precedence the abandon path already applies. The regression test cancels from inside the pre-flight StopStreaming send, which lands in the exact window between the entry guard and the gate check. Fast-fail test: dropped the 250ms wall-clock assertion, which could fail on a contended runner even when correct. The exception TYPE is the real proof — waiting out another deadline yields TimeoutException, never InvalidOperationException — so the timing check is now only a loose hang guard. Co-Authored-By: Claude Opus 5 --- .../Device/SdCard/SdCardOperationsTests.cs | 61 +++++++++++++++---- .../Device/DaqifiStreamingDevice.cs | 9 +++ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs index 0ff0a846..1f3d6234 100644 --- a/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs +++ b/src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs @@ -1900,18 +1900,47 @@ public async Task DownloadSdCardFileAsync_WhileAPreviousTransferIsStillAbandoned await Assert.ThrowsAsync( () => device.DownloadSdCardFileAsync("data.bin", firstDestination)); - // The first transfer is still parked, so the gate is still held. - var elapsed = Stopwatch.StartNew(); - var ex = await Assert.ThrowsAsync( - () => device.DownloadSdCardFileAsync("other.bin", secondDestination)); - elapsed.Stop(); - + // The first transfer is still parked, so the gate is still held. The exception TYPE + // is what proves the refusal came from the gate: waiting out another deadline would + // have produced TimeoutException, never InvalidOperationException. The wall-clock + // bound is only a hang guard, kept loose so a contended CI runner can't fail it. + var secondCall = device.DownloadSdCardFileAsync("other.bin", secondDestination); + var winner = await Task.WhenAny(secondCall, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.Same((Task)secondCall, winner); + + var ex = await Assert.ThrowsAsync(() => secondCall); Assert.Contains("abandoned", ex.Message, StringComparison.OrdinalIgnoreCase); + } + finally + { + device.Release(); + } + } - // Fail fast, not another full deadline. - Assert.True( - elapsed.Elapsed < TimeSpan.FromMilliseconds(250), - $"Second download took {elapsed.ElapsedMilliseconds}ms — it should be refused immediately."); + [Fact] + public async Task DownloadSdCardFileAsync_WhenGateIsHeldAndCallerCancelled_ReportsCancellation() + { + // Cancellation outranks the gate: a caller who cancelled should get their own + // cancellation back, not a report about a different transfer they can do nothing about. + var device = new ParkedDownloadDevice(ParkMode.Asynchronous, TimeSpan.FromMilliseconds(300)); + device.Connect(); + using var firstDestination = new MemoryStream(); + using var secondDestination = new MemoryStream(); + + try + { + await Assert.ThrowsAsync( + () => device.DownloadSdCardFileAsync("data.bin", firstDestination)); + + // Cancel from inside the pre-flight StopStreaming send: that lands in the exact + // window this guards — after DownloadSdCardFileAsync's entry check, before the + // gate is examined. A token cancelled before the call would just trip the entry + // check and prove nothing. + using var cts = new CancellationTokenSource(); + device.CancelOnNextSend = cts; + + await Assert.ThrowsAnyAsync( + () => device.DownloadSdCardFileAsync("other.bin", secondDestination, null, cts.Token)); } finally { @@ -2321,13 +2350,23 @@ public ParkedDownloadDevice(ParkMode mode, TimeSpan budget) /// Set once the transfer has actually reached the park. public ManualResetEventSlim Parked { get; } = new(false); + /// + /// When set, the next cancels it. Lets a test cancel from inside + /// the download's pre-flight command, i.e. between its entry guard and the SD-download + /// gate check. + /// + public CancellationTokenSource? CancelOnNextSend { get; set; } + public override bool IsUsbConnection => true; internal override TimeSpan SdCardDownloadTimeout => _budget; public override void Send(IOutboundMessage message) { - // Swallowed: this device never gets as far as exchanging commands. + // Otherwise swallowed: this device never gets as far as exchanging commands. + var cancelSource = CancelOnNextSend; + CancelOnNextSend = null; + cancelSource?.Cancel(); } protected override async Task ExecuteRawCaptureAsync( diff --git a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs index c5761852..ed4c8b2d 100644 --- a/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs +++ b/src/Daqifi.Core/Device/DaqifiStreamingDevice.cs @@ -2332,10 +2332,19 @@ private async Task RunWithHardDeadlineAsync( string fileName, CancellationToken cancellationToken) { + // Checked before taking the gate so a cancelled caller neither acquires it nor gets an + // answer about some other transfer. + cancellationToken.ThrowIfCancellationRequested(); + // Fail fast rather than becoming a second reader on a stream an abandoned transfer // still holds. Wait(0) never blocks: this either takes the gate or reports the state. if (!_sdDownloadGate.Wait(0)) { + // Cancellation wins when it raced the gate check — the same precedence the abandon + // path below applies. The caller asked to stop; that is a truer answer than a + // report about a different download. + cancellationToken.ThrowIfCancellationRequested(); + throw new InvalidOperationException( "A previous SD card download is still in flight, or was abandoned after timing out and " + "is still parked on the transport. Reconnect the device before retrying.");