fix(twincat): retract the fresh ADS notification when unsubscribe raced the replay (05/STAB-17)

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"
       ]

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)

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs

@@ -118,6 +118,9 @@ internal class FakeTwinCATClient : ITwinCATClient
     public bool ThrowOnAddNotification { get; set; }
     /// <summary>Records the most recently-supplied <c>maxDelayMs</c> for Driver.TwinCAT-014 tests.</summary>
     public int LastMaxDelayMs { get; private set; }
+    /// <summary>When set, <see cref="AddNotificationAsync"/> awaits this gate before creating the handle —
+    ///     lets a test wedge a replay/register mid-flight (STAB-17 orphan race).</summary>
+    public TaskCompletionSource? AddNotificationGate { get; set; }
 
     /// <summary>Simulates adding a notification for value changes.</summary>
     /// <param name="symbolPath">The path to the symbol to watch.</param>
@@ -128,17 +131,20 @@ internal class FakeTwinCATClient : ITwinCATClient
     /// <param name="onChange">The callback to invoke on value change.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A task that returns a notification handle.</returns>
-    public virtual Task<ITwinCATNotificationHandle> AddNotificationAsync(
+    public virtual async Task<ITwinCATNotificationHandle> AddNotificationAsync(
         string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime,
         int maxDelayMs, Action<string, object?> 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<ITwinCATNotificationHandle>(reg);
+        return reg;
     }
 
     /// <summary>Fire a change event through the registered callback for <paramref name="symbolPath"/>.</summary>

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);
     }
 
+    /// <summary>
+    ///     STAB-17: unsubscribing WHILE a replay is mid-<c>AddNotificationAsync</c> 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.
+    /// </summary>
+    [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<bool> condition, TimeSpan timeout)
     {
         var deadline = DateTime.UtcNow + timeout;