fix(dcl): dispose+detach previous OPC UA client on reconnect (S1) — stops session leak and stale ConnectionLost flapping

This commit is contained in:
Joseph Doherty
2026-07-08 23:16:27 -04:00
parent 5d5c747517
commit 11eb5157e6
2 changed files with 70 additions and 0 deletions
@@ -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);
@@ -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);
}
}