diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json index 587d853a..4ffe46a8 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json @@ -208,7 +208,7 @@ { "id": "B6.4", "subject": "B6: post-SwapHandle ownership re-check retracts the fresh ADS notification when unsubscribe raced the replay, tests FAIL->PASS \u2014 STAB-17", - "status": "pending", + "status": "completed", "blockedBy": [ "B6.3" ] diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index bfe45438..fdb77852 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -878,6 +878,15 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery _options.NotificationMaxDelayMs, reg.OnChange, ct).ConfigureAwait(false); var old = reg.SwapHandle(newHandle); try { old?.Dispose(); } catch { /* dead handle from the disposed client */ } + // 05/STAB-17: an UnsubscribeAsync (gate-free) can race this replay — if it removed the + // registration between the intents snapshot and this swap, the fresh handle is + // ownerless. Retract it immediately so the ADS notification isn't leaked live. + if (!device.NativeRegistrations.ContainsKey(reg.Id)) + { + var orphan = reg.MarkHandleDead(); + try { orphan?.Dispose(); } catch { /* best-effort */ } + continue; + } replayed++; } catch (Exception ex) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs index 40c55cd9..ff5c6a13 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs @@ -118,6 +118,9 @@ internal class FakeTwinCATClient : ITwinCATClient public bool ThrowOnAddNotification { get; set; } /// Records the most recently-supplied maxDelayMs for Driver.TwinCAT-014 tests. public int LastMaxDelayMs { get; private set; } + /// When set, awaits this gate before creating the handle — + /// lets a test wedge a replay/register mid-flight (STAB-17 orphan race). + public TaskCompletionSource? AddNotificationGate { get; set; } /// Simulates adding a notification for value changes. /// The path to the symbol to watch. @@ -128,17 +131,20 @@ internal class FakeTwinCATClient : ITwinCATClient /// The callback to invoke on value change. /// The cancellation token. /// A task that returns a notification handle. - public virtual Task AddNotificationAsync( + public virtual async Task AddNotificationAsync( string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime, int maxDelayMs, Action onChange, CancellationToken cancellationToken) { if (ThrowOnAddNotification) throw Exception ?? new InvalidOperationException("fake AddNotification failure"); + if (AddNotificationGate is { } gate) + await gate.Task.ConfigureAwait(false); + LastMaxDelayMs = maxDelayMs; var reg = new FakeNotification(symbolPath, type, bitIndex, onChange, this); Notifications.Add(reg); - return Task.FromResult(reg); + return reg; } /// Fire a change event through the registered callback for . diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs index 30b6f4f7..0d3f2c4f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs @@ -280,6 +280,54 @@ public sealed class TwinCATReconnectReplayTests await drv.ShutdownAsync(TestContext.Current.CancellationToken); } + /// + /// STAB-17: unsubscribing WHILE a replay is mid-AddNotificationAsync must not leak a + /// live ADS notification. After the fresh handle is installed, an ownership re-check finds the + /// registration was unsubscribed and retracts (disposes) the fresh handle — no orphan. + /// + [Fact] + public async Task UnsubscribeDuringReplay_DisposesFreshHandle_NoOrphan() + { + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var build = 0; + var factory = new FakeTwinCATClientFactory + { + Customise = () => + { + var c = new FakeTwinCATClient(); + if (++build == 2) c.AddNotificationGate = gate; // client2's replay blocks on the gate + return c; + }, + }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + Tags = [new TwinCATTagDefinition("X", Host, "MAIN.X", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = true, + }, "drv-1", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var handle = await drv.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); + + // Drop the wire; a read reconnects onto client2, whose replay AddNotification blocks on the gate. + factory.Clients[0].SimulateWireDrop(); + var readTask = Task.Run(() => drv.ReadAsync(["X"], CancellationToken.None)); + await WaitForAsync(() => factory.Clients.Count >= 2, TimeSpan.FromSeconds(2)); + await Task.Delay(50); // let the replay reach the blocked AddNotification + + // Unsubscribe mid-replay (gate-free) — removes the registration from the device registry. + await drv.UnsubscribeAsync(handle, TestContext.Current.CancellationToken); + + // Release the gate: AddNotification completes, SwapHandle installs the fresh handle, and the + // ownership re-check must retract it (no owner) rather than leave a live orphan notification. + gate.SetResult(); + await readTask; + + await WaitForAsync(() => factory.Clients[1].Notifications.Count == 0, TimeSpan.FromSeconds(2)); + factory.Clients[1].Notifications.ShouldBeEmpty("the fresh handle must be disposed — no orphan ADS notification"); + } + private static async Task WaitForAsync(Func condition, TimeSpan timeout) { var deadline = DateTime.UtcNow + timeout;