From fd4d05534e3ea45926f43fae89319aeed90654c6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 12 Jun 2026 11:30:39 -0400 Subject: [PATCH] fix(historian-client): dispose TcpClient/SslStream on connect+TLS failure (review) --- .../WonderwareHistorianClientOptions.cs | 2 +- .../Internal/FrameChannel.cs | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Contracts/WonderwareHistorianClientOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Contracts/WonderwareHistorianClientOptions.cs index 57e190eb..47f0aa68 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Contracts/WonderwareHistorianClientOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Contracts/WonderwareHistorianClientOptions.cs @@ -8,7 +8,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client; /// /// /// Retry / backoff ownership (finding 006): this module performs exactly one -/// in-place transport reconnect inside PipeChannel.InvokeAsync with no delay, +/// in-place transport reconnect inside FrameChannel.InvokeAsync with no delay, /// and does NOT implement exponential reconnect backoff. Broader retry/backoff is the /// caller's responsibility — the alarm drain worker /// (Core.AlarmHistorian.SqliteStoreAndForwardSink) and the read-side diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client/Internal/FrameChannel.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client/Internal/FrameChannel.cs index 19c16808..0e27e482 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client/Internal/FrameChannel.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client/Internal/FrameChannel.cs @@ -59,13 +59,21 @@ internal sealed class FrameChannel : IAsyncDisposable throw new InvalidOperationException("WonderwareHistorianClientOptions.Host is required for the TCP transport."); var tcp = new TcpClient(); - using (var connectCts = CancellationTokenSource.CreateLinkedTokenSource(ct)) + try { + using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(ct); connectCts.CancelAfter(opts.EffectiveConnectTimeout); await tcp.ConnectAsync(opts.Host!, opts.Port, connectCts.Token).ConfigureAwait(false); } + catch + { + tcp.Dispose(); + throw; + } tcp.NoDelay = true; + // The returned NetworkStream owns the socket (TcpClient.GetStream() uses ownsSocket: true), + // so FrameChannel.ResetTransport() disposing this stream closes the underlying socket. Stream stream = tcp.GetStream(); if (!opts.UseTls) return stream; @@ -75,7 +83,15 @@ internal sealed class FrameChannel : IAsyncDisposable return string.Equals(cert?.GetCertHashString(), opts.ServerCertThumbprint, StringComparison.OrdinalIgnoreCase); return errors == SslPolicyErrors.None; }); - await ssl.AuthenticateAsClientAsync(opts.Host!).ConfigureAwait(false); + try + { + await ssl.AuthenticateAsClientAsync(opts.Host!).ConfigureAwait(false); + } + catch + { + await ssl.DisposeAsync().ConfigureAwait(false); + throw; + } return ssl; };