fix(drivers): InitializeAsync releases previous device/session state before overwrite (#630)

FOCAS, AbLegacy, AbCip, TwinCAT, S7 and OpcUaClient orphaned the previous probe/recycle/fixed-tree loops and clients when re-initialised on a live driver. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-08 14:28:55 -04:00
parent d898262c4a
commit 461133235a
13 changed files with 494 additions and 36 deletions
@@ -0,0 +1,54 @@
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
+
+/// <summary>
+///     #630 (same pattern) — <c>InitializeAsync</c> on a live driver overwrote <c>_devices[host]</c> and
+///     orphaned the previous state's probe loop + handles. It now releases them first.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class AbCipReinitializeReleasesStateTests
+{
+    /// <summary>Initialize twice: the first state's probe loop has stopped and its probe handle is disposed.</summary>
+    [Fact]
+    public async Task SecondInitialize_StopsFirstStatesLoop_AndDisposesItsHandles()
+    {
+        const string host = "ab://10.0.0.5/1,0";
+        var factory = new FakeAbCipTagFactory();
+        var drv = new AbCipDriver(new AbCipDriverOptions
+        {
+            Devices = [new AbCipDeviceOptions(host)],
+            Probe = new AbCipProbeOptions
+            {
+                Enabled = true,
+                Interval = TimeSpan.FromMilliseconds(20),
+                Timeout = TimeSpan.FromMilliseconds(200),
+                ProbeTagPath = "@raw_cpu_type",
+            },
+        }, "drv-reinit", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var first = drv.GetDeviceState(host)!;
+        // Positive control: the first probe loop is running and has initialised its runtime.
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!(factory.Tags.TryGetValue("@raw_cpu_type", out var t) && t.InitializeCount > 0))
+        {
+            if (DateTime.UtcNow > deadline) throw new TimeoutException("probe never started");
+            await Task.Delay(10);
+        }
+        var firstTag = factory.Tags["@raw_cpu_type"];
+        var firstLoop = first.ProbeTask;
+        firstLoop.ShouldNotBeNull();
+        firstLoop!.IsCompleted.ShouldBeFalse();
+        firstTag.Disposed.ShouldBeFalse();
+
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        firstLoop.IsCompleted.ShouldBeTrue(); // joined
+        firstTag.Disposed.ShouldBeTrue();
+        drv.GetDeviceState(host).ShouldNotBeSameAs(first);
+
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+}
@@ -0,0 +1,50 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests;
+
+/// <summary>
+///     #630 — <c>InitializeAsync</c> on a live driver (<c>DriverInstanceActor.AdoptConfigDuringInit</c> runs
+///     it with no teardown first) overwrote <c>_devices[host]</c> and orphaned the previous state's
+///     probe loop + runtime. It now releases them first.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class AbLegacyReinitializeReleasesStateTests
+{
+    /// <summary>Initialize twice: the first state's probe loop has stopped and its probe runtime is disposed.</summary>
+    [Fact]
+    public async Task SecondInitialize_StopsFirstStatesLoop_AndDisposesItsRuntime()
+    {
+        const string host = "ab://10.0.0.5/1,0";
+        var factory = new FakeAbLegacyTagFactory();
+        var drv = new AbLegacyDriver(new AbLegacyDriverOptions
+        {
+            Devices = [new AbLegacyDeviceOptions(host, AbLegacyPlcFamily.Slc500)],
+            Probe = new AbLegacyProbeOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(20), ProbeAddress = "S:0" },
+        }, "drv-reinit", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var first = drv.GetDeviceState(host)!;
+        // Positive control: the first state's probe loop is running and has created + initialised its runtime.
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!(factory.Tags.TryGetValue("S:0", out var t) && t.InitializeCount > 0))
+        {
+            if (DateTime.UtcNow > deadline) throw new TimeoutException("probe never started");
+            await Task.Delay(10);
+        }
+        var firstTag = factory.Tags["S:0"];
+        var firstLoop = first.ProbeTask;
+        firstLoop.ShouldNotBeNull();
+        firstLoop!.IsCompleted.ShouldBeFalse();
+        firstTag.Disposed.ShouldBeFalse();
+
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        firstLoop.IsCompleted.ShouldBeTrue(); // joined
+        firstTag.Disposed.ShouldBeTrue();
+        drv.GetDeviceState(host).ShouldNotBeSameAs(first);
+
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+}
@@ -0,0 +1,58 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests;
+
+/// <summary>
+///     #630 — <c>InitializeAsync</c> on a live driver (<c>DriverInstanceActor.AdoptConfigDuringInit</c> runs
+///     it with no teardown first) overwrote <c>_devices[host]</c> and orphaned the previous state's
+///     probe loop + client. It now releases them first.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class FocasReinitializeReleasesStateTests
+{
+    /// <summary>Initialize twice: the first state's probe loop has finished and its client is disposed.</summary>
+    [Fact]
+    public async Task SecondInitialize_StopsFirstStatesLoop_AndDisposesItsClient()
+    {
+        const string host = "focas://10.0.0.5:8193";
+        var factory = new FakeFocasClientFactory();
+        var options = new FocasDriverOptions
+        {
+            Devices = [new FocasDeviceOptions(host)],
+            RawTags = [FocasRawTags.Entry(new FocasTagDefinition("R", host, "R100", FocasDataType.Byte))],
+            Probe = new FocasProbeOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(20), Timeout = TimeSpan.FromSeconds(2) },
+        };
+        var drv = new FocasDriver(options, "focas-reinit", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var first = drv.GetDeviceState(host)!;
+        // Positive control: the first state's probe loop is demonstrably running and holds a live client.
+        await WaitUntilAsync(() => factory.Clients.Count > 0 && factory.Clients[0].IsConnected);
+        var firstLoop = first.ProbeTask;
+        firstLoop.ShouldNotBeNull();
+        firstLoop!.IsCompleted.ShouldBeFalse();
+        var firstClient = factory.Clients[0];
+
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        firstLoop.IsCompleted.ShouldBeTrue(); // joined
+        firstClient.DisposeCount.ShouldBeGreaterThanOrEqualTo(1);
+        firstClient.IsConnected.ShouldBeFalse();
+        drv.GetDeviceState(host).ShouldNotBeSameAs(first);
+        drv.GetDeviceState(host)!.ProbeTask.ShouldNotBeNull(); // the new state is running
+
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    private static async Task WaitUntilAsync(Func<bool> cond)
+    {
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!cond())
+        {
+            if (DateTime.UtcNow > deadline) throw new TimeoutException("condition not met");
+            await Task.Delay(10);
+        }
+    }
+}
@@ -0,0 +1,30 @@
+using Moq;
+using Opc.Ua.Client;
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
+
+/// <summary>
+///     #630 (same pattern) — <c>InitializeAsync</c> on a live driver (no <c>ShutdownAsync</c> first) used to
+///     overwrite <c>Session</c> and orphan the previous session + its keep-alive handler.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class OpcUaClientReinitializeReleasesSessionTests
+{
+    /// <summary>A second Initialize disposes the live session before attempting to build a new one.</summary>
+    [Fact]
+    public async Task Initialize_OnLiveDriver_DisposesThePreviousSession()
+    {
+        var options = new OpcUaClientDriverOptions { EndpointUrl = "opc.tcp://127.0.0.1:1" };
+        using var drv = new OpcUaClientDriver(options, "opcua-reinit");
+        var old = new Mock<ISession>();
+        drv.SetSessionForTest(old.Object);
+
+        // The new connect is refused (nothing listens on port 1) — what matters is what happened to the old one.
+        await Should.ThrowAsync<Exception>(() => drv.InitializeAsync("{}", TestContext.Current.CancellationToken));
+
+        old.Verify(s => s.Dispose(), Times.AtLeastOnce);
+        drv.Session.ShouldBeNull();
+    }
+}
@@ -482,6 +482,37 @@ public sealed class S7DriverReconnectTests
        (factory.Created.Count - atFirstAttempt).ShouldBeLessThanOrEqualTo(1);
    }

+    /// <summary>
+    ///     #630 — a second <c>InitializeAsync</c> on a live driver (no <c>ShutdownAsync</c> between, as
+    ///     <c>DriverInstanceActor.AdoptConfigDuringInit</c> does) must stop the first probe loop and
+    ///     dispose the first connection rather than orphan them.
+    /// </summary>
+    [Fact]
+    public async Task Second_Initialize_stops_first_probe_loop_and_disposes_first_connection()
+    {
+        var factory = new FakeS7PlcFactory();
+        using var drv = new S7Driver(ProbeEnabledOptions(), "s7-reinit", factory);
+        await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
+        var first = factory.Created[0];
+
+        // Positive control: the first probe loop is demonstrably running on the first connection.
+        await WaitUntilAsync(() => Volatile.Read(ref first.ReadStatusCount) >= 2, TimeSpan.FromSeconds(5));
+        Volatile.Read(ref first.ReadStatusCount).ShouldBeGreaterThanOrEqualTo(2);
+        first.Disposed.ShouldBeFalse();
+
+        await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
+
+        first.Disposed.ShouldBeTrue();
+        var frozen = Volatile.Read(ref first.ReadStatusCount);
+        var second = factory.Created[^1];
+        second.ShouldNotBeSameAs(first);
+        // Positive control for the absence check: the NEW loop keeps ticking over the same window.
+        var secondBefore = Volatile.Read(ref second.ReadStatusCount);
+        await WaitUntilAsync(() => Volatile.Read(ref second.ReadStatusCount) >= secondBefore + 3, TimeSpan.FromSeconds(5));
+        Volatile.Read(ref second.ReadStatusCount).ShouldBeGreaterThanOrEqualTo(secondBefore + 3);
+        Volatile.Read(ref first.ReadStatusCount).ShouldBe(frozen);
+    }
+
    private static S7DriverOptions ProbeEnabledOptions() => new()
    {
        Host = "192.0.2.1",
@@ -626,8 +657,12 @@ public sealed class S7DriverReconnectTests
        public Task WriteBytesAsync(S7NetDataType area, int db, int startByteAdr, byte[] value, CancellationToken cancellationToken) =>
            Task.CompletedTask;

+        /// <summary>Number of probe <see cref="ReadStatusAsync"/> calls made on this connection.</summary>
+        public int ReadStatusCount;
+
        public async Task ReadStatusAsync(CancellationToken cancellationToken)
        {
+            Interlocked.Increment(ref ReadStatusCount);
            if (ReadStatusHangsUntilCancelled)
                await Task.Delay(System.Threading.Timeout.Infinite, cancellationToken).ConfigureAwait(false);
            if (ReadStatusThrows is { } ex)
@@ -0,0 +1,48 @@
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
+
+/// <summary>
+///     #630 (same pattern) — <c>InitializeAsync</c> on a live driver overwrote <c>_devices[host]</c> and
+///     orphaned the previous state's probe loop + client. It now releases them first.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class TwinCATReinitializeReleasesStateTests
+{
+    /// <summary>Initialize twice: the first state's probe loop has finished and its client is disposed.</summary>
+    [Fact]
+    public async Task SecondInitialize_StopsFirstStatesLoop_AndDisposesItsClient()
+    {
+        const string host = "ads://5.23.91.23.1.1:851";
+        var factory = new FakeTwinCATClientFactory();
+        var drv = new TwinCATDriver(new TwinCATDriverOptions
+        {
+            Devices = [new TwinCATDeviceOptions(host)],
+            Probe = new TwinCATProbeOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(20), Timeout = TimeSpan.FromSeconds(1) },
+        }, "drv-reinit", factory);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var first = drv.GetDeviceState(host)!;
+        // Positive control: the first probe loop is running and holds a connected client.
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!(factory.Clients.Count > 0 && factory.Clients[0].IsConnected))
+        {
+            if (DateTime.UtcNow > deadline) throw new TimeoutException("probe never connected");
+            await Task.Delay(10);
+        }
+        var firstClient = factory.Clients[0];
+        var firstLoop = first.ProbeTask;
+        firstLoop.ShouldNotBeNull();
+        firstLoop!.IsCompleted.ShouldBeFalse();
+
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        firstLoop.IsCompleted.ShouldBeTrue(); // joined
+        firstClient.DisposeCount.ShouldBeGreaterThanOrEqualTo(1);
+        firstClient.IsConnected.ShouldBeFalse();
+        drv.GetDeviceState(host).ShouldNotBeSameAs(first);
+
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+}