fix(r2-01): cancellation observed mid-PDU marks the connection dead before propagating (STAB-15a, task 8)

This commit is contained in:
Joseph Doherty
2026-07-13 10:01:20 -04:00
parent dab0029add
commit 33044061c3
3 changed files with 78 additions and 3 deletions
@@ -52,7 +52,7 @@
{ {
"id": 8, "id": 8,
"subject": "RED->GREEN: cancellation-during-I/O marks handle dead (T6 read + T7 write; per-item OCE catches set _plcDead before propagating)", "subject": "RED->GREEN: cancellation-during-I/O marks handle dead (T6 read + T7 write; per-item OCE catches set _plcDead before propagating)",
"status": "pending", "status": "completed",
"blockedBy": [7] "blockedBy": [7]
}, },
{ {
@@ -521,6 +521,17 @@ public sealed class S7Driver
results[i] = new DataValueSnapshot(value, 0u, now, now); results[i] = new DataValueSnapshot(value, 0u, now, now);
_health = new DriverHealth(DriverState.Healthy, now, null); _health = new DriverHealth(DriverState.Healthy, now, null);
} }
catch (OperationCanceledException)
{
// Cancellation observed mid-PDU abandons a half-read ISO-on-TCP response on the
// single gated stream — the handle must be reopened, not reused (STAB-15a). Mark
// dead (we hold _gate) and propagate: the caller cancelled, nobody consumes the
// batch. Marking is unconditional on OCE (not routed through the type classifier):
// whether bytes moved is unknowable, and a false positive costs one cheap reopen
// versus permanent desync for a false negative.
_plcDead = true;
throw;
}
catch (NotSupportedException) catch (NotSupportedException)
{ {
results[i] = new DataValueSnapshot(null, StatusBadNotSupported, null, now); results[i] = new DataValueSnapshot(null, StatusBadNotSupported, null, now);
@@ -1057,8 +1068,12 @@ public sealed class S7Driver
} }
catch (OperationCanceledException) catch (OperationCanceledException)
{ {
// Let cancellation propagate rather than turning it into // Cancellation observed mid-PDU abandons a half-written ISO-on-TCP request on
// a status code — the gate is still held so Release() runs in finally. // the single gated stream — the handle must be reopened, not reused (STAB-15a).
// Mark dead (we hold _gate) and propagate: the caller cancelled, nobody consumes
// the batch, and Release() still runs in finally. Marking is unconditional on
// OCE — a false positive costs one cheap reopen versus permanent desync.
_plcDead = true;
throw; throw;
} }
catch (NotSupportedException) catch (NotSupportedException)
@@ -291,6 +291,66 @@ public sealed class S7DriverReconnectTests
factory.Created[0].Disposed.ShouldBeTrue(); factory.Created[0].Disposed.ShouldBeTrue();
} }
// ---- STAB-15a: cancellation observed mid-I/O must mark the handle dead ----
/// <summary>
/// T6 (STAB-15a). Cancellation observed WHILE a read is in flight abandons a half-read
/// ISO-on-TCP response on the single gated stream — the handle can't be reused. The OCE
/// must propagate (caller cancelled; nobody consumes the batch) AND mark the handle dead so
/// the next read reopens. Before the fix the read path converted the OCE to a Bad snapshot
/// and kept the desynced handle.
/// </summary>
[Fact]
public async Task Cancelled_read_marks_handle_dead_and_next_read_reopens()
{
var factory = new FakeS7PlcFactory();
using var drv = new S7Driver(Options(), "s7-cancel-read", factory);
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
factory.Created[0].ReadHangsUntilCancelled = true;
using var cts = new CancellationTokenSource();
cts.CancelAfter(50);
await Should.ThrowAsync<OperationCanceledException>(
async () => await drv.ReadAsync(["W0"], cts.Token));
// Fresh-token read reopens a fresh connection.
var recovered = await drv.ReadAsync(["W0"], TestContext.Current.CancellationToken);
recovered[0].StatusCode.ShouldBe(0u);
factory.Created.Count.ShouldBe(2);
factory.Created[0].Disposed.ShouldBeTrue();
}
/// <summary>
/// T7 (STAB-15a). Cancellation observed while a WRITE is in flight likewise marks the
/// handle dead before propagating (the OCE already propagated pre-fix; the reopen is what
/// was missing).
/// </summary>
[Fact]
public async Task Cancelled_write_marks_handle_dead_and_next_read_reopens()
{
var opts = new S7DriverOptions
{
Host = "192.0.2.1",
Timeout = TimeSpan.FromMilliseconds(250),
Probe = new S7ProbeOptions { Enabled = false },
Tags = [new S7TagDefinition("WW0", "DB1.DBW0", S7DataType.UInt16, Writable: true)],
};
var factory = new FakeS7PlcFactory();
using var drv = new S7Driver(opts, "s7-cancel-write", factory);
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
factory.Created[0].WriteHangsUntilCancelled = true;
using var cts = new CancellationTokenSource();
cts.CancelAfter(50);
await Should.ThrowAsync<OperationCanceledException>(
async () => await drv.WriteAsync([new WriteRequest("WW0", (ushort)1)], cts.Token));
var recovered = await drv.ReadAsync(["WW0"], TestContext.Current.CancellationToken);
recovered[0].StatusCode.ShouldBe(0u);
factory.Created.Count.ShouldBe(2);
factory.Created[0].Disposed.ShouldBeTrue();
}
// ---- fakes ---- // ---- fakes ----
private sealed class FakeS7PlcFactory : IS7PlcFactory private sealed class FakeS7PlcFactory : IS7PlcFactory