From 54a00fd760bac2a0596e300f1217f4a988630853 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 21:28:00 -0400 Subject: [PATCH] fix(focas): a caller's own cancellation during connect does not open the backoff window; deterministic concurrent-connect test Review follow-up on c940349f: a cancelled caller recorded a connect failure, throttling every caller queued on ConnectGate for ~1 s. The 200 ms settle delay in the concurrent-connect test is replaced by a TaskCompletionSource the fake connect signals on entry. Both tests verified red with their fix reverted. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../FocasDriver.cs | 7 ++- .../FocasConcurrentConnectTests.cs | 48 +++++++++++++++++-- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index f42322bf..673b271f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -1855,12 +1855,15 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, { await client.ConnectAsync(device.ParsedAddress, _options.Timeout, ct).ConfigureAwait(false); } - catch + catch (Exception ex) { // Dispose only OUR client — never one another path installed meanwhile (recycle/teardown). client.Dispose(); if (ReferenceEquals(device.Client, client)) device.Client = null; - device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window + // The caller's OWN cancellation says nothing about the CNC: do not open the backoff window, or + // every caller queued on ConnectGate behind it would be throttled for ~1 s for nothing. + if (!(ex is OperationCanceledException && ct.IsCancellationRequested)) + device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window throw; } device.Backoff.RecordSuccess(); // 05/STAB-8: recovery resets the window immediately diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConcurrentConnectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConcurrentConnectTests.cs index 0c92bc6a..a92b195c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConcurrentConnectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConcurrentConnectTests.cs @@ -19,12 +19,13 @@ public sealed class FocasConcurrentConnectTests { /// A fake whose connect parks until released, and which — like the real wire client — refuses to /// finish connecting once disposed. - private sealed class GatedConnectClient(Task release) : FakeFocasClient + private sealed class GatedConnectClient(Task release, TaskCompletionSource? entered = null) : FakeFocasClient { private bool _disposed; public override async Task ConnectAsync(FocasHostAddress address, TimeSpan timeout, CancellationToken ct) { + entered?.TrySetResult(); await release.WaitAsync(ct); if (_disposed) throw new ObjectDisposedException("FocasWireClient"); await base.ConnectAsync(address, timeout, ct); @@ -42,7 +43,8 @@ public sealed class FocasConcurrentConnectTests public async Task Concurrent_first_reads_open_one_session_and_both_succeed() { var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var factory = new FakeFocasClientFactory { Customise = () => new GatedConnectClient(release.Task) }; + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var factory = new FakeFocasClientFactory { Customise = () => new GatedConnectClient(release.Task, entered) }; await using var drv = new FocasDriver(new FocasDriverOptions { Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], @@ -55,9 +57,11 @@ public sealed class FocasConcurrentConnectTests await drv.InitializeAsync("{}", CancellationToken.None); var first = drv.ReadAsync(["A"], CancellationToken.None); + // Deterministic: the first caller is now parked INSIDE its connect (client created, not yet connected) — + // the exact window the race needs. The second caller then runs synchronously up to its first real await: + // pre-fix that is "dispose the first client, Create() a second"; with the gate it parks on ConnectGate. + await entered.Task.WaitAsync(TimeSpan.FromSeconds(5)); var second = drv.ReadAsync(["B"], CancellationToken.None); - // Give both callers time to reach the connect path before the (single) connect is allowed to finish. - await Task.Delay(200); release.SetResult(); var results = await Task.WhenAll(first, second); @@ -89,4 +93,40 @@ public sealed class FocasConcurrentConnectTests var read = drv.ReadAsync(["A"], CancellationToken.None); (await read.WaitAsync(TimeSpan.FromSeconds(5))).Single().StatusCode.ShouldBe(FocasStatusMapper.Good); } + + /// + /// A caller cancelled during its connect must not open the backoff window: the cancellation is the caller's, + /// not the CNC's, and a recorded failure would throttle every caller queued behind it on the gate. + /// + [Fact] + public async Task A_caller_cancelled_during_connect_does_not_throttle_the_next_caller() + { + var never = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var factory = new FakeFocasClientFactory(); + var created = 0; + // First client parks in connect until cancelled; every later client connects immediately. + factory.Customise = () => Interlocked.Increment(ref created) == 1 + ? new GatedConnectClient(never.Task, entered) + : new FakeFocasClient(); + await using var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + RawTags = FocasRawTags.Entries([new FocasTagDefinition("A", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)]), + Probe = new FocasProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + using var cts = new CancellationTokenSource(); + var cancelled = drv.ReadAsync(["A"], cts.Token); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(5)); + cts.Cancel(); + try { await cancelled; } catch (OperationCanceledException) { /* either surfaced or mapped — both fine */ } + + // Immediately — well inside the 1 s initial backoff window a recorded failure would have opened. + var next = await drv.ReadAsync(["A"], CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5)); + next.Single().StatusCode.ShouldBe(FocasStatusMapper.Good, "a caller's own cancellation must not throttle the next connect"); + factory.Clients.Count.ShouldBe(2); + } }