From 11eb5157e68f97750da7f073f2a926a1bb92ed4d Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Wed, 8 Jul 2026 23:16:27 -0400
Subject: [PATCH] =?UTF-8?q?fix(dcl):=20dispose+detach=20previous=20OPC=20U?=
 =?UTF-8?q?A=20client=20on=20reconnect=20(S1)=20=E2=80=94=20stops=20sessio?=
 =?UTF-8?q?n=20leak=20and=20stale=20ConnectionLost=20flapping?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../Adapters/OpcUaDataConnection.cs           | 18 +++++++
 .../OpcUaDataConnectionTests.cs               | 52 +++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs
index 009636d2..10344eed 100644
--- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs
@@ -96,6 +96,24 @@ public class OpcUaDataConnection : IDataConnection, IBrowsableDataConnection, IA
 
         _status = ConnectionHealth.Connecting;
 
+        // Reconnect reuses this adapter instance (DataConnectionActor re-calls
+        // ConnectAsync on auto-reconnect): detach + dispose the previous client so
+        // (a) its session/keep-alive/socket do not leak per flap and (b) its late
+        // ConnectionLost cannot flap the NEW healthy session after _disconnectFired
+        // is reset below. Also stop the previous heartbeat monitor so the old
+        // StaleTagMonitor cannot fire against the new session. Dispose is best-effort
+        // fire-and-forget (CloseAsync against a dead server can block for the
+        // operation timeout); faults are logged, never thrown into the connect path.
+        StopHeartbeatMonitor();
+        if (_client is { } previousClient)
+        {
+            previousClient.ConnectionLost -= OnClientConnectionLost;
+            _ = previousClient.DisposeAsync().AsTask().ContinueWith(
+                t => _logger.LogWarning(t.Exception?.GetBaseException(),
+                    "Disposing previous OPC UA client failed for {Endpoint}", _endpointUrl),
+                TaskContinuationOptions.OnlyOnFaulted);
+        }
+
         _client = _clientFactory.Create();
         _client.ConnectionLost += OnClientConnectionLost;
         await _client.ConnectAsync(_endpointUrl, options, cancellationToken);
diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaDataConnectionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaDataConnectionTests.cs
index 4e228698..cec163cb 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaDataConnectionTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaDataConnectionTests.cs
@@ -559,4 +559,56 @@ public class OpcUaDataConnectionTests
             Arg.Is<OpcUaConnectionOptions?>(o => o != null && o.AutoAcceptUntrustedCerts == false),
             Arg.Any<CancellationToken>());
     }
+
+    // DataConnectionLayer-S1: the adapter instance is reused across reconnects
+    // (DataConnectionActor calls ConnectAsync again on auto-reconnect). Before the
+    // fix, ConnectAsync overwrote _client without disposing/detaching the previous
+    // one, leaking its session/keep-alive/socket per flap AND leaving its stale
+    // ConnectionLost handler wired — so a late keep-alive death on the OLD client
+    // flapped the NEW healthy session (after _disconnectFired was reset).
+    [Fact]
+    public async Task Reconnect_DisposesAndDetachesPreviousClient()
+    {
+        var clients = new List<IOpcUaClient>();
+        _mockFactory.Create().Returns(_ =>
+        {
+            var c = Substitute.For<IOpcUaClient>();
+            clients.Add(c);
+            return c;
+        });
+        var details = new Dictionary<string, string> { ["EndpointUrl"] = "opc.tcp://x:4840" };
+
+        await _adapter.ConnectAsync(details);   // client[0]
+        await _adapter.ConnectAsync(details);   // reconnect -> client[1]
+
+        Assert.Equal(2, clients.Count);
+        await clients[0].Received(1).DisposeAsync();
+        await clients[1].DidNotReceive().DisposeAsync();
+    }
+
+    [Fact]
+    public async Task StaleClientConnectionLost_DoesNotRaiseDisconnected_AfterReconnect()
+    {
+        var clients = new List<IOpcUaClient>();
+        _mockFactory.Create().Returns(_ =>
+        {
+            var c = Substitute.For<IOpcUaClient>();
+            clients.Add(c);
+            return c;
+        });
+        var details = new Dictionary<string, string> { ["EndpointUrl"] = "opc.tcp://x:4840" };
+        var disconnects = 0;
+        _adapter.Disconnected += () => Interlocked.Increment(ref disconnects);
+
+        await _adapter.ConnectAsync(details);
+        await _adapter.ConnectAsync(details);   // reconnect resets _disconnectFired
+
+        // The OLD client's keep-alive dies later — must be a no-op now.
+        clients[0].ConnectionLost += Raise.Event<Action>();
+        Assert.Equal(0, disconnects);
+
+        // The CURRENT client's loss must still signal.
+        clients[1].ConnectionLost += Raise.Event<Action>();
+        Assert.Equal(1, disconnects);
+    }
 }