From f329d8795409c457e02bd95fe1fc2f4043ea1273 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 30 Dec 2025 10:53:35 +0000 Subject: [PATCH 01/18] feat: Improve Unix socket connection handling for proxy compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use SocketsHttpHandler with ConnectCallback for .NET 8+ (more robust) - Add KeepAlive socket option for better proxy socket compatibility - Add configurable UnixSocketConnectTimeout (default 30s) - Improve error handling with proper socket disposal - Add unit tests for DockerClientConfiguration This change addresses issues with Docker socket proxy wrappers (like those used in some CI environments) that may not work with the legacy ManagedHandler. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/Docker.DotNet/DockerClient.cs | 66 +++++++++++++++++-- .../DockerClientConfiguration.cs | 14 +++- .../DockerClientConfigurationTests.cs | 39 +++++++++++ 3 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 test/Docker.DotNet.Tests/DockerClientConfigurationTests.cs diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 2c32db03..b6aa2643 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -89,17 +89,73 @@ await stream.ConnectAsync(timeout, cancellationToken) case "unix": var pipeString = uri.LocalPath; - handler = new ManagedHandler(async (host, port, cancellationToken) => + var socketTimeout = Configuration.SocketConnectTimeout; + +#if NET8_0_OR_GREATER + var socketsHandler = new SocketsHttpHandler { - var sock = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + ConnectCallback = async (context, cancellationToken) => + { + var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + try + { + socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + + using var timeoutCts = new CancellationTokenSource(socketTimeout); + using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); + + await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), linkedCts.Token) + .ConfigureAwait(false); + + return new NetworkStream(socket, ownsSocket: true); + } + catch + { + socket.Dispose(); + throw; + } + }, + PooledConnectionLifetime = TimeSpan.FromMinutes(5), + PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2), + EnableMultipleHttp2Connections = false, + }; - await sock.ConnectAsync(new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString)) - .ConfigureAwait(false); + uri = new UriBuilder("http", uri.Segments.Last()).Uri; + _endpointBaseUri = uri; - return sock; + _client = new HttpClient(socketsHandler, true); + _client.Timeout = Timeout.InfiniteTimeSpan; + return; +#else + handler = new ManagedHandler(async (host, port, cancellationToken) => + { + var sock = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + try + { + sock.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + + var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); + var connectTask = sock.ConnectAsync(endpoint); + var timeoutTask = Task.Delay(socketTimeout, cancellationToken); + + if (await Task.WhenAny(connectTask, timeoutTask).ConfigureAwait(false) == timeoutTask) + { + sock.Dispose(); + throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds} seconds."); + } + + await connectTask.ConfigureAwait(false); + return sock; + } + catch + { + sock.Dispose(); + throw; + } }, logger); uri = new UriBuilder("http", uri.Segments.Last()).Uri; break; +#endif default: throw new Exception($"Unknown URL scheme {configuration.EndpointBaseUri.Scheme}"); diff --git a/src/Docker.DotNet/DockerClientConfiguration.cs b/src/Docker.DotNet/DockerClientConfiguration.cs index a1fb82d3..2998d0d6 100644 --- a/src/Docker.DotNet/DockerClientConfiguration.cs +++ b/src/Docker.DotNet/DockerClientConfiguration.cs @@ -8,8 +8,9 @@ public DockerClientConfiguration( Credentials credentials = null, TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null) - : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, defaultHttpRequestHeaders) + IReadOnlyDictionary defaultHttpRequestHeaders = null, + TimeSpan socketConnectTimeout = default) + : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, defaultHttpRequestHeaders, socketConnectTimeout) { } @@ -18,7 +19,8 @@ public DockerClientConfiguration( Credentials credentials = null, TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null) + IReadOnlyDictionary defaultHttpRequestHeaders = null, + TimeSpan socketConnectTimeout = default) { if (endpoint == null) { @@ -34,6 +36,7 @@ public DockerClientConfiguration( Credentials = credentials ?? new AnonymousCredentials(); DefaultTimeout = TimeSpan.Equals(TimeSpan.Zero, defaultTimeout) ? TimeSpan.FromSeconds(100) : defaultTimeout; NamedPipeConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, namedPipeConnectTimeout) ? TimeSpan.FromMilliseconds(100) : namedPipeConnectTimeout; + SocketConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, socketConnectTimeout) ? TimeSpan.FromSeconds(30) : socketConnectTimeout; DefaultHttpRequestHeaders = defaultHttpRequestHeaders ?? new Dictionary(); } @@ -50,6 +53,11 @@ public DockerClientConfiguration( public TimeSpan NamedPipeConnectTimeout { get; } + /// + /// Gets the timeout for socket connections (Unix sockets and TCP). + /// + public TimeSpan SocketConnectTimeout { get; } + public DockerClient CreateClient(Version requestedApiVersion = null, ILogger logger = null) { return new DockerClient(this, requestedApiVersion, logger); diff --git a/test/Docker.DotNet.Tests/DockerClientConfigurationTests.cs b/test/Docker.DotNet.Tests/DockerClientConfigurationTests.cs new file mode 100644 index 00000000..d8593c37 --- /dev/null +++ b/test/Docker.DotNet.Tests/DockerClientConfigurationTests.cs @@ -0,0 +1,39 @@ +namespace Docker.DotNet.Tests; + +public class DockerClientConfigurationTests +{ + [Fact] + public void SocketConnectTimeout_DefaultValue_Is30Seconds() + { + using var config = new DockerClientConfiguration(); + + Assert.Equal(TimeSpan.FromSeconds(30), config.SocketConnectTimeout); + } + + [Fact] + public void SocketConnectTimeout_CustomValue_IsPreserved() + { + var customTimeout = TimeSpan.FromSeconds(60); + + using var config = new DockerClientConfiguration( + socketConnectTimeout: customTimeout); + + Assert.Equal(customTimeout, config.SocketConnectTimeout); + } + + [Fact] + public void NamedPipeConnectTimeout_DefaultValue_Is100Milliseconds() + { + using var config = new DockerClientConfiguration(); + + Assert.Equal(TimeSpan.FromMilliseconds(100), config.NamedPipeConnectTimeout); + } + + [Fact] + public void DefaultTimeout_DefaultValue_Is100Seconds() + { + using var config = new DockerClientConfiguration(); + + Assert.Equal(TimeSpan.FromSeconds(100), config.DefaultTimeout); + } +} From a48643ba6b9ff0bd8c901cbeef68d85bf464c3cb Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 2 Jan 2026 10:34:40 +0000 Subject: [PATCH 02/18] fix: Update to Assert.ThrowsAnyAsync for OperationCanceledException in container log tests --- test/Docker.DotNet.Tests/IContainerOperationsTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Docker.DotNet.Tests/IContainerOperationsTests.cs b/test/Docker.DotNet.Tests/IContainerOperationsTests.cs index cc2ae303..e22cadfc 100644 --- a/test/Docker.DotNet.Tests/IContainerOperationsTests.cs +++ b/test/Docker.DotNet.Tests/IContainerOperationsTests.cs @@ -190,7 +190,7 @@ await _testFixture.DockerClient.Containers.StartContainerAsync( containerLogsCts.CancelAfter(TimeSpan.FromSeconds(5)); - await Assert.ThrowsAsync(() => _testFixture.DockerClient.Containers.GetContainerLogsAsync( + await Assert.ThrowsAnyAsync(() => _testFixture.DockerClient.Containers.GetContainerLogsAsync( createContainerResponse.ID, new ContainerLogsParameters { @@ -240,7 +240,7 @@ await _testFixture.DockerClient.Containers.StartContainerAsync( containerLogsCts.Token ); - await Assert.ThrowsAsync(() => containerLogsTask); + await Assert.ThrowsAnyAsync(() => containerLogsTask); } [Fact] @@ -288,7 +288,7 @@ await _testFixture.DockerClient.Containers.StopContainerAsync( _testFixture.Cts.Token ); - await Assert.ThrowsAsync(() => containerLogsTask); + await Assert.ThrowsAnyAsync(() => containerLogsTask); _testOutputHelper.WriteLine($"Line count: {logList.Count}"); Assert.NotEmpty(logList); From ed2fc1142fbe28b3db9319b8bf7e375aab7b50f1 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 2 Jan 2026 10:48:19 +0000 Subject: [PATCH 03/18] fix: Improve Unix socket connection handling by using configured credentials and adding cancellation support --- src/Docker.DotNet/DockerClient.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index b6aa2643..b7090f96 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -123,7 +123,7 @@ await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeSt uri = new UriBuilder("http", uri.Segments.Last()).Uri; _endpointBaseUri = uri; - _client = new HttpClient(socketsHandler, true); + _client = new HttpClient(Configuration.Credentials.GetHandler(socketsHandler), true); _client.Timeout = Timeout.InfiniteTimeSpan; return; #else @@ -136,14 +136,21 @@ await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeSt var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); var connectTask = sock.ConnectAsync(endpoint); - var timeoutTask = Task.Delay(socketTimeout, cancellationToken); + + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(socketTimeout); + var timeoutTask = Task.Delay(Timeout.InfiniteTimeSpan, timeoutCts.Token); if (await Task.WhenAny(connectTask, timeoutTask).ConfigureAwait(false) == timeoutTask) { sock.Dispose(); + // Check if cancellation was requested by the caller before throwing timeout + cancellationToken.ThrowIfCancellationRequested(); throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds} seconds."); } + // Cancel the timeout task to clean up the timer + timeoutCts.Cancel(); await connectTask.ConfigureAwait(false); return sock; } From 568f421f8f98c93d4a918be7d050d89ecc4f48cd Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Mon, 19 Jan 2026 16:33:47 +0000 Subject: [PATCH 04/18] fix: Refactor DockerClientConfiguration constructor parameters for clarity and improve Unix socket connection timeout documentation --- src/Docker.DotNet/DockerClient.cs | 58 ++++++++----------- .../DockerClientConfiguration.cs | 12 ++-- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index b7090f96..ce53181c 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -34,7 +34,7 @@ internal DockerClient(DockerClientConfiguration configuration, Version requested Plugin = new PluginOperations(this); Exec = new ExecOperations(this); - ManagedHandler handler; + HttpMessageHandler handler; var uri = Configuration.EndpointBaseUri; switch (uri.Scheme.ToLowerInvariant()) { @@ -90,79 +90,69 @@ await stream.ConnectAsync(timeout, cancellationToken) case "unix": var pipeString = uri.LocalPath; var socketTimeout = Configuration.SocketConnectTimeout; - #if NET8_0_OR_GREATER - var socketsHandler = new SocketsHttpHandler + handler = new SocketsHttpHandler { - ConnectCallback = async (context, cancellationToken) => + ConnectCallback = async (_, cancellationToken) => { var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + try { socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - using var timeoutCts = new CancellationTokenSource(socketTimeout); - using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(socketTimeout); - await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), linkedCts.Token) + await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), timeoutCts.Token) .ConfigureAwait(false); - return new NetworkStream(socket, ownsSocket: true); + return new NetworkStream(socket, true); } catch { socket.Dispose(); throw; } - }, - PooledConnectionLifetime = TimeSpan.FromMinutes(5), - PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2), - EnableMultipleHttp2Connections = false, + } }; - - uri = new UriBuilder("http", uri.Segments.Last()).Uri; - _endpointBaseUri = uri; - - _client = new HttpClient(Configuration.Credentials.GetHandler(socketsHandler), true); - _client.Timeout = Timeout.InfiniteTimeSpan; - return; #else - handler = new ManagedHandler(async (host, port, cancellationToken) => + handler = new ManagedHandler(async (_, _, cancellationToken) => { - var sock = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); + var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + try { - sock.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - - var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); - var connectTask = sock.ConnectAsync(endpoint); + socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); timeoutCts.CancelAfter(socketTimeout); + + var connectTask = socket.ConnectAsync(endpoint); var timeoutTask = Task.Delay(Timeout.InfiniteTimeSpan, timeoutCts.Token); - if (await Task.WhenAny(connectTask, timeoutTask).ConfigureAwait(false) == timeoutTask) + var completedTask = await Task.WhenAny(connectTask, timeoutTask) + .ConfigureAwait(false); + + if (completedTask == timeoutTask) { - sock.Dispose(); - // Check if cancellation was requested by the caller before throwing timeout cancellationToken.ThrowIfCancellationRequested(); - throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds} seconds."); + throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds}s."); } - // Cancel the timeout task to clean up the timer - timeoutCts.Cancel(); await connectTask.ConfigureAwait(false); - return sock; + return socket; } catch { - sock.Dispose(); + socket.Dispose(); throw; } }, logger); +#endif uri = new UriBuilder("http", uri.Segments.Last()).Uri; break; -#endif default: throw new Exception($"Unknown URL scheme {configuration.EndpointBaseUri.Scheme}"); diff --git a/src/Docker.DotNet/DockerClientConfiguration.cs b/src/Docker.DotNet/DockerClientConfiguration.cs index 2998d0d6..12be1e81 100644 --- a/src/Docker.DotNet/DockerClientConfiguration.cs +++ b/src/Docker.DotNet/DockerClientConfiguration.cs @@ -8,9 +8,9 @@ public DockerClientConfiguration( Credentials credentials = null, TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null, - TimeSpan socketConnectTimeout = default) - : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, defaultHttpRequestHeaders, socketConnectTimeout) + TimeSpan socketConnectTimeout = default, + IReadOnlyDictionary defaultHttpRequestHeaders = null) + : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, socketConnectTimeout, defaultHttpRequestHeaders) { } @@ -19,8 +19,8 @@ public DockerClientConfiguration( Credentials credentials = null, TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null, - TimeSpan socketConnectTimeout = default) + TimeSpan socketConnectTimeout = default, + IReadOnlyDictionary defaultHttpRequestHeaders = null) { if (endpoint == null) { @@ -54,7 +54,7 @@ public DockerClientConfiguration( public TimeSpan NamedPipeConnectTimeout { get; } /// - /// Gets the timeout for socket connections (Unix sockets and TCP). + /// Gets the timeout for Unix domain socket connections. /// public TimeSpan SocketConnectTimeout { get; } From 12796ee12e3b2f9e800ddd150ff6030879c0ee7d Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Mon, 19 Jan 2026 17:10:57 +0000 Subject: [PATCH 05/18] refactor: Simplify Unix socket handling by removing SocketsHttpHandler and using ManagedHandler for connection hijacking support --- src/Docker.DotNet/DockerClient.cs | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index ce53181c..2402986a 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -90,33 +90,8 @@ await stream.ConnectAsync(timeout, cancellationToken) case "unix": var pipeString = uri.LocalPath; var socketTimeout = Configuration.SocketConnectTimeout; -#if NET8_0_OR_GREATER - handler = new SocketsHttpHandler - { - ConnectCallback = async (_, cancellationToken) => - { - var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); - - try - { - socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeoutCts.CancelAfter(socketTimeout); - - await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), timeoutCts.Token) - .ConfigureAwait(false); - - return new NetworkStream(socket, true); - } - catch - { - socket.Dispose(); - throw; - } - } - }; -#else + // ManagedHandler is required for hijacked stream support (attach/exec operations). + // SocketsHttpHandler doesn't support the HttpConnectionResponseContent needed for connection hijacking. handler = new ManagedHandler(async (_, _, cancellationToken) => { var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); @@ -150,7 +125,6 @@ await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeSt throw; } }, logger); -#endif uri = new UriBuilder("http", uri.Segments.Last()).Uri; break; From 22980b95e03ef7a2bc610f4207bd90ea0fd762f1 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Mon, 19 Jan 2026 17:23:50 +0000 Subject: [PATCH 06/18] feat: Enhance Unix socket handling with improved connection support and hijacking capabilities --- src/Docker.DotNet/DockerClient.cs | 107 ++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 2402986a..14f42d26 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -11,6 +11,12 @@ public sealed class DockerClient : IDockerClient private readonly HttpClient _client; +#if NET8_0_OR_GREATER + // Separate client for hijacked stream operations (attach/exec) when using SocketsHttpHandler. + // SocketsHttpHandler doesn't support connection hijacking, so we need ManagedHandler for those operations. + private readonly HttpClient _hijackClient; +#endif + private readonly Uri _endpointBaseUri; private readonly Version _requestedApiVersion; @@ -90,8 +96,78 @@ await stream.ConnectAsync(timeout, cancellationToken) case "unix": var pipeString = uri.LocalPath; var socketTimeout = Configuration.SocketConnectTimeout; - // ManagedHandler is required for hijacked stream support (attach/exec operations). + uri = new UriBuilder("http", uri.Segments.Last()).Uri; +#if NET8_0_OR_GREATER + // Use SocketsHttpHandler for regular operations (better proxy compatibility) + handler = new SocketsHttpHandler + { + ConnectCallback = async (_, cancellationToken) => + { + var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + + try + { + socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(socketTimeout); + + await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), timeoutCts.Token) + .ConfigureAwait(false); + + return new NetworkStream(socket, true); + } + catch + { + socket.Dispose(); + throw; + } + } + }; + + // ManagedHandler is required for hijacked stream operations (attach/exec). // SocketsHttpHandler doesn't support the HttpConnectionResponseContent needed for connection hijacking. + var hijackHandler = new ManagedHandler(async (_, _, cancellationToken) => + { + var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); + var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + + try + { + socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(socketTimeout); + + var connectTask = socket.ConnectAsync(endpoint); + var timeoutTask = Task.Delay(Timeout.InfiniteTimeSpan, timeoutCts.Token); + + var completedTask = await Task.WhenAny(connectTask, timeoutTask) + .ConfigureAwait(false); + + if (completedTask == timeoutTask) + { + cancellationToken.ThrowIfCancellationRequested(); + throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds}s."); + } + + await connectTask.ConfigureAwait(false); + return socket; + } + catch + { + socket.Dispose(); + throw; + } + }, logger); + + _endpointBaseUri = uri; + _client = new HttpClient(Configuration.Credentials.GetHandler(handler), true); + _client.Timeout = Timeout.InfiniteTimeSpan; + _hijackClient = new HttpClient(Configuration.Credentials.GetHandler(hijackHandler), true); + _hijackClient.Timeout = Timeout.InfiniteTimeSpan; + return; +#else handler = new ManagedHandler(async (_, _, cancellationToken) => { var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); @@ -125,8 +201,8 @@ await stream.ConnectAsync(timeout, cancellationToken) throw; } }, logger); - uri = new UriBuilder("http", uri.Segments.Last()).Uri; break; +#endif default: throw new Exception($"Unknown URL scheme {configuration.EndpointBaseUri.Scheme}"); @@ -170,6 +246,9 @@ public void Dispose() { Configuration.Dispose(); _client.Dispose(); +#if NET8_0_OR_GREATER + _hijackClient?.Dispose(); +#endif } internal Task MakeRequestAsync( @@ -416,8 +495,14 @@ internal async Task MakeRequestForHijackedStreamAsync( headers.Add("Upgrade", "tcp"); headers.Add("Connection", "upgrade"); +#if NET8_0_OR_GREATER + // Use _hijackClient for SocketsHttpHandler scenarios where _client doesn't support hijacking + var response = await PrivateMakeRequestAsync(_hijackClient ?? _client, timeout, HttpCompletionOption.ResponseHeadersRead, method, path, queryString, headers, body, cancellationToken) + .ConfigureAwait(false); +#else var response = await PrivateMakeRequestAsync(timeout, HttpCompletionOption.ResponseHeadersRead, method, path, queryString, headers, body, cancellationToken) .ConfigureAwait(false); +#endif await HandleIfErrorResponseAsync(response.StatusCode, response, errorHandlers) .ConfigureAwait(false); @@ -430,7 +515,21 @@ await HandleIfErrorResponseAsync(response.StatusCode, response, errorHandlers) return content.HijackStream(); } + private Task PrivateMakeRequestAsync( + TimeSpan timeout, + HttpCompletionOption completionOption, + HttpMethod method, + string path, + IQueryString queryString, + IDictionary headers, + IRequestContent data, + CancellationToken cancellationToken) + { + return PrivateMakeRequestAsync(_client, timeout, completionOption, method, path, queryString, headers, data, cancellationToken); + } + private async Task PrivateMakeRequestAsync( + HttpClient client, TimeSpan timeout, HttpCompletionOption completionOption, HttpMethod method, @@ -448,7 +547,7 @@ private async Task PrivateMakeRequestAsync( using var disposable = cancellationToken.Register(() => tcs.SetCanceled()); - return await await Task.WhenAny(tcs.Task, _client.SendAsync(request, completionOption, cancellationToken)) + return await await Task.WhenAny(tcs.Task, client.SendAsync(request, completionOption, cancellationToken)) .ConfigureAwait(false); } else @@ -457,7 +556,7 @@ private async Task PrivateMakeRequestAsync( using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); - return await _client.SendAsync(request, completionOption, linkedCts.Token) + return await client.SendAsync(request, completionOption, linkedCts.Token) .ConfigureAwait(false); } } From 9d275f32e8ed5ef396244d209719f15e5ab7afde Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 10:07:47 +0000 Subject: [PATCH 07/18] refactor: Modernize ManagedHandler for proxy compatibility while preserving hijacked streams Remove dual-mode SocketsHttpHandler approach as SocketsHttpHandler cannot support stream hijacking (required for exec/attach/logs). Instead, modernize ManagedHandler with configurable socket options for better proxy compatibility. Changes: - Add SocketConfiguration class for configurable socket options (KeepAlive, KeepAliveTime, KeepAliveInterval, KeepAliveRetryCount, NoDelay, buffer sizes) - Update ManagedHandler to accept SocketConfiguration and apply modern socket options including TCP keep-alive settings (.NET 5+/7+) - Remove SocketsHttpHandler and _hijackClient from DockerClient - Use ManagedHandler for all operations to preserve HttpConnectionResponseContent hijacking capability - Reorganize DockerClientConfiguration properties to group timeouts together - Use modern Socket.ConnectAsync with CancellationToken on .NET 5+ This addresses PR feedback that the dual-mode approach was incorrect and that we should properly understand why SocketsHttpHandler cannot support hijacking. --- src/Docker.DotNet/DockerClient.cs | 110 +++--------- .../DockerClientConfiguration.cs | 33 +++- .../ManagedHandler.cs | 35 +++- src/Docker.DotNet/SocketConfiguration.cs | 158 ++++++++++++++++++ 4 files changed, 241 insertions(+), 95 deletions(-) create mode 100644 src/Docker.DotNet/SocketConfiguration.cs diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 14f42d26..18d6de75 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -11,12 +11,6 @@ public sealed class DockerClient : IDockerClient private readonly HttpClient _client; -#if NET8_0_OR_GREATER - // Separate client for hijacked stream operations (attach/exec) when using SocketsHttpHandler. - // SocketsHttpHandler doesn't support connection hijacking, so we need ManagedHandler for those operations. - private readonly HttpClient _hijackClient; -#endif - private readonly Uri _endpointBaseUri; private readonly Version _requestedApiVersion; @@ -86,59 +80,44 @@ await stream.ConnectAsync(timeout, cancellationToken) Scheme = configuration.Credentials.IsTlsCredentials() ? "https" : "http" }; uri = builder.Uri; - handler = new ManagedHandler(logger); + handler = new ManagedHandler(logger, Configuration.SocketConfiguration); break; case "https": - handler = new ManagedHandler(logger); + handler = new ManagedHandler(logger, Configuration.SocketConfiguration); break; case "unix": var pipeString = uri.LocalPath; var socketTimeout = Configuration.SocketConnectTimeout; + var socketConfig = Configuration.SocketConfiguration; uri = new UriBuilder("http", uri.Segments.Last()).Uri; -#if NET8_0_OR_GREATER - // Use SocketsHttpHandler for regular operations (better proxy compatibility) - handler = new SocketsHttpHandler - { - ConnectCallback = async (_, cancellationToken) => - { - var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); - - try - { - socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeoutCts.CancelAfter(socketTimeout); - - await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeString), timeoutCts.Token) - .ConfigureAwait(false); - - return new NetworkStream(socket, true); - } - catch - { - socket.Dispose(); - throw; - } - } - }; - // ManagedHandler is required for hijacked stream operations (attach/exec). - // SocketsHttpHandler doesn't support the HttpConnectionResponseContent needed for connection hijacking. - var hijackHandler = new ManagedHandler(async (_, _, cancellationToken) => + // Use ManagedHandler for Unix socket connections. + // ManagedHandler is required for hijacked stream operations (attach/exec/logs) + // as it provides HttpConnectionResponseContent needed for connection hijacking. + // SocketsHttpHandler cannot support hijacking because it encapsulates the transport stream. + handler = new ManagedHandler(async (_, _, cancellationToken) => { var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); try { - socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + // Apply socket configuration for better proxy compatibility + if (socketConfig.KeepAlive) + { + socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); + } using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); timeoutCts.CancelAfter(socketTimeout); +#if NET5_0_OR_GREATER + // Use modern ConnectAsync with cancellation support + await socket.ConnectAsync(endpoint, timeoutCts.Token) + .ConfigureAwait(false); +#else var connectTask = socket.ConnectAsync(endpoint); var timeoutTask = Task.Delay(Timeout.InfiniteTimeSpan, timeoutCts.Token); @@ -152,57 +131,21 @@ await socket.ConnectAsync(new System.Net.Sockets.UnixDomainSocketEndPoint(pipeSt } await connectTask.ConfigureAwait(false); +#endif return socket; } - catch + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { socket.Dispose(); - throw; - } - }, logger); - - _endpointBaseUri = uri; - _client = new HttpClient(Configuration.Credentials.GetHandler(handler), true); - _client.Timeout = Timeout.InfiniteTimeSpan; - _hijackClient = new HttpClient(Configuration.Credentials.GetHandler(hijackHandler), true); - _hijackClient.Timeout = Timeout.InfiniteTimeSpan; - return; -#else - handler = new ManagedHandler(async (_, _, cancellationToken) => - { - var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); - var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); - - try - { - socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeoutCts.CancelAfter(socketTimeout); - - var connectTask = socket.ConnectAsync(endpoint); - var timeoutTask = Task.Delay(Timeout.InfiniteTimeSpan, timeoutCts.Token); - - var completedTask = await Task.WhenAny(connectTask, timeoutTask) - .ConfigureAwait(false); - - if (completedTask == timeoutTask) - { - cancellationToken.ThrowIfCancellationRequested(); - throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds}s."); - } - - await connectTask.ConfigureAwait(false); - return socket; + throw new TimeoutException($"Connection to Unix socket '{pipeString}' timed out after {socketTimeout.TotalSeconds}s."); } catch { socket.Dispose(); throw; } - }, logger); + }, logger, socketConfig); break; -#endif default: throw new Exception($"Unknown URL scheme {configuration.EndpointBaseUri.Scheme}"); @@ -246,9 +189,6 @@ public void Dispose() { Configuration.Dispose(); _client.Dispose(); -#if NET8_0_OR_GREATER - _hijackClient?.Dispose(); -#endif } internal Task MakeRequestAsync( @@ -495,14 +435,8 @@ internal async Task MakeRequestForHijackedStreamAsync( headers.Add("Upgrade", "tcp"); headers.Add("Connection", "upgrade"); -#if NET8_0_OR_GREATER - // Use _hijackClient for SocketsHttpHandler scenarios where _client doesn't support hijacking - var response = await PrivateMakeRequestAsync(_hijackClient ?? _client, timeout, HttpCompletionOption.ResponseHeadersRead, method, path, queryString, headers, body, cancellationToken) - .ConfigureAwait(false); -#else var response = await PrivateMakeRequestAsync(timeout, HttpCompletionOption.ResponseHeadersRead, method, path, queryString, headers, body, cancellationToken) .ConfigureAwait(false); -#endif await HandleIfErrorResponseAsync(response.StatusCode, response, errorHandlers) .ConfigureAwait(false); diff --git a/src/Docker.DotNet/DockerClientConfiguration.cs b/src/Docker.DotNet/DockerClientConfiguration.cs index 12be1e81..b3f22bf7 100644 --- a/src/Docker.DotNet/DockerClientConfiguration.cs +++ b/src/Docker.DotNet/DockerClientConfiguration.cs @@ -9,8 +9,9 @@ public DockerClientConfiguration( TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, TimeSpan socketConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null) - : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, socketConnectTimeout, defaultHttpRequestHeaders) + IReadOnlyDictionary defaultHttpRequestHeaders = null, + SocketConfiguration socketConfiguration = null) + : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, socketConnectTimeout, defaultHttpRequestHeaders, socketConfiguration) { } @@ -20,7 +21,8 @@ public DockerClientConfiguration( TimeSpan defaultTimeout = default, TimeSpan namedPipeConnectTimeout = default, TimeSpan socketConnectTimeout = default, - IReadOnlyDictionary defaultHttpRequestHeaders = null) + IReadOnlyDictionary defaultHttpRequestHeaders = null, + SocketConfiguration socketConfiguration = null) { if (endpoint == null) { @@ -38,19 +40,27 @@ public DockerClientConfiguration( NamedPipeConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, namedPipeConnectTimeout) ? TimeSpan.FromMilliseconds(100) : namedPipeConnectTimeout; SocketConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, socketConnectTimeout) ? TimeSpan.FromSeconds(30) : socketConnectTimeout; DefaultHttpRequestHeaders = defaultHttpRequestHeaders ?? new Dictionary(); + SocketConfiguration = socketConfiguration ?? SocketConfiguration.Default; } /// - /// Gets the collection of default HTTP request headers. + /// Gets the Docker endpoint base URI. /// - public IReadOnlyDictionary DefaultHttpRequestHeaders { get; } - public Uri EndpointBaseUri { get; } + /// + /// Gets the credentials used for authentication. + /// public Credentials Credentials { get; } + /// + /// Gets the default timeout for API requests. + /// public TimeSpan DefaultTimeout { get; } + /// + /// Gets the timeout for named pipe connections (Windows). + /// public TimeSpan NamedPipeConnectTimeout { get; } /// @@ -58,6 +68,17 @@ public DockerClientConfiguration( /// public TimeSpan SocketConnectTimeout { get; } + /// + /// Gets the socket configuration options for connection handling. + /// These settings help improve proxy compatibility and connection reliability. + /// + public SocketConfiguration SocketConfiguration { get; } + + /// + /// Gets the collection of default HTTP request headers. + /// + public IReadOnlyDictionary DefaultHttpRequestHeaders { get; } + public DockerClient CreateClient(Version requestedApiVersion = null, ILogger logger = null) { return new DockerClient(this, requestedApiVersion, logger); diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index 728cf6eb..289a5393 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -1,6 +1,7 @@ namespace Microsoft.Net.Http.Client; using System; +using Docker.DotNet; public class ManagedHandler : HttpMessageHandler { @@ -10,6 +11,8 @@ public class ManagedHandler : HttpMessageHandler private readonly SocketOpener _socketOpener; + private readonly SocketConfiguration _socketConfiguration; + private IWebProxy _proxy; public delegate Task StreamOpener(string host, int port, CancellationToken cancellationToken); @@ -17,20 +20,35 @@ public class ManagedHandler : HttpMessageHandler public delegate Task SocketOpener(string host, int port, CancellationToken cancellationToken); public ManagedHandler(ILogger logger) + : this(logger, SocketConfiguration.Default) + { + } + + public ManagedHandler(ILogger logger, SocketConfiguration socketConfiguration) { _logger = logger; + _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = TcpSocketOpenerAsync; } public ManagedHandler(StreamOpener opener, ILogger logger) { _logger = logger; + _socketConfiguration = SocketConfiguration.Default; _streamOpener = opener ?? throw new ArgumentNullException(nameof(opener)); } public ManagedHandler(SocketOpener opener, ILogger logger) { _logger = logger; + _socketConfiguration = SocketConfiguration.Default; + _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); + } + + public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration socketConfiguration) + { + _logger = logger; + _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); } @@ -315,10 +333,16 @@ private ProxyMode DetermineProxyModeAndAddressLine(HttpRequestMessage request) return ProxyMode.Tunnel; } - private static async Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) + private async Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) { +#if NET5_0_OR_GREATER + // Use modern DNS resolution with cancellation support + var addresses = await Dns.GetHostAddressesAsync(host, cancellationToken) + .ConfigureAwait(false); +#else var addresses = await Dns.GetHostAddressesAsync(host) .ConfigureAwait(false); +#endif if (addresses.Length == 0) { @@ -333,8 +357,17 @@ private static async Task TcpSocketOpenerAsync(string host, int port, Ca try { + // Apply socket configuration for better proxy compatibility + _socketConfiguration.ApplyTo(socket); + +#if NET5_0_OR_GREATER + // Use modern ConnectAsync with cancellation support + await socket.ConnectAsync(address, port, cancellationToken) + .ConfigureAwait(false); +#else await socket.ConnectAsync(address, port) .ConfigureAwait(false); +#endif return socket; } diff --git a/src/Docker.DotNet/SocketConfiguration.cs b/src/Docker.DotNet/SocketConfiguration.cs new file mode 100644 index 00000000..95d4d2be --- /dev/null +++ b/src/Docker.DotNet/SocketConfiguration.cs @@ -0,0 +1,158 @@ +namespace Docker.DotNet; + +using System; + +/// +/// Configuration options for socket connections. +/// These settings help improve proxy compatibility and connection reliability. +/// +public sealed class SocketConfiguration +{ + /// + /// Default socket configuration with sensible defaults for Docker socket connections. + /// + public static SocketConfiguration Default { get; } = new SocketConfiguration(); + + /// + /// Gets or sets whether TCP keep-alive is enabled. + /// Default is true for better proxy compatibility. + /// + public bool KeepAlive { get; set; } = true; + + /// + /// Gets or sets the time (in seconds) the connection needs to remain idle + /// before TCP starts sending keep-alive probes. + /// Only applies when is true. + /// Default is 30 seconds. + /// + /// + /// This setting requires .NET 5.0+ for TCP sockets. On Unix sockets, this is ignored. + /// + public int KeepAliveTime { get; set; } = 30; + + /// + /// Gets or sets the interval (in seconds) between keep-alive probes. + /// Only applies when is true. + /// Default is 10 seconds. + /// + /// + /// This setting requires .NET 5.0+ for TCP sockets. On Unix sockets, this is ignored. + /// + public int KeepAliveInterval { get; set; } = 10; + + /// + /// Gets or sets the number of keep-alive probes to send before considering the connection dead. + /// Only applies when is true. + /// Default is 3 retries. + /// + /// + /// This setting requires .NET 7.0+ for TCP sockets. On Unix sockets, this is ignored. + /// + public int KeepAliveRetryCount { get; set; } = 3; + + /// + /// Gets or sets whether to disable the Nagle algorithm (TCP_NODELAY). + /// Default is true for lower latency with Docker API calls. + /// + /// + /// Disabling Nagle's algorithm reduces latency for small packets at the cost of + /// potentially increased network traffic. This is generally desirable for Docker API + /// communication which consists of many small request/response pairs. + /// + public bool NoDelay { get; set; } = true; + + /// + /// Gets or sets the socket send buffer size in bytes. + /// Default is null (uses system default). + /// + public int? SendBufferSize { get; set; } + + /// + /// Gets or sets the socket receive buffer size in bytes. + /// Default is null (uses system default). + /// + public int? ReceiveBufferSize { get; set; } + + /// + /// Applies the configuration to a socket. + /// + /// The socket to configure. + /// Thrown when socket is null. + public void ApplyTo(System.Net.Sockets.Socket socket) + { + if (socket == null) + { + throw new ArgumentNullException(nameof(socket)); + } + + // Apply KeepAlive - works on all platforms + if (KeepAlive) + { + socket.SetSocketOption( + System.Net.Sockets.SocketOptionLevel.Socket, + System.Net.Sockets.SocketOptionName.KeepAlive, + true); + } + + // Apply TCP-specific options only for TCP sockets (not Unix domain sockets) + if (socket.ProtocolType == System.Net.Sockets.ProtocolType.Tcp) + { + if (NoDelay) + { + socket.NoDelay = true; + } + +#if NET5_0_OR_GREATER + // TCP KeepAlive time and interval (requires .NET 5.0+) + if (KeepAlive) + { + try + { + socket.SetSocketOption( + System.Net.Sockets.SocketOptionLevel.Tcp, + System.Net.Sockets.SocketOptionName.TcpKeepAliveTime, + KeepAliveTime); + + socket.SetSocketOption( + System.Net.Sockets.SocketOptionLevel.Tcp, + System.Net.Sockets.SocketOptionName.TcpKeepAliveInterval, + KeepAliveInterval); + } + catch (System.Net.Sockets.SocketException) + { + // These options may not be available on all platforms, ignore failures + } + } +#endif + +#if NET7_0_OR_GREATER + // TCP KeepAlive retry count (requires .NET 7.0+) + if (KeepAlive) + { + try + { + socket.SetSocketOption( + System.Net.Sockets.SocketOptionLevel.Tcp, + System.Net.Sockets.SocketOptionName.TcpKeepAliveRetryCount, + KeepAliveRetryCount); + } + catch (System.Net.Sockets.SocketException) + { + // This option may not be available on all platforms, ignore failures + } + } +#endif + } + + // Apply buffer sizes if specified + if (SendBufferSize.HasValue) + { + socket.SendBufferSize = SendBufferSize.Value; + } + + if (ReceiveBufferSize.HasValue) + { + socket.ReceiveBufferSize = ReceiveBufferSize.Value; + } + } +} From 444c5d22d6cba785c51cd0743056f8495800fbb1 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 10:14:36 +0000 Subject: [PATCH 08/18] refactor: Remove redundant PrivateMakeRequestAsync overload The overload accepting HttpClient parameter was only needed for the dual-mode approach with _hijackClient. Since we now use only _client, the indirection is unnecessary. --- src/Docker.DotNet/DockerClient.cs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 18d6de75..2d568351 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -449,21 +449,7 @@ await HandleIfErrorResponseAsync(response.StatusCode, response, errorHandlers) return content.HijackStream(); } - private Task PrivateMakeRequestAsync( - TimeSpan timeout, - HttpCompletionOption completionOption, - HttpMethod method, - string path, - IQueryString queryString, - IDictionary headers, - IRequestContent data, - CancellationToken cancellationToken) - { - return PrivateMakeRequestAsync(_client, timeout, completionOption, method, path, queryString, headers, data, cancellationToken); - } - private async Task PrivateMakeRequestAsync( - HttpClient client, TimeSpan timeout, HttpCompletionOption completionOption, HttpMethod method, @@ -481,7 +467,7 @@ private async Task PrivateMakeRequestAsync( using var disposable = cancellationToken.Register(() => tcs.SetCanceled()); - return await await Task.WhenAny(tcs.Task, client.SendAsync(request, completionOption, cancellationToken)) + return await await Task.WhenAny(tcs.Task, _client.SendAsync(request, completionOption, cancellationToken)) .ConfigureAwait(false); } else @@ -490,7 +476,7 @@ private async Task PrivateMakeRequestAsync( using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); - return await client.SendAsync(request, completionOption, linkedCts.Token) + return await _client.SendAsync(request, completionOption, linkedCts.Token) .ConfigureAwait(false); } } From a50cff0f1d2246a2c93c4c9d93788d5eaac02a1f Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 12:32:41 +0000 Subject: [PATCH 09/18] feat: Modernize ManagedHandler with memory-efficient APIs, connection pooling, and modern .NET features - Add Memory/ReadOnlyMemory overloads and IAsyncDisposable to stream classes - Implement modern TLS with SslClientAuthenticationOptions and ConnectTimeout property - Add RetryPolicy with exponential backoff and jitter for transient connection failures - Implement Happy Eyeballs (RFC 8305) for parallel IPv6/IPv4 connection racing - Add ConnectionPool for HTTP connection reuse with idle timeout and lifetime limits - Use HttpClient.DefaultProxy for modern proxy resolution on NET6+ - Add PipeReader-based line reading with automatic fallback - Add comprehensive test coverage in ManagedHandlerTests.cs Co-Authored-By: Claude Opus 4.5 --- .../BufferedReadStream.cs | 177 ++++++ .../ChunkedReadStream.cs | 87 +++ .../ChunkedWriteStream.cs | 54 ++ .../ConnectionPool.cs | 296 ++++++++++ .../ContentLengthReadStream.cs | 39 +- .../HttpConnection.cs | 32 +- .../HttpConnectionResponseContent.cs | 55 +- .../ManagedHandler.cs | 438 ++++++++++++++- src/Docker.DotNet/RetryPolicy.cs | 194 +++++++ src/Docker.DotNet/SocketConfiguration.cs | 20 + .../ManagedHandlerTests.cs | 504 ++++++++++++++++++ 11 files changed, 1869 insertions(+), 27 deletions(-) create mode 100644 src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs create mode 100644 src/Docker.DotNet/RetryPolicy.cs create mode 100644 test/Docker.DotNet.Tests/ManagedHandlerTests.cs diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs index d59b1db3..9d1f6635 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs @@ -1,6 +1,9 @@ namespace Microsoft.Net.Http.Client; internal sealed class BufferedReadStream : WriteClosableStream, IPeekableStream +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + , IAsyncDisposable +#endif { private readonly Stream _inner; private readonly Socket _socket; @@ -9,6 +12,23 @@ internal sealed class BufferedReadStream : WriteClosableStream, IPeekableStream private int _bufferRefCount; private int _bufferOffset; private int _bufferCount; + private bool _disposed; + +#if NET6_0_OR_GREATER + private PipeReader _pipeReader; + private bool _usePipeReader = true; + private bool _pipeReaderFailed; + + /// + /// Gets or sets whether to use PipeReader for line reading operations. + /// Default is true. Falls back to byte-by-byte reading on failure. + /// + public bool UsePipeReader + { + get => _usePipeReader; + set => _usePipeReader = value; + } +#endif public BufferedReadStream(Stream inner, Socket socket, ILogger logger) : this(inner, socket, 8192, logger) @@ -59,19 +79,55 @@ public override long Position protected override void Dispose(bool disposing) { + if (_disposed) + { + return; + } + if (disposing) { + _disposed = true; if (Interlocked.Exchange(ref _bufferRefCount, 0) == 1) { ArrayPool.Shared.Return(_buffer); } +#if NET6_0_OR_GREATER + _pipeReader?.Complete(); +#endif + _inner.Dispose(); } base.Dispose(disposing); } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public new async ValueTask DisposeAsync() + { + if (_disposed) + { + return; + } + + _disposed = true; + if (Interlocked.Exchange(ref _bufferRefCount, 0) == 1) + { + ArrayPool.Shared.Return(_buffer); + } + +#if NET6_0_OR_GREATER + if (_pipeReader != null) + { + await _pipeReader.CompleteAsync().ConfigureAwait(false); + } +#endif + + await _inner.DisposeAsync().ConfigureAwait(false); + GC.SuppressFinalize(this); + } +#endif + public override long Seek(long offset, SeekOrigin origin) { throw new NotSupportedException(); @@ -102,6 +158,13 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati return _inner.WriteAsync(buffer, offset, count, cancellationToken); } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) + { + return _inner.WriteAsync(buffer, cancellationToken); + } +#endif + public override int Read(byte[] buffer, int offset, int count) { int read = ReadBuffer(buffer, offset, count); @@ -124,6 +187,33 @@ public override Task ReadAsync(byte[] buffer, int offset, int count, Cancel return _inner.ReadAsync(buffer, offset, count, cancellationToken); } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + int read = ReadBufferMemory(buffer.Span); + if (read > 0) + { + return new ValueTask(read); + } + + return _inner.ReadAsync(buffer, cancellationToken); + } + + private int ReadBufferMemory(Span destination) + { + if (_bufferCount > 0) + { + int toCopy = Math.Min(_bufferCount, destination.Length); + _buffer.AsSpan(_bufferOffset, toCopy).CopyTo(destination); + _bufferOffset += toCopy; + _bufferCount -= toCopy; + return toCopy; + } + + return 0; + } +#endif + public override void CloseWrite() { if (_socket != null) @@ -158,6 +248,93 @@ public bool Peek(byte[] buffer, uint toPeek, out uint peeked, out uint available } public async Task ReadLineAsync(CancellationToken cancellationToken) + { +#if NET6_0_OR_GREATER + // Try PipeReader-based implementation first (more efficient) + if (_usePipeReader && !_pipeReaderFailed) + { + try + { + return await ReadLineWithPipeReaderAsync(cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + // Fall back to byte-by-byte on failure + _pipeReaderFailed = true; + _logger.LogWarning(ex, "PipeReader line reading failed, falling back to byte-by-byte implementation"); + } + } +#endif + + return await ReadLineByteByByteAsync(cancellationToken).ConfigureAwait(false); + } + +#if NET6_0_OR_GREATER + private async Task ReadLineWithPipeReaderAsync(CancellationToken cancellationToken) + { + // Create PipeReader lazily + if (_pipeReader == null) + { + // First, consume any buffered data + if (_bufferCount > 0) + { + // We have buffered data, fall back to byte-by-byte for this call + // to avoid complexity of merging buffer with PipeReader + return await ReadLineByteByByteAsync(cancellationToken).ConfigureAwait(false); + } + + _pipeReader = PipeReader.Create(_inner, new StreamPipeReaderOptions(leaveOpen: true)); + } + + while (true) + { + var result = await _pipeReader.ReadAsync(cancellationToken).ConfigureAwait(false); + var buffer = result.Buffer; + + // Look for CRLF in the buffer + var position = buffer.PositionOf((byte)'\n'); + + if (position != null) + { + // Found newline - extract line up to it + var lineBuffer = buffer.Slice(0, position.Value); + var lineBytes = lineBuffer.ToArray(); + + // Remove trailing CR if present + var lineLength = lineBytes.Length; + if (lineLength > 0 && lineBytes[lineLength - 1] == '\r') + { + lineLength--; + } + + var line = Encoding.ASCII.GetString(lineBytes, 0, lineLength); + + // Tell the PipeReader we've consumed up to and including the newline + _pipeReader.AdvanceTo(buffer.GetPosition(1, position.Value)); + + return line; + } + + if (result.IsCompleted) + { + if (buffer.Length > 0) + { + // Return remaining data as the last line + var line = Encoding.ASCII.GetString(buffer.ToArray()); + _pipeReader.AdvanceTo(buffer.End); + return line; + } + + throw new EndOfStreamException("Unexpected end of stream while reading line"); + } + + // Tell the reader we need more data + _pipeReader.AdvanceTo(buffer.Start, buffer.End); + } + } +#endif + + private async Task ReadLineByteByByteAsync(CancellationToken cancellationToken) { var line = new StringBuilder(32); diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs index e4991829..3bfdf8af 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs @@ -1,10 +1,14 @@ namespace Microsoft.Net.Http.Client; internal sealed class ChunkedReadStream : Stream +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + , IAsyncDisposable +#endif { private readonly BufferedReadStream _inner; private int _chunkBytesRemaining; private bool _done; + private bool _disposed; public ChunkedReadStream(BufferedReadStream stream) { @@ -122,6 +126,59 @@ public override async Task ReadAsync(byte[] buffer, int offset, int count, return readBytesCount; } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + if (_done) + { + return 0; + } + + if (_chunkBytesRemaining == 0) + { + var headerLine = await _inner.ReadLineAsync(cancellationToken) + .ConfigureAwait(false); + + if (!int.TryParse(headerLine, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out _chunkBytesRemaining)) + { + throw new IOException($"Invalid chunk header encountered: '{headerLine}'."); + } + } + + var readBytesCount = 0; + + if (_chunkBytesRemaining > 0) + { + var remainingBytesCount = Math.Min(_chunkBytesRemaining, buffer.Length); + + readBytesCount = await _inner.ReadAsync(buffer.Slice(0, remainingBytesCount), cancellationToken) + .ConfigureAwait(false); + + if (readBytesCount == 0) + { + throw new EndOfStreamException(); + } + + _chunkBytesRemaining -= readBytesCount; + } + + if (_chunkBytesRemaining == 0) + { + var emptyLine = await _inner.ReadLineAsync(cancellationToken) + .ConfigureAwait(false); + + if (!string.IsNullOrEmpty(emptyLine)) + { + throw new IOException($"Expected an empty line, but received: '{emptyLine}'."); + } + + _done = readBytesCount == 0; + } + + return readBytesCount; + } +#endif + public override void Write(byte[] buffer, int offset, int count) { _inner.Write(buffer, offset, count); @@ -132,6 +189,13 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati return _inner.WriteAsync(buffer, offset, count, cancellationToken); } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) + { + return _inner.WriteAsync(buffer, cancellationToken); + } +#endif + public override long Seek(long offset, SeekOrigin origin) { throw new NotSupportedException(); @@ -146,4 +210,27 @@ public override void Flush() { _inner.Flush(); } + + protected override void Dispose(bool disposing) + { + if (!_disposed && disposing) + { + _disposed = true; + // Note: We don't dispose _inner here as it's owned by HttpConnection + } + base.Dispose(disposing); + } + +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public new ValueTask DisposeAsync() + { + if (!_disposed) + { + _disposed = true; + // Note: We don't dispose _inner here as it's owned by HttpConnection + } + GC.SuppressFinalize(this); + return default; + } +#endif } \ No newline at end of file diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs index 4a1ca6c0..f46e8071 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs @@ -1,10 +1,14 @@ namespace Microsoft.Net.Http.Client; internal sealed class ChunkedWriteStream : Stream +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + , IAsyncDisposable +#endif { private static readonly byte[] EndOfContentBytes = Encoding.ASCII.GetBytes("0\r\n\r\n"); private readonly Stream _inner; + private bool _disposed; public ChunkedWriteStream(Stream stream) { @@ -87,4 +91,54 @@ public Task EndContentAsync(CancellationToken cancellationToken) { return _inner.WriteAsync(EndOfContentBytes, 0, EndOfContentBytes.Length, cancellationToken); } + +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override async ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) + { + if (buffer.Length == 0) + { + return; + } + + const string crlf = "\r\n"; + + var chunkHeader = buffer.Length.ToString("X") + crlf; + var headerBytes = Encoding.ASCII.GetBytes(chunkHeader); + + // Write the chunk header + await _inner.WriteAsync(headerBytes.AsMemory(), cancellationToken) + .ConfigureAwait(false); + + // Write the chunk data + await _inner.WriteAsync(buffer, cancellationToken) + .ConfigureAwait(false); + + // Write the chunk footer (CRLF) + await _inner.WriteAsync(headerBytes.AsMemory(headerBytes.Length - 2, 2), cancellationToken) + .ConfigureAwait(false); + } +#endif + + protected override void Dispose(bool disposing) + { + if (!_disposed && disposing) + { + _disposed = true; + // Note: We don't dispose _inner here as it's owned by the caller + } + base.Dispose(disposing); + } + +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public new ValueTask DisposeAsync() + { + if (!_disposed) + { + _disposed = true; + // Note: We don't dispose _inner here as it's owned by the caller + } + GC.SuppressFinalize(this); + return default; + } +#endif } \ No newline at end of file diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs new file mode 100644 index 00000000..4fa672d4 --- /dev/null +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs @@ -0,0 +1,296 @@ +#if NET5_0_OR_GREATER +namespace Microsoft.Net.Http.Client; + +/// +/// Manages a pool of HTTP connections for reuse. +/// +internal sealed class ConnectionPool : IDisposable +{ + private readonly ConcurrentDictionary> _pools = new(); + private readonly ILogger _logger; + private readonly Timer _cleanupTimer; + private bool _disposed; + + /// + /// Gets or sets the maximum time a connection can remain idle in the pool before being removed. + /// Default is 2 minutes. + /// + public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromMinutes(2); + + /// + /// Gets or sets the maximum lifetime of a connection, regardless of activity. + /// Default is 10 minutes. + /// + public TimeSpan ConnectionLifetime { get; set; } = TimeSpan.FromMinutes(10); + + /// + /// Gets or sets the maximum number of connections per host. + /// Default is 10. + /// + public int MaxConnectionsPerHost { get; set; } = 10; + + public ConnectionPool(ILogger logger) + { + _logger = logger; + // Run cleanup every 30 seconds + _cleanupTimer = new Timer(CleanupCallback, null, TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(30)); + } + + /// + /// Attempts to get a pooled connection for the specified host and port. + /// + /// The host name. + /// The port number. + /// Whether the connection uses HTTPS. + /// The pooled connection if found. + /// True if a valid pooled connection was found; otherwise, false. + public bool TryGetConnection(string host, int port, bool isHttps, out PooledConnection connection) + { + connection = null; + + if (_disposed) + { + return false; + } + + var key = GetPoolKey(host, port, isHttps); + + if (!_pools.TryGetValue(key, out var pool)) + { + return false; + } + + while (pool.TryDequeue(out var candidate)) + { + // Check if connection is still valid + if (IsConnectionValid(candidate)) + { + connection = candidate; + _logger.LogDebug("Reusing pooled connection to {Host}:{Port}", host, port); + return true; + } + + // Connection is no longer valid, dispose it + try + { + candidate.Dispose(); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error disposing invalid pooled connection"); + } + } + + return false; + } + + /// + /// Returns a connection to the pool for reuse. + /// + /// The connection to return. + /// True if the connection was returned to the pool; otherwise, false. + public bool ReturnConnection(PooledConnection connection) + { + if (_disposed || connection == null) + { + connection?.Dispose(); + return false; + } + + // Check if connection is still valid for reuse + if (!IsConnectionValid(connection)) + { + _logger.LogDebug("Connection not valid for pooling, disposing"); + connection.Dispose(); + return false; + } + + var key = connection.PoolKey; + var pool = _pools.GetOrAdd(key, _ => new ConcurrentQueue()); + + // Check pool size limit + if (pool.Count >= MaxConnectionsPerHost) + { + _logger.LogDebug("Pool for {Key} is full ({Count}/{Max}), disposing connection", + key, pool.Count, MaxConnectionsPerHost); + connection.Dispose(); + return false; + } + + connection.LastUsed = DateTime.UtcNow; + pool.Enqueue(connection); + _logger.LogDebug("Returned connection to pool for {Key}", key); + return true; + } + + /// + /// Creates a new pooled connection wrapper. + /// + public PooledConnection CreatePooledConnection( + string host, + int port, + bool isHttps, + Socket socket, + Stream transport, + BufferedReadStream bufferedStream) + { + var key = GetPoolKey(host, port, isHttps); + return new PooledConnection(key, socket, transport, bufferedStream); + } + + private bool IsConnectionValid(PooledConnection connection) + { + // Check lifetime + if (DateTime.UtcNow - connection.Created > ConnectionLifetime) + { + return false; + } + + // Check idle timeout + if (DateTime.UtcNow - connection.LastUsed > IdleTimeout) + { + return false; + } + + // Check if socket is still connected + if (connection.Socket != null) + { + try + { + // Poll with zero timeout to check socket state + // SelectRead with available=0 means the connection was closed gracefully + if (connection.Socket.Poll(0, SelectMode.SelectRead)) + { + if (connection.Socket.Available == 0) + { + return false; // Connection was closed + } + } + } + catch + { + return false; + } + } + + return true; + } + + private static string GetPoolKey(string host, int port, bool isHttps) + { + return $"{(isHttps ? "https" : "http")}://{host}:{port}"; + } + + private void CleanupCallback(object state) + { + if (_disposed) + { + return; + } + + foreach (var kvp in _pools) + { + var pool = kvp.Value; + var validConnections = new List(); + + while (pool.TryDequeue(out var connection)) + { + if (IsConnectionValid(connection)) + { + validConnections.Add(connection); + } + else + { + try + { + connection.Dispose(); + _logger.LogDebug("Cleaned up expired connection from pool {Key}", kvp.Key); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error disposing expired pooled connection"); + } + } + } + + // Re-add valid connections + foreach (var conn in validConnections) + { + pool.Enqueue(conn); + } + } + } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + _cleanupTimer.Dispose(); + + foreach (var kvp in _pools) + { + while (kvp.Value.TryDequeue(out var connection)) + { + try + { + connection.Dispose(); + } + catch + { + // Ignore cleanup errors + } + } + } + + _pools.Clear(); + } +} + +/// +/// Represents a pooled connection that can be returned to the pool for reuse. +/// +internal sealed class PooledConnection : IDisposable +{ + private bool _disposed; + + public PooledConnection(string poolKey, Socket socket, Stream transport, BufferedReadStream bufferedStream) + { + PoolKey = poolKey; + Socket = socket; + Transport = transport; + BufferedStream = bufferedStream; + Created = DateTime.UtcNow; + LastUsed = DateTime.UtcNow; + } + + public string PoolKey { get; } + public Socket Socket { get; } + public Stream Transport { get; } + public BufferedReadStream BufferedStream { get; } + public DateTime Created { get; } + public DateTime LastUsed { get; set; } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + + try + { + BufferedStream?.Dispose(); + } + catch + { + // Ignore + } + } +} +#endif diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs index 2d38d85c..a030a7f6 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs @@ -1,6 +1,9 @@ namespace Microsoft.Net.Http.Client; internal class ContentLengthReadStream : Stream +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + , IAsyncDisposable +#endif { private readonly Stream _inner; private long _bytesRemaining; @@ -120,15 +123,49 @@ public override async Task ReadAsync(byte[] buffer, int offset, int count, return read; } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + if (_disposed) + { + return 0; + } + + if (_bytesRemaining == 0) + { + return 0; + } + + cancellationToken.ThrowIfCancellationRequested(); + int toRead = (int)Math.Min(buffer.Length, _bytesRemaining); + int read = await _inner.ReadAsync(buffer.Slice(0, toRead), cancellationToken).ConfigureAwait(false); + UpdateBytesRemaining(read); + return read; + } +#endif + protected override void Dispose(bool disposing) { - if (disposing) + if (disposing && !_disposed) { + _disposed = true; // TODO: Sync drain with timeout if small number of bytes remaining? This will let us re-use the connection. _inner.Dispose(); } } +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public new async ValueTask DisposeAsync() + { + if (!_disposed) + { + _disposed = true; + await _inner.DisposeAsync().ConfigureAwait(false); + } + GC.SuppressFinalize(this); + } +#endif + private void CheckDisposed() { if (_disposed) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs index 22972f8b..6b246bd1 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs @@ -1,6 +1,9 @@ namespace Microsoft.Net.Http.Client; internal sealed class HttpConnection : IDisposable +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + , IAsyncDisposable +#endif { private static readonly ISet DockerStreamHeaders = new HashSet{ "application/vnd.docker.raw-stream", "application/vnd.docker.multiplexed-stream" }; @@ -18,27 +21,39 @@ public async Task SendAsync(HttpRequestMessage request, Can // Serialize headers & send string rawRequest = SerializeRequest(request); byte[] requestBytes = Encoding.ASCII.GetBytes(rawRequest); - await Transport.WriteAsync(requestBytes, 0, requestBytes.Length, cancellationToken); +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + await Transport.WriteAsync(requestBytes.AsMemory(), cancellationToken).ConfigureAwait(false); +#else + await Transport.WriteAsync(requestBytes, 0, requestBytes.Length, cancellationToken).ConfigureAwait(false); +#endif if (request.Content != null) { if (request.Content.Headers.ContentLength.HasValue) { - await request.Content.CopyToAsync(Transport); +#if NET5_0_OR_GREATER + await request.Content.CopyToAsync(Transport, cancellationToken).ConfigureAwait(false); +#else + await request.Content.CopyToAsync(Transport).ConfigureAwait(false); +#endif } else { // The length of the data is unknown. Send it in chunked mode. using (var chunkedStream = new ChunkedWriteStream(Transport)) { - await request.Content.CopyToAsync(chunkedStream); - await chunkedStream.EndContentAsync(cancellationToken); +#if NET5_0_OR_GREATER + await request.Content.CopyToAsync(chunkedStream, cancellationToken).ConfigureAwait(false); +#else + await request.Content.CopyToAsync(chunkedStream).ConfigureAwait(false); +#endif + await chunkedStream.EndContentAsync(cancellationToken).ConfigureAwait(false); } } } // Receive headers - List responseLines = await ReadResponseLinesAsync(cancellationToken); + List responseLines = await ReadResponseLinesAsync(cancellationToken).ConfigureAwait(false); // Receive body and determine the response type (Content-Length, Transfer-Encoding, Opaque) return CreateResponseMessage(responseLines); @@ -190,4 +205,11 @@ public void Dispose() { Transport.Dispose(); } + +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + public ValueTask DisposeAsync() + { + return Transport.DisposeAsync(); + } +#endif } \ No newline at end of file diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs index 01a8ea16..f4c3cb24 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs @@ -4,12 +4,33 @@ public class HttpConnectionResponseContent : HttpContent { private readonly HttpConnection _connection; private Stream _responseStream; + private bool _hijacked; + +#if NET5_0_OR_GREATER + private PooledConnection _pooledConnection; + private Action _poolReturnCallback; +#endif internal HttpConnectionResponseContent(HttpConnection connection) { _connection = connection; } +#if NET5_0_OR_GREATER + internal HttpConnectionResponseContent(HttpConnection connection, PooledConnection pooledConnection, Action poolReturnCallback) + { + _connection = connection; + _pooledConnection = pooledConnection; + _poolReturnCallback = poolReturnCallback; + } + + internal void SetPoolReturnCallback(PooledConnection pooledConnection, Action poolReturnCallback) + { + _pooledConnection = pooledConnection; + _poolReturnCallback = poolReturnCallback; + } +#endif + internal void ResolveResponseStream(bool chunked, bool isConnectionUpgrade) { if (_responseStream != null) @@ -37,6 +58,14 @@ public WriteClosableStream HijackStream() throw new InvalidOperationException("cannot hijack chunked or content length stream"); } + _hijacked = true; + +#if NET5_0_OR_GREATER + // Hijacked connections cannot be returned to the pool + _poolReturnCallback = null; + _pooledConnection = null; +#endif + return _connection.Transport; } @@ -62,7 +91,31 @@ protected override void Dispose(bool disposing) { if (disposing) { - _responseStream.Dispose(); +#if NET5_0_OR_GREATER + // Try to return connection to pool if not hijacked + if (!_hijacked && _poolReturnCallback != null && _pooledConnection != null) + { + try + { + // Drain any remaining response data before returning to pool + // Only attempt for content-length streams where we know the remaining bytes + if (_responseStream is ContentLengthReadStream) + { + // Pool return callback will handle disposal if pooling fails + _poolReturnCallback(_pooledConnection); + _pooledConnection = null; + _poolReturnCallback = null; + return; // Don't dispose the connection, it's returned to pool + } + } + catch + { + // Fall through to normal disposal + } + } +#endif + + _responseStream?.Dispose(); _connection.Dispose(); } } diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index 289a5393..d5d67026 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -15,6 +15,13 @@ public class ManagedHandler : HttpMessageHandler private IWebProxy _proxy; + private RetryPolicy _retryPolicy = RetryPolicy.Default; + +#if NET5_0_OR_GREATER + private readonly ConnectionPool _connectionPool; + private bool _enableConnectionPooling = true; +#endif + public delegate Task StreamOpener(string host, int port, CancellationToken cancellationToken); public delegate Task SocketOpener(string host, int port, CancellationToken cancellationToken); @@ -29,6 +36,9 @@ public ManagedHandler(ILogger logger, SocketConfiguration socketConfiguration) _logger = logger; _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = TcpSocketOpenerAsync; +#if NET5_0_OR_GREATER + _connectionPool = new ConnectionPool(logger); +#endif } public ManagedHandler(StreamOpener opener, ILogger logger) @@ -36,6 +46,9 @@ public ManagedHandler(StreamOpener opener, ILogger logger) _logger = logger; _socketConfiguration = SocketConfiguration.Default; _streamOpener = opener ?? throw new ArgumentNullException(nameof(opener)); +#if NET5_0_OR_GREATER + _connectionPool = new ConnectionPool(logger); +#endif } public ManagedHandler(SocketOpener opener, ILogger logger) @@ -43,6 +56,9 @@ public ManagedHandler(SocketOpener opener, ILogger logger) _logger = logger; _socketConfiguration = SocketConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); +#if NET5_0_OR_GREATER + _connectionPool = new ConnectionPool(logger); +#endif } public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration socketConfiguration) @@ -50,6 +66,9 @@ public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration s _logger = logger; _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); +#if NET5_0_OR_GREATER + _connectionPool = new ConnectionPool(logger); +#endif } public IWebProxy Proxy @@ -58,7 +77,12 @@ public IWebProxy Proxy { if (_proxy == null) { +#if NET6_0_OR_GREATER + // Use modern HttpClient.DefaultProxy for better proxy resolution + _proxy = HttpClient.DefaultProxy; +#else _proxy = WebRequest.DefaultWebProxy; +#endif } return _proxy; @@ -79,6 +103,41 @@ public IWebProxy Proxy public X509CertificateCollection ClientCertificates { get; set; } = new X509Certificate2Collection(); + /// + /// Gets or sets the connection timeout. Default is 30 seconds. + /// Set to to disable connection timeout. + /// + public TimeSpan ConnectTimeout { get; set; } = TimeSpan.FromSeconds(30); + + /// + /// Gets or sets the retry policy for transient connection failures. + /// Set to to disable retries. + /// Default uses exponential backoff with 3 retries. + /// + public RetryPolicy RetryPolicy + { + get => _retryPolicy; + set => _retryPolicy = value ?? RetryPolicy.NoRetry; + } + +#if NET5_0_OR_GREATER + /// + /// Gets or sets whether connection pooling is enabled. + /// Default is true. When enabled, connections are reused for subsequent requests. + /// Connection pooling is automatically disabled for hijacking requests (attach/exec). + /// + public bool EnableConnectionPooling + { + get => _enableConnectionPooling; + set => _enableConnectionPooling = value; + } + + /// + /// Gets the connection pool used by this handler. + /// + internal ConnectionPool ConnectionPool => _connectionPool; +#endif + protected override async Task SendAsync(HttpRequestMessage httpRequestMessage, CancellationToken cancellationToken) { if (httpRequestMessage == null) @@ -159,45 +218,215 @@ private async Task ProcessRequestAsync(HttpRequestMessage r ProcessUrl(request); ProcessHostHeader(request); + + // Check if this is a hijacking request (attach/exec) - these cannot use pooled connections + var isHijackingRequest = IsHijackingRequest(request); + +#if NET5_0_OR_GREATER + // For non-hijacking requests with pooling enabled, try to get a pooled connection + if (_enableConnectionPooling && !isHijackingRequest) + { + request.Headers.ConnectionClose = false; // Keep connection alive for pooling + } + else + { + request.Headers.ConnectionClose = !request.Headers.Contains("Connection"); + } +#else request.Headers.ConnectionClose = !request.Headers.Contains("Connection"); // TODO: Connection reuse is not supported. +#endif ProxyMode proxyMode = DetermineProxyModeAndAddressLine(request); - Socket socket; - Stream transport; + Socket socket = null; + Stream transport = null; + BufferedReadStream bufferedReadStream = null; - try +#if NET5_0_OR_GREATER + // Try to get a pooled connection first + bool usePooledConnection = false; + PooledConnection pooledConnection = null; + + if (_enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None) { - if (_socketOpener != null) + try { - socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, cancellationToken).ConfigureAwait(false); - transport = new NetworkStream(socket, true); + if (_connectionPool.TryGetConnection( + request.GetConnectionHostProperty(), + request.GetConnectionPortProperty().Value, + request.IsHttps(), + out pooledConnection)) + { + socket = pooledConnection.Socket; + transport = pooledConnection.Transport; + bufferedReadStream = pooledConnection.BufferedStream; + usePooledConnection = true; + _logger.LogDebug("Using pooled connection for {Host}:{Port}", + request.GetConnectionHostProperty(), request.GetConnectionPortProperty()); + } } - else + catch (Exception ex) { - socket = null; - transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, cancellationToken).ConfigureAwait(false); + _logger.LogWarning(ex, "Error getting pooled connection, creating new connection"); } } - catch (SocketException e) + + if (!usePooledConnection) { - throw new HttpRequestException("Connection failed.", e); +#endif + // Create linked cancellation token for connection timeout + using var timeoutCts = ConnectTimeout != Timeout.InfiniteTimeSpan + ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken) + : null; + + if (timeoutCts != null) + { + timeoutCts.CancelAfter(ConnectTimeout); + } + + var effectiveCancellationToken = timeoutCts?.Token ?? cancellationToken; + + try + { + if (_socketOpener != null) + { + socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); + transport = new NetworkStream(socket, true); + } + else + { + socket = null; + transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); + } + } + catch (OperationCanceledException) when (timeoutCts?.IsCancellationRequested == true && !cancellationToken.IsCancellationRequested) + { + throw new TimeoutException($"Connection to {request.GetConnectionHostProperty()}:{request.GetConnectionPortProperty()} timed out after {ConnectTimeout.TotalSeconds} seconds."); + } + catch (SocketException e) + { + throw new HttpRequestException("Connection failed.", e); + } + + if (proxyMode == ProxyMode.Tunnel) + { + await TunnelThroughProxyAsync(request, transport, cancellationToken); + } + + if (request.IsHttps()) + { + transport = await EstablishSslAsync(transport, request.GetHostProperty(), cancellationToken).ConfigureAwait(false); + } + + bufferedReadStream = new BufferedReadStream(transport, socket, _logger); +#if NET5_0_OR_GREATER } +#endif + + var connection = new HttpConnection(bufferedReadStream); - if (proxyMode == ProxyMode.Tunnel) +#if NET5_0_OR_GREATER + // Create pool return callback for non-hijacking requests + Action poolReturnCallback = null; + if (_enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None) { - await TunnelThroughProxyAsync(request, transport, cancellationToken); + var host = request.GetConnectionHostProperty(); + var port = request.GetConnectionPortProperty().Value; + var isHttps = request.IsHttps(); + + poolReturnCallback = pooledConn => + { + try + { + if (!_connectionPool.ReturnConnection(pooledConn)) + { + pooledConn.Dispose(); + } + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error returning connection to pool"); + pooledConn?.Dispose(); + } + }; } - if (request.IsHttps()) + var responseContent = new HttpConnectionResponseContent( + connection, + _enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None + ? _connectionPool.CreatePooledConnection( + request.GetConnectionHostProperty(), + request.GetConnectionPortProperty().Value, + request.IsHttps(), + socket, + transport, + bufferedReadStream) + : null, + poolReturnCallback); + + var response = await connection.SendAsync(request, cancellationToken); + + // Replace the content with our pooling-aware content + var originalContent = response.Content as HttpConnectionResponseContent; + if (originalContent != null && poolReturnCallback != null) { - SslStream sslStream = new SslStream(transport, false, ServerCertificateValidationCallback); - await sslStream.AuthenticateAsClientAsync(request.GetHostProperty(), ClientCertificates, SslProtocols.Tls12, false); - transport = sslStream; + originalContent.SetPoolReturnCallback( + _connectionPool.CreatePooledConnection( + request.GetConnectionHostProperty(), + request.GetConnectionPortProperty().Value, + request.IsHttps(), + socket, + transport, + bufferedReadStream), + poolReturnCallback); } - var bufferedReadStream = new BufferedReadStream(transport, socket, _logger); - var connection = new HttpConnection(bufferedReadStream); + return response; +#else return await connection.SendAsync(request, cancellationToken); +#endif + } + + private static bool IsHijackingRequest(HttpRequestMessage request) + { + // Check for requests that require connection hijacking + // These are typically attach/exec operations that upgrade the connection + var pathAndQuery = request.GetPathAndQueryProperty() ?? request.RequestUri?.PathAndQuery ?? string.Empty; + + return pathAndQuery.Contains("/attach") || + pathAndQuery.Contains("/exec/") || + request.Headers.TryGetValues("Upgrade", out var upgradeValues) && + upgradeValues.Any(v => "tcp".Equals(v, StringComparison.OrdinalIgnoreCase)); + } + + private async Task EstablishSslAsync(Stream transport, string targetHost, CancellationToken cancellationToken) + { + var sslStream = new SslStream(transport, false, ServerCertificateValidationCallback); + + try + { +#if NET5_0_OR_GREATER + // Use modern SslClientAuthenticationOptions for better TLS configuration + var sslOptions = new SslClientAuthenticationOptions + { + TargetHost = targetHost, + ClientCertificates = ClientCertificates, + // Let the OS choose the best protocol (TLS 1.2/1.3) + EnabledSslProtocols = SslProtocols.None, + CertificateRevocationCheckMode = X509RevocationMode.NoCheck + }; + + await sslStream.AuthenticateAsClientAsync(sslOptions, cancellationToken).ConfigureAwait(false); +#else + // Fallback for older frameworks - use TLS 1.2 as minimum + await sslStream.AuthenticateAsClientAsync(targetHost, ClientCertificates, SslProtocols.Tls12, checkCertificateRevocation: false).ConfigureAwait(false); +#endif + return sslStream; + } + catch + { + sslStream.Dispose(); + throw; + } } // Data comes from either the request.RequestUri or from the request.Properties @@ -333,7 +562,14 @@ private ProxyMode DetermineProxyModeAndAddressLine(HttpRequestMessage request) return ProxyMode.Tunnel; } - private async Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) + private Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) + { + return _retryPolicy.ExecuteAsync( + ct => TcpSocketOpenerCoreAsync(host, port, ct), + cancellationToken); + } + + private async Task TcpSocketOpenerCoreAsync(string host, int port, CancellationToken cancellationToken) { #if NET5_0_OR_GREATER // Use modern DNS resolution with cancellation support @@ -349,6 +585,27 @@ private async Task TcpSocketOpenerAsync(string host, int port, Cancellat throw new Exception($"Unable to resolve any IP addresses for the host '{host}'."); } +#if NET6_0_OR_GREATER + // Use Happy Eyeballs if enabled and we have both IPv6 and IPv4 addresses + if (_socketConfiguration.EnableHappyEyeballs) + { + var ipv6Addresses = addresses.Where(a => a.AddressFamily == AddressFamily.InterNetworkV6).ToArray(); + var ipv4Addresses = addresses.Where(a => a.AddressFamily == AddressFamily.InterNetwork).ToArray(); + + if (ipv6Addresses.Length > 0 && ipv4Addresses.Length > 0) + { + return await ConnectWithHappyEyeballsAsync(ipv6Addresses, ipv4Addresses, port, cancellationToken) + .ConfigureAwait(false); + } + } +#endif + + // Fallback to sequential connection attempts + return await ConnectSequentialAsync(addresses, port, cancellationToken).ConfigureAwait(false); + } + + private async Task ConnectSequentialAsync(IPAddress[] addresses, int port, CancellationToken cancellationToken) + { var exceptions = new List(); foreach (var address in addresses) @@ -381,6 +638,136 @@ await socket.ConnectAsync(address, port) throw new AggregateException(exceptions); } +#if NET6_0_OR_GREATER + private async Task ConnectWithHappyEyeballsAsync( + IPAddress[] ipv6Addresses, + IPAddress[] ipv4Addresses, + int port, + CancellationToken cancellationToken) + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + var exceptions = new ConcurrentBag(); + Socket winningSocket = null; + + // Start IPv6 connection attempts + var ipv6Task = Task.Run(async () => + { + try + { + return await ConnectToAddressesAsync(ipv6Addresses, port, cts.Token).ConfigureAwait(false); + } + catch (Exception ex) + { + exceptions.Add(ex); + return null; + } + }, cts.Token); + + // Start IPv4 connection after delay (Happy Eyeballs delay) + var ipv4Task = Task.Run(async () => + { + try + { + await Task.Delay(_socketConfiguration.HappyEyeballsDelay, cts.Token).ConfigureAwait(false); + + // Only proceed if IPv6 hasn't connected yet + if (!ipv6Task.IsCompleted) + { + return await ConnectToAddressesAsync(ipv4Addresses, port, cts.Token).ConfigureAwait(false); + } + + return null; + } + catch (OperationCanceledException) + { + return null; + } + catch (Exception ex) + { + exceptions.Add(ex); + return null; + } + }, cts.Token); + + // Wait for either to complete + var completedTasks = new List> { ipv6Task, ipv4Task }; + + while (completedTasks.Count > 0) + { + var completedTask = await Task.WhenAny(completedTasks).ConfigureAwait(false); + completedTasks.Remove(completedTask); + + try + { + var socket = await completedTask.ConfigureAwait(false); + if (socket != null && socket.Connected) + { + winningSocket = socket; + cts.Cancel(); // Cancel the other connection attempt + break; + } + } + catch (Exception ex) + { + exceptions.Add(ex); + } + } + + // Clean up any remaining sockets from tasks that didn't win + foreach (var task in completedTasks) + { + try + { + if (task.IsCompletedSuccessfully) + { + var socket = await task.ConfigureAwait(false); + socket?.Dispose(); + } + } + catch + { + // Ignore cleanup errors + } + } + + if (winningSocket != null) + { + return winningSocket; + } + + throw new AggregateException("Failed to connect using Happy Eyeballs.", exceptions); + } + + private async Task ConnectToAddressesAsync(IPAddress[] addresses, int port, CancellationToken cancellationToken) + { + foreach (var address in addresses) + { + cancellationToken.ThrowIfCancellationRequested(); + + var socket = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + + try + { + _socketConfiguration.ApplyTo(socket); + await socket.ConnectAsync(address, port, cancellationToken).ConfigureAwait(false); + return socket; + } + catch (OperationCanceledException) + { + socket.Dispose(); + throw; + } + catch + { + socket.Dispose(); + // Try next address + } + } + + return null; + } +#endif + private async Task TunnelThroughProxyAsync(HttpRequestMessage request, Stream transport, CancellationToken cancellationToken) { // Send a Connect request: @@ -417,4 +804,15 @@ private async Task TunnelThroughProxyAsync(HttpRequestMessage request, Stream tr throw new HttpRequestException("Failed to negotiate the proxy tunnel: " + connectResponse); } } + + protected override void Dispose(bool disposing) + { + if (disposing) + { +#if NET5_0_OR_GREATER + _connectionPool?.Dispose(); +#endif + } + base.Dispose(disposing); + } } \ No newline at end of file diff --git a/src/Docker.DotNet/RetryPolicy.cs b/src/Docker.DotNet/RetryPolicy.cs new file mode 100644 index 00000000..20466839 --- /dev/null +++ b/src/Docker.DotNet/RetryPolicy.cs @@ -0,0 +1,194 @@ +namespace Docker.DotNet; + +/// +/// Configures retry behavior for transient connection failures. +/// +public sealed class RetryPolicy +{ + private static readonly Random Jitter = new Random(); + + /// + /// Gets the default retry policy with sensible defaults. + /// + public static RetryPolicy Default { get; } = new RetryPolicy(); + + /// + /// Gets a retry policy that disables retries. + /// + public static RetryPolicy NoRetry { get; } = new RetryPolicy { MaxRetries = 0 }; + + /// + /// Gets or sets the maximum number of retry attempts. Default is 3. + /// Set to 0 to disable retries. + /// + public int MaxRetries { get; set; } = 3; + + /// + /// Gets or sets the initial delay before the first retry. Default is 100ms. + /// + public TimeSpan InitialDelay { get; set; } = TimeSpan.FromMilliseconds(100); + + /// + /// Gets or sets the maximum delay between retries. Default is 10 seconds. + /// + public TimeSpan MaxDelay { get; set; } = TimeSpan.FromSeconds(10); + + /// + /// Gets or sets the jitter factor to add randomness to delays. Default is 0.2 (20%). + /// This helps prevent thundering herd problems when multiple clients retry simultaneously. + /// + public double JitterFactor { get; set; } = 0.2; + + /// + /// Gets or sets the backoff multiplier for exponential backoff. Default is 2.0. + /// + public double BackoffMultiplier { get; set; } = 2.0; + + /// + /// Determines if the specified socket exception represents a transient error that can be retried. + /// + /// The socket exception to evaluate. + /// True if the error is transient and the operation can be retried; otherwise, false. + public bool IsTransientError(SocketException exception) + { + if (exception == null) + { + return false; + } + + // Transient socket errors that are safe to retry + return exception.SocketErrorCode switch + { + // Connection refused - server might not be ready yet + SocketError.ConnectionRefused => true, + + // Connection reset - connection was forcibly closed + SocketError.ConnectionReset => true, + + // Host unreachable - temporary network issue + SocketError.HostUnreachable => true, + + // Network unreachable - temporary network issue + SocketError.NetworkUnreachable => true, + + // Timed out - might succeed on retry + SocketError.TimedOut => true, + + // Try again - explicit retry suggestion + SocketError.TryAgain => true, + + // Host not found - DNS might be propagating + SocketError.HostNotFound => true, + + // No buffer space available - temporary resource constraint + SocketError.NoBufferSpaceAvailable => true, + + // Too many open sockets - temporary resource constraint + SocketError.TooManyOpenSockets => true, + + // Connection aborted - connection was aborted by the network + SocketError.ConnectionAborted => true, + + // All other errors are not considered transient + _ => false + }; + } + + /// + /// Determines if the specified exception represents a transient error that can be retried. + /// + /// The exception to evaluate. + /// True if the error is transient and the operation can be retried; otherwise, false. + public bool IsTransientError(Exception exception) + { + // Check for SocketException directly + if (exception is SocketException socketEx) + { + return IsTransientError(socketEx); + } + + // Check for SocketException in inner exception + if (exception.InnerException is SocketException innerSocketEx) + { + return IsTransientError(innerSocketEx); + } + + // IOException can wrap socket errors + if (exception is IOException ioEx && ioEx.InnerException is SocketException ioSocketEx) + { + return IsTransientError(ioSocketEx); + } + + // TimeoutException is transient + if (exception is TimeoutException) + { + return true; + } + + return false; + } + + /// + /// Calculates the delay before the next retry attempt using exponential backoff with jitter. + /// + /// The current retry attempt number (1-based). + /// The delay before the next retry. + public TimeSpan CalculateDelay(int retryAttempt) + { + if (retryAttempt < 1) + { + return InitialDelay; + } + + // Calculate exponential backoff + var exponentialDelay = InitialDelay.TotalMilliseconds * Math.Pow(BackoffMultiplier, retryAttempt - 1); + + // Cap at max delay + var cappedDelay = Math.Min(exponentialDelay, MaxDelay.TotalMilliseconds); + + // Add jitter + double jitterAmount; + lock (Jitter) + { + jitterAmount = Jitter.NextDouble() * JitterFactor * 2 - JitterFactor; // Range: [-JitterFactor, +JitterFactor] + } + var delayWithJitter = cappedDelay * (1 + jitterAmount); + + // Ensure delay is not negative + return TimeSpan.FromMilliseconds(Math.Max(0, delayWithJitter)); + } + + /// + /// Executes an async operation with retry logic. + /// + /// The return type of the operation. + /// The operation to execute. + /// Cancellation token. + /// The result of the operation. + /// Thrown when all retry attempts fail. + public async Task ExecuteAsync(Func> operation, CancellationToken cancellationToken) + { + if (MaxRetries == 0) + { + return await operation(cancellationToken).ConfigureAwait(false); + } + + var exceptions = new List(); + + for (int attempt = 0; attempt <= MaxRetries; attempt++) + { + try + { + return await operation(cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (attempt < MaxRetries && !cancellationToken.IsCancellationRequested && IsTransientError(ex)) + { + exceptions.Add(ex); + var delay = CalculateDelay(attempt + 1); + await Task.Delay(delay, cancellationToken).ConfigureAwait(false); + } + } + + throw new AggregateException($"Operation failed after {MaxRetries + 1} attempts.", exceptions); + } +} diff --git a/src/Docker.DotNet/SocketConfiguration.cs b/src/Docker.DotNet/SocketConfiguration.cs index 95d4d2be..627b2be0 100644 --- a/src/Docker.DotNet/SocketConfiguration.cs +++ b/src/Docker.DotNet/SocketConfiguration.cs @@ -73,6 +73,26 @@ public sealed class SocketConfiguration /// public int? ReceiveBufferSize { get; set; } +#if NET6_0_OR_GREATER + /// + /// Gets or sets whether Happy Eyeballs (RFC 8305) is enabled for dual-stack hosts. + /// When enabled, IPv6 and IPv4 connections are raced to find the fastest route. + /// Default is true. + /// + /// + /// Happy Eyeballs improves connection times on dual-stack networks by attempting + /// IPv6 first, then starting an IPv4 connection after a short delay if IPv6 + /// hasn't connected yet, and using whichever connects first. + /// + public bool EnableHappyEyeballs { get; set; } = true; + + /// + /// Gets or sets the delay before starting an IPv4 connection when Happy Eyeballs is enabled. + /// Default is 250ms as recommended by RFC 8305. + /// + public TimeSpan HappyEyeballsDelay { get; set; } = TimeSpan.FromMilliseconds(250); +#endif + /// /// Applies the configuration to a socket. /// diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs new file mode 100644 index 00000000..780737c2 --- /dev/null +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -0,0 +1,504 @@ +namespace Docker.DotNet.Tests; + +using System.IO; +using System.Net.Sockets; +using System.Text; +using Microsoft.Net.Http.Client; +using Xunit; + +/// +/// Tests for ManagedHandler modernization features. +/// +public class ManagedHandlerTests +{ + #region Phase 1: Memory-Efficient APIs & IAsyncDisposable Tests + + [Fact] + public async Task BufferedReadStream_ImplementsIAsyncDisposable() + { +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + // Arrange + var mockStream = new MemoryStream(Encoding.ASCII.GetBytes("Hello, World!")); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + var bufferedStream = new BufferedReadStream(mockStream, null, logger); + + // Act & Assert + Assert.True(bufferedStream is IAsyncDisposable); + await ((IAsyncDisposable)bufferedStream).DisposeAsync(); +#else + await Task.CompletedTask; +#endif + } + +#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER + [Fact] + public async Task BufferedReadStream_ReadAsyncMemory_ReadsFromBuffer() + { + // Arrange + var testData = "Hello, World!"u8.ToArray(); + var mockStream = new MemoryStream(testData); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + var bufferedStream = new BufferedReadStream(mockStream, null, logger); + + // Act + var buffer = new byte[5]; + var bytesRead = await bufferedStream.ReadAsync(buffer.AsMemory()); + + // Assert + Assert.Equal(5, bytesRead); + Assert.Equal("Hello", Encoding.ASCII.GetString(buffer)); + + await bufferedStream.DisposeAsync(); + } + + [Fact] + public async Task BufferedReadStream_WriteAsyncMemory_WritesToInner() + { + // Arrange + var outputStream = new MemoryStream(); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + var bufferedStream = new BufferedReadStream(outputStream, null, logger); + + // Act + var data = "Test Data"u8.ToArray(); + await bufferedStream.WriteAsync(data.AsMemory()); + + // Assert + outputStream.Position = 0; + var result = new byte[data.Length]; + await outputStream.ReadAsync(result.AsMemory()); + Assert.Equal("Test Data", Encoding.ASCII.GetString(result)); + + await bufferedStream.DisposeAsync(); + } +#endif + + #endregion + + #region Phase 2: Connection Timeout Tests + + [Fact] + public void ManagedHandler_ConnectTimeout_HasDefaultValue() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Assert + Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); + } + + [Fact] + public void ManagedHandler_ConnectTimeout_CanBeSet() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Act + handler.ConnectTimeout = TimeSpan.FromSeconds(60); + + // Assert + Assert.Equal(TimeSpan.FromSeconds(60), handler.ConnectTimeout); + } + + [Fact] + public void ManagedHandler_ConnectTimeout_CanBeDisabled() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Act + handler.ConnectTimeout = Timeout.InfiniteTimeSpan; + + // Assert + Assert.Equal(Timeout.InfiniteTimeSpan, handler.ConnectTimeout); + } + + #endregion + + #region Phase 3: Retry Policy Tests + + [Fact] + public void RetryPolicy_Default_HasSensibleDefaults() + { + // Arrange & Act + var policy = RetryPolicy.Default; + + // Assert + Assert.Equal(3, policy.MaxRetries); + Assert.Equal(TimeSpan.FromMilliseconds(100), policy.InitialDelay); + Assert.Equal(TimeSpan.FromSeconds(10), policy.MaxDelay); + Assert.Equal(0.2, policy.JitterFactor); + } + + [Fact] + public void RetryPolicy_NoRetry_DisablesRetries() + { + // Arrange & Act + var policy = RetryPolicy.NoRetry; + + // Assert + Assert.Equal(0, policy.MaxRetries); + } + + [Theory] + [InlineData(SocketError.ConnectionRefused, true)] + [InlineData(SocketError.ConnectionReset, true)] + [InlineData(SocketError.HostUnreachable, true)] + [InlineData(SocketError.NetworkUnreachable, true)] + [InlineData(SocketError.TimedOut, true)] + [InlineData(SocketError.TryAgain, true)] + [InlineData(SocketError.AccessDenied, false)] + [InlineData(SocketError.AddressAlreadyInUse, false)] + public void RetryPolicy_IsTransientError_ClassifiesSocketErrors(SocketError errorCode, bool expected) + { + // Arrange + var policy = new RetryPolicy(); + var exception = new SocketException((int)errorCode); + + // Act + var isTransient = policy.IsTransientError(exception); + + // Assert + Assert.Equal(expected, isTransient); + } + + [Theory] + [InlineData(1, 100)] // First retry: 100ms + [InlineData(2, 200)] // Second retry: 200ms + [InlineData(3, 400)] // Third retry: 400ms + [InlineData(10, 10000)] // Should cap at MaxDelay (10s) + public void RetryPolicy_CalculateDelay_UsesExponentialBackoff(int attempt, double expectedBaseDelayMs) + { + // Arrange + var policy = new RetryPolicy { JitterFactor = 0 }; // Disable jitter for deterministic testing + + // Act + var delay = policy.CalculateDelay(attempt); + + // Assert + Assert.Equal(expectedBaseDelayMs, delay.TotalMilliseconds); + } + + [Fact] + public async Task RetryPolicy_ExecuteAsync_RetriesOnTransientError() + { + // Arrange + var policy = new RetryPolicy { MaxRetries = 2, InitialDelay = TimeSpan.FromMilliseconds(1) }; + var attempts = 0; + + // Act + var result = await policy.ExecuteAsync(async ct => + { + attempts++; + if (attempts < 3) + { + throw new SocketException((int)SocketError.ConnectionRefused); + } + return 42; + }, CancellationToken.None); + + // Assert + Assert.Equal(3, attempts); + Assert.Equal(42, result); + } + + [Fact] + public async Task RetryPolicy_ExecuteAsync_ThrowsOnExhaustion() + { + // Arrange + var policy = new RetryPolicy { MaxRetries = 2, InitialDelay = TimeSpan.FromMilliseconds(1) }; + var attempts = 0; + + // Act & Assert + var ex = await Assert.ThrowsAnyAsync(async () => + { + await policy.ExecuteAsync(async ct => + { + attempts++; + throw new SocketException((int)SocketError.ConnectionRefused); + }, CancellationToken.None); + }); + + // Should have tried 3 times (initial + 2 retries) + Assert.Equal(3, attempts); + Assert.True(ex is AggregateException || ex is SocketException); + } + + [Fact] + public void ManagedHandler_RetryPolicy_HasDefault() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Assert + Assert.NotNull(handler.RetryPolicy); + Assert.Equal(RetryPolicy.Default.MaxRetries, handler.RetryPolicy.MaxRetries); + } + + [Fact] + public void ManagedHandler_RetryPolicy_CanBeDisabled() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Act + handler.RetryPolicy = RetryPolicy.NoRetry; + + // Assert + Assert.Equal(0, handler.RetryPolicy.MaxRetries); + } + + #endregion + + #region Phase 4: Happy Eyeballs Tests + +#if NET6_0_OR_GREATER + [Fact] + public void SocketConfiguration_HappyEyeballs_DefaultEnabled() + { + // Arrange & Act + var config = new SocketConfiguration(); + + // Assert + Assert.True(config.EnableHappyEyeballs); + Assert.Equal(TimeSpan.FromMilliseconds(250), config.HappyEyeballsDelay); + } + + [Fact] + public void SocketConfiguration_HappyEyeballs_CanBeDisabled() + { + // Arrange + var config = new SocketConfiguration(); + + // Act + config.EnableHappyEyeballs = false; + + // Assert + Assert.False(config.EnableHappyEyeballs); + } + + [Fact] + public void SocketConfiguration_HappyEyeballsDelay_CanBeCustomized() + { + // Arrange + var config = new SocketConfiguration(); + + // Act + config.HappyEyeballsDelay = TimeSpan.FromMilliseconds(500); + + // Assert + Assert.Equal(TimeSpan.FromMilliseconds(500), config.HappyEyeballsDelay); + } +#endif + + #endregion + + #region Phase 5: Connection Pooling Tests + +#if NET5_0_OR_GREATER + [Fact] + public void ManagedHandler_ConnectionPooling_DefaultEnabled() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Assert + Assert.True(handler.EnableConnectionPooling); + } + + [Fact] + public void ManagedHandler_ConnectionPooling_CanBeDisabled() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Act + handler.EnableConnectionPooling = false; + + // Assert + Assert.False(handler.EnableConnectionPooling); + } + + [Fact] + public void ConnectionPool_Default_HasSensibleDefaults() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + var pool = new ConnectionPool(logger); + + // Assert + Assert.Equal(TimeSpan.FromMinutes(2), pool.IdleTimeout); + Assert.Equal(TimeSpan.FromMinutes(10), pool.ConnectionLifetime); + Assert.Equal(10, pool.MaxConnectionsPerHost); + + pool.Dispose(); + } + + [Fact] + public void ConnectionPool_TryGetConnection_ReturnsFalseWhenEmpty() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var pool = new ConnectionPool(logger); + + // Act + var result = pool.TryGetConnection("localhost", 2375, false, out var connection); + + // Assert + Assert.False(result); + Assert.Null(connection); + } +#endif + + #endregion + + #region Phase 6: Modern Proxy Resolution Tests + + [Fact] + public void ManagedHandler_Proxy_CanBeSet() + { + // Arrange + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + var proxy = new System.Net.WebProxy("http://proxy.example.com:8080"); + + // Act + handler.Proxy = proxy; + + // Assert + Assert.Equal(proxy, handler.Proxy); + } + + [Fact] + public void ManagedHandler_UseProxy_DefaultTrue() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Assert + Assert.True(handler.UseProxy); + } + + #endregion + + #region Phase 7: PipeReader Line Reading Tests + +#if NET6_0_OR_GREATER + [Fact] + public void BufferedReadStream_UsePipeReader_DefaultTrue() + { + // Arrange + var mockStream = new MemoryStream(); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + + // Act + using var bufferedStream = new BufferedReadStream(mockStream, null, logger); + + // Assert + Assert.True(bufferedStream.UsePipeReader); + } + + [Fact] + public void BufferedReadStream_UsePipeReader_CanBeDisabled() + { + // Arrange + var mockStream = new MemoryStream(); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var bufferedStream = new BufferedReadStream(mockStream, null, logger); + + // Act + bufferedStream.UsePipeReader = false; + + // Assert + Assert.False(bufferedStream.UsePipeReader); + } + + [Fact] + public async Task BufferedReadStream_ReadLineAsync_ReadsLine() + { + // Arrange + var testData = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; + var mockStream = new MemoryStream(Encoding.ASCII.GetBytes(testData)); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + await using var bufferedStream = new BufferedReadStream(mockStream, null, logger); + + // Act + var line = await bufferedStream.ReadLineAsync(CancellationToken.None); + + // Assert + Assert.Equal("HTTP/1.1 200 OK", line); + } + + [Fact] + public async Task BufferedReadStream_ReadLineAsync_WithPipeReaderDisabled_ReadsLine() + { + // Arrange + var testData = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; + var mockStream = new MemoryStream(Encoding.ASCII.GetBytes(testData)); + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + await using var bufferedStream = new BufferedReadStream(mockStream, null, logger); + bufferedStream.UsePipeReader = false; + + // Act + var line = await bufferedStream.ReadLineAsync(CancellationToken.None); + + // Assert + Assert.Equal("HTTP/1.1 200 OK", line); + } +#endif + + #endregion + + #region Integration Tests + + [Fact] + public void ManagedHandler_FullConfiguration_AllPropertiesAccessible() + { + // Arrange & Act + var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); + using var handler = new ManagedHandler(logger); + + // Assert - All properties should be accessible + Assert.NotNull(handler.Proxy); + Assert.True(handler.UseProxy); + Assert.Equal(20, handler.MaxAutomaticRedirects); + Assert.Equal(RedirectMode.NoDowngrade, handler.RedirectMode); + Assert.Null(handler.ServerCertificateValidationCallback); + Assert.NotNull(handler.ClientCertificates); + Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); + Assert.NotNull(handler.RetryPolicy); +#if NET5_0_OR_GREATER + Assert.True(handler.EnableConnectionPooling); +#endif + } + + [Fact] + public void SocketConfiguration_FullConfiguration_AllPropertiesAccessible() + { + // Arrange & Act + var config = new SocketConfiguration(); + + // Assert - All properties should be accessible + Assert.True(config.KeepAlive); + Assert.Equal(30, config.KeepAliveTime); + Assert.Equal(10, config.KeepAliveInterval); + Assert.Equal(3, config.KeepAliveRetryCount); + Assert.True(config.NoDelay); + Assert.Null(config.SendBufferSize); + Assert.Null(config.ReceiveBufferSize); +#if NET6_0_OR_GREATER + Assert.True(config.EnableHappyEyeballs); + Assert.Equal(TimeSpan.FromMilliseconds(250), config.HappyEyeballsDelay); +#endif + } + + #endregion +} From efac370fafdcd447839ae0805f2eaf98b4fc2680 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 12:52:12 +0000 Subject: [PATCH 10/18] fix: Remove PipeReader from BufferedReadStream to avoid chunked stream interference The PipeReader implementation was bypassing the internal buffer and causing issues with chunked stream reading. Removed for now - can be re-added later with proper integration if needed. Co-Authored-By: Claude Opus 4.5 --- .../BufferedReadStream.cs | 113 ------------------ .../ManagedHandlerTests.cs | 48 +------- test/Docker.DotNet.Tests/TestFixture.cs | 6 + 3 files changed, 7 insertions(+), 160 deletions(-) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs index 9d1f6635..e52fe7da 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs @@ -14,21 +14,6 @@ internal sealed class BufferedReadStream : WriteClosableStream, IPeekableStream private int _bufferCount; private bool _disposed; -#if NET6_0_OR_GREATER - private PipeReader _pipeReader; - private bool _usePipeReader = true; - private bool _pipeReaderFailed; - - /// - /// Gets or sets whether to use PipeReader for line reading operations. - /// Default is true. Falls back to byte-by-byte reading on failure. - /// - public bool UsePipeReader - { - get => _usePipeReader; - set => _usePipeReader = value; - } -#endif public BufferedReadStream(Stream inner, Socket socket, ILogger logger) : this(inner, socket, 8192, logger) @@ -92,10 +77,6 @@ protected override void Dispose(bool disposing) ArrayPool.Shared.Return(_buffer); } -#if NET6_0_OR_GREATER - _pipeReader?.Complete(); -#endif - _inner.Dispose(); } @@ -116,13 +97,6 @@ protected override void Dispose(bool disposing) ArrayPool.Shared.Return(_buffer); } -#if NET6_0_OR_GREATER - if (_pipeReader != null) - { - await _pipeReader.CompleteAsync().ConfigureAwait(false); - } -#endif - await _inner.DisposeAsync().ConfigureAwait(false); GC.SuppressFinalize(this); } @@ -248,93 +222,6 @@ public bool Peek(byte[] buffer, uint toPeek, out uint peeked, out uint available } public async Task ReadLineAsync(CancellationToken cancellationToken) - { -#if NET6_0_OR_GREATER - // Try PipeReader-based implementation first (more efficient) - if (_usePipeReader && !_pipeReaderFailed) - { - try - { - return await ReadLineWithPipeReaderAsync(cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - // Fall back to byte-by-byte on failure - _pipeReaderFailed = true; - _logger.LogWarning(ex, "PipeReader line reading failed, falling back to byte-by-byte implementation"); - } - } -#endif - - return await ReadLineByteByByteAsync(cancellationToken).ConfigureAwait(false); - } - -#if NET6_0_OR_GREATER - private async Task ReadLineWithPipeReaderAsync(CancellationToken cancellationToken) - { - // Create PipeReader lazily - if (_pipeReader == null) - { - // First, consume any buffered data - if (_bufferCount > 0) - { - // We have buffered data, fall back to byte-by-byte for this call - // to avoid complexity of merging buffer with PipeReader - return await ReadLineByteByByteAsync(cancellationToken).ConfigureAwait(false); - } - - _pipeReader = PipeReader.Create(_inner, new StreamPipeReaderOptions(leaveOpen: true)); - } - - while (true) - { - var result = await _pipeReader.ReadAsync(cancellationToken).ConfigureAwait(false); - var buffer = result.Buffer; - - // Look for CRLF in the buffer - var position = buffer.PositionOf((byte)'\n'); - - if (position != null) - { - // Found newline - extract line up to it - var lineBuffer = buffer.Slice(0, position.Value); - var lineBytes = lineBuffer.ToArray(); - - // Remove trailing CR if present - var lineLength = lineBytes.Length; - if (lineLength > 0 && lineBytes[lineLength - 1] == '\r') - { - lineLength--; - } - - var line = Encoding.ASCII.GetString(lineBytes, 0, lineLength); - - // Tell the PipeReader we've consumed up to and including the newline - _pipeReader.AdvanceTo(buffer.GetPosition(1, position.Value)); - - return line; - } - - if (result.IsCompleted) - { - if (buffer.Length > 0) - { - // Return remaining data as the last line - var line = Encoding.ASCII.GetString(buffer.ToArray()); - _pipeReader.AdvanceTo(buffer.End); - return line; - } - - throw new EndOfStreamException("Unexpected end of stream while reading line"); - } - - // Tell the reader we need more data - _pipeReader.AdvanceTo(buffer.Start, buffer.End); - } - } -#endif - - private async Task ReadLineByteByByteAsync(CancellationToken cancellationToken) { var line = new StringBuilder(32); diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index 780737c2..ffb308a5 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -389,38 +389,9 @@ public void ManagedHandler_UseProxy_DefaultTrue() #endregion - #region Phase 7: PipeReader Line Reading Tests + #region Phase 7: Line Reading Tests #if NET6_0_OR_GREATER - [Fact] - public void BufferedReadStream_UsePipeReader_DefaultTrue() - { - // Arrange - var mockStream = new MemoryStream(); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - - // Act - using var bufferedStream = new BufferedReadStream(mockStream, null, logger); - - // Assert - Assert.True(bufferedStream.UsePipeReader); - } - - [Fact] - public void BufferedReadStream_UsePipeReader_CanBeDisabled() - { - // Arrange - var mockStream = new MemoryStream(); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var bufferedStream = new BufferedReadStream(mockStream, null, logger); - - // Act - bufferedStream.UsePipeReader = false; - - // Assert - Assert.False(bufferedStream.UsePipeReader); - } - [Fact] public async Task BufferedReadStream_ReadLineAsync_ReadsLine() { @@ -436,23 +407,6 @@ public async Task BufferedReadStream_ReadLineAsync_ReadsLine() // Assert Assert.Equal("HTTP/1.1 200 OK", line); } - - [Fact] - public async Task BufferedReadStream_ReadLineAsync_WithPipeReaderDisabled_ReadsLine() - { - // Arrange - var testData = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; - var mockStream = new MemoryStream(Encoding.ASCII.GetBytes(testData)); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - await using var bufferedStream = new BufferedReadStream(mockStream, null, logger); - bufferedStream.UsePipeReader = false; - - // Act - var line = await bufferedStream.ReadLineAsync(CancellationToken.None); - - // Assert - Assert.Equal("HTTP/1.1 200 OK", line); - } #endif #endregion diff --git a/test/Docker.DotNet.Tests/TestFixture.cs b/test/Docker.DotNet.Tests/TestFixture.cs index b32ecd6e..64a606d9 100644 --- a/test/Docker.DotNet.Tests/TestFixture.cs +++ b/test/Docker.DotNet.Tests/TestFixture.cs @@ -116,6 +116,12 @@ await DockerClient.Swarm.LeaveSwarmAsync(new SwarmLeaveParameters { Force = true .ConfigureAwait(false); } + // If InitializeAsync failed, Image may be null + if (Image == null) + { + return; + } + var containers = await DockerClient.Containers.ListContainersAsync( new ContainersListParameters { From b0c844702ee7e85b818c56ff53a379057a6502f0 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 12:57:33 +0000 Subject: [PATCH 11/18] refactor: Remove retry logic from ManagedHandler Remove RetryPolicy class and related retry wrapper around socket connections. Co-Authored-By: Claude Opus 4.5 --- .../ManagedHandler.cs | 22 +- src/Docker.DotNet/RetryPolicy.cs | 194 ------------------ .../ManagedHandlerTests.cs | 138 ------------- 3 files changed, 1 insertion(+), 353 deletions(-) delete mode 100644 src/Docker.DotNet/RetryPolicy.cs diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index d5d67026..224c5d1e 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -15,8 +15,6 @@ public class ManagedHandler : HttpMessageHandler private IWebProxy _proxy; - private RetryPolicy _retryPolicy = RetryPolicy.Default; - #if NET5_0_OR_GREATER private readonly ConnectionPool _connectionPool; private bool _enableConnectionPooling = true; @@ -109,17 +107,6 @@ public IWebProxy Proxy /// public TimeSpan ConnectTimeout { get; set; } = TimeSpan.FromSeconds(30); - /// - /// Gets or sets the retry policy for transient connection failures. - /// Set to to disable retries. - /// Default uses exponential backoff with 3 retries. - /// - public RetryPolicy RetryPolicy - { - get => _retryPolicy; - set => _retryPolicy = value ?? RetryPolicy.NoRetry; - } - #if NET5_0_OR_GREATER /// /// Gets or sets whether connection pooling is enabled. @@ -562,14 +549,7 @@ private ProxyMode DetermineProxyModeAndAddressLine(HttpRequestMessage request) return ProxyMode.Tunnel; } - private Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) - { - return _retryPolicy.ExecuteAsync( - ct => TcpSocketOpenerCoreAsync(host, port, ct), - cancellationToken); - } - - private async Task TcpSocketOpenerCoreAsync(string host, int port, CancellationToken cancellationToken) + private async Task TcpSocketOpenerAsync(string host, int port, CancellationToken cancellationToken) { #if NET5_0_OR_GREATER // Use modern DNS resolution with cancellation support diff --git a/src/Docker.DotNet/RetryPolicy.cs b/src/Docker.DotNet/RetryPolicy.cs deleted file mode 100644 index 20466839..00000000 --- a/src/Docker.DotNet/RetryPolicy.cs +++ /dev/null @@ -1,194 +0,0 @@ -namespace Docker.DotNet; - -/// -/// Configures retry behavior for transient connection failures. -/// -public sealed class RetryPolicy -{ - private static readonly Random Jitter = new Random(); - - /// - /// Gets the default retry policy with sensible defaults. - /// - public static RetryPolicy Default { get; } = new RetryPolicy(); - - /// - /// Gets a retry policy that disables retries. - /// - public static RetryPolicy NoRetry { get; } = new RetryPolicy { MaxRetries = 0 }; - - /// - /// Gets or sets the maximum number of retry attempts. Default is 3. - /// Set to 0 to disable retries. - /// - public int MaxRetries { get; set; } = 3; - - /// - /// Gets or sets the initial delay before the first retry. Default is 100ms. - /// - public TimeSpan InitialDelay { get; set; } = TimeSpan.FromMilliseconds(100); - - /// - /// Gets or sets the maximum delay between retries. Default is 10 seconds. - /// - public TimeSpan MaxDelay { get; set; } = TimeSpan.FromSeconds(10); - - /// - /// Gets or sets the jitter factor to add randomness to delays. Default is 0.2 (20%). - /// This helps prevent thundering herd problems when multiple clients retry simultaneously. - /// - public double JitterFactor { get; set; } = 0.2; - - /// - /// Gets or sets the backoff multiplier for exponential backoff. Default is 2.0. - /// - public double BackoffMultiplier { get; set; } = 2.0; - - /// - /// Determines if the specified socket exception represents a transient error that can be retried. - /// - /// The socket exception to evaluate. - /// True if the error is transient and the operation can be retried; otherwise, false. - public bool IsTransientError(SocketException exception) - { - if (exception == null) - { - return false; - } - - // Transient socket errors that are safe to retry - return exception.SocketErrorCode switch - { - // Connection refused - server might not be ready yet - SocketError.ConnectionRefused => true, - - // Connection reset - connection was forcibly closed - SocketError.ConnectionReset => true, - - // Host unreachable - temporary network issue - SocketError.HostUnreachable => true, - - // Network unreachable - temporary network issue - SocketError.NetworkUnreachable => true, - - // Timed out - might succeed on retry - SocketError.TimedOut => true, - - // Try again - explicit retry suggestion - SocketError.TryAgain => true, - - // Host not found - DNS might be propagating - SocketError.HostNotFound => true, - - // No buffer space available - temporary resource constraint - SocketError.NoBufferSpaceAvailable => true, - - // Too many open sockets - temporary resource constraint - SocketError.TooManyOpenSockets => true, - - // Connection aborted - connection was aborted by the network - SocketError.ConnectionAborted => true, - - // All other errors are not considered transient - _ => false - }; - } - - /// - /// Determines if the specified exception represents a transient error that can be retried. - /// - /// The exception to evaluate. - /// True if the error is transient and the operation can be retried; otherwise, false. - public bool IsTransientError(Exception exception) - { - // Check for SocketException directly - if (exception is SocketException socketEx) - { - return IsTransientError(socketEx); - } - - // Check for SocketException in inner exception - if (exception.InnerException is SocketException innerSocketEx) - { - return IsTransientError(innerSocketEx); - } - - // IOException can wrap socket errors - if (exception is IOException ioEx && ioEx.InnerException is SocketException ioSocketEx) - { - return IsTransientError(ioSocketEx); - } - - // TimeoutException is transient - if (exception is TimeoutException) - { - return true; - } - - return false; - } - - /// - /// Calculates the delay before the next retry attempt using exponential backoff with jitter. - /// - /// The current retry attempt number (1-based). - /// The delay before the next retry. - public TimeSpan CalculateDelay(int retryAttempt) - { - if (retryAttempt < 1) - { - return InitialDelay; - } - - // Calculate exponential backoff - var exponentialDelay = InitialDelay.TotalMilliseconds * Math.Pow(BackoffMultiplier, retryAttempt - 1); - - // Cap at max delay - var cappedDelay = Math.Min(exponentialDelay, MaxDelay.TotalMilliseconds); - - // Add jitter - double jitterAmount; - lock (Jitter) - { - jitterAmount = Jitter.NextDouble() * JitterFactor * 2 - JitterFactor; // Range: [-JitterFactor, +JitterFactor] - } - var delayWithJitter = cappedDelay * (1 + jitterAmount); - - // Ensure delay is not negative - return TimeSpan.FromMilliseconds(Math.Max(0, delayWithJitter)); - } - - /// - /// Executes an async operation with retry logic. - /// - /// The return type of the operation. - /// The operation to execute. - /// Cancellation token. - /// The result of the operation. - /// Thrown when all retry attempts fail. - public async Task ExecuteAsync(Func> operation, CancellationToken cancellationToken) - { - if (MaxRetries == 0) - { - return await operation(cancellationToken).ConfigureAwait(false); - } - - var exceptions = new List(); - - for (int attempt = 0; attempt <= MaxRetries; attempt++) - { - try - { - return await operation(cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) when (attempt < MaxRetries && !cancellationToken.IsCancellationRequested && IsTransientError(ex)) - { - exceptions.Add(ex); - var delay = CalculateDelay(attempt + 1); - await Task.Delay(delay, cancellationToken).ConfigureAwait(false); - } - } - - throw new AggregateException($"Operation failed after {MaxRetries + 1} attempts.", exceptions); - } -} diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index ffb308a5..8b8d3b1f 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -118,143 +118,6 @@ public void ManagedHandler_ConnectTimeout_CanBeDisabled() #endregion - #region Phase 3: Retry Policy Tests - - [Fact] - public void RetryPolicy_Default_HasSensibleDefaults() - { - // Arrange & Act - var policy = RetryPolicy.Default; - - // Assert - Assert.Equal(3, policy.MaxRetries); - Assert.Equal(TimeSpan.FromMilliseconds(100), policy.InitialDelay); - Assert.Equal(TimeSpan.FromSeconds(10), policy.MaxDelay); - Assert.Equal(0.2, policy.JitterFactor); - } - - [Fact] - public void RetryPolicy_NoRetry_DisablesRetries() - { - // Arrange & Act - var policy = RetryPolicy.NoRetry; - - // Assert - Assert.Equal(0, policy.MaxRetries); - } - - [Theory] - [InlineData(SocketError.ConnectionRefused, true)] - [InlineData(SocketError.ConnectionReset, true)] - [InlineData(SocketError.HostUnreachable, true)] - [InlineData(SocketError.NetworkUnreachable, true)] - [InlineData(SocketError.TimedOut, true)] - [InlineData(SocketError.TryAgain, true)] - [InlineData(SocketError.AccessDenied, false)] - [InlineData(SocketError.AddressAlreadyInUse, false)] - public void RetryPolicy_IsTransientError_ClassifiesSocketErrors(SocketError errorCode, bool expected) - { - // Arrange - var policy = new RetryPolicy(); - var exception = new SocketException((int)errorCode); - - // Act - var isTransient = policy.IsTransientError(exception); - - // Assert - Assert.Equal(expected, isTransient); - } - - [Theory] - [InlineData(1, 100)] // First retry: 100ms - [InlineData(2, 200)] // Second retry: 200ms - [InlineData(3, 400)] // Third retry: 400ms - [InlineData(10, 10000)] // Should cap at MaxDelay (10s) - public void RetryPolicy_CalculateDelay_UsesExponentialBackoff(int attempt, double expectedBaseDelayMs) - { - // Arrange - var policy = new RetryPolicy { JitterFactor = 0 }; // Disable jitter for deterministic testing - - // Act - var delay = policy.CalculateDelay(attempt); - - // Assert - Assert.Equal(expectedBaseDelayMs, delay.TotalMilliseconds); - } - - [Fact] - public async Task RetryPolicy_ExecuteAsync_RetriesOnTransientError() - { - // Arrange - var policy = new RetryPolicy { MaxRetries = 2, InitialDelay = TimeSpan.FromMilliseconds(1) }; - var attempts = 0; - - // Act - var result = await policy.ExecuteAsync(async ct => - { - attempts++; - if (attempts < 3) - { - throw new SocketException((int)SocketError.ConnectionRefused); - } - return 42; - }, CancellationToken.None); - - // Assert - Assert.Equal(3, attempts); - Assert.Equal(42, result); - } - - [Fact] - public async Task RetryPolicy_ExecuteAsync_ThrowsOnExhaustion() - { - // Arrange - var policy = new RetryPolicy { MaxRetries = 2, InitialDelay = TimeSpan.FromMilliseconds(1) }; - var attempts = 0; - - // Act & Assert - var ex = await Assert.ThrowsAnyAsync(async () => - { - await policy.ExecuteAsync(async ct => - { - attempts++; - throw new SocketException((int)SocketError.ConnectionRefused); - }, CancellationToken.None); - }); - - // Should have tried 3 times (initial + 2 retries) - Assert.Equal(3, attempts); - Assert.True(ex is AggregateException || ex is SocketException); - } - - [Fact] - public void ManagedHandler_RetryPolicy_HasDefault() - { - // Arrange & Act - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Assert - Assert.NotNull(handler.RetryPolicy); - Assert.Equal(RetryPolicy.Default.MaxRetries, handler.RetryPolicy.MaxRetries); - } - - [Fact] - public void ManagedHandler_RetryPolicy_CanBeDisabled() - { - // Arrange - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Act - handler.RetryPolicy = RetryPolicy.NoRetry; - - // Assert - Assert.Equal(0, handler.RetryPolicy.MaxRetries); - } - - #endregion - #region Phase 4: Happy Eyeballs Tests #if NET6_0_OR_GREATER @@ -428,7 +291,6 @@ public void ManagedHandler_FullConfiguration_AllPropertiesAccessible() Assert.Null(handler.ServerCertificateValidationCallback); Assert.NotNull(handler.ClientCertificates); Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); - Assert.NotNull(handler.RetryPolicy); #if NET5_0_OR_GREATER Assert.True(handler.EnableConnectionPooling); #endif From ccb228ec337b75538f6540088142220030de510c Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 13:04:36 +0000 Subject: [PATCH 12/18] refactor: Remove connection pooling from ManagedHandler Remove connection pooling feature to simplify the codebase: - Remove ConnectionPool.cs - Simplify ProcessRequestAsync in ManagedHandler.cs - Remove pooling code from HttpConnectionResponseContent.cs - Remove connection pooling tests from ManagedHandlerTests.cs Co-Authored-By: Claude Opus 4.5 --- .../ConnectionPool.cs | 296 ------------------ .../HttpConnectionResponseContent.cs | 55 +--- .../ManagedHandler.cs | 228 ++------------ .../ManagedHandlerTests.cs | 64 ---- 4 files changed, 33 insertions(+), 610 deletions(-) delete mode 100644 src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs deleted file mode 100644 index 4fa672d4..00000000 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ConnectionPool.cs +++ /dev/null @@ -1,296 +0,0 @@ -#if NET5_0_OR_GREATER -namespace Microsoft.Net.Http.Client; - -/// -/// Manages a pool of HTTP connections for reuse. -/// -internal sealed class ConnectionPool : IDisposable -{ - private readonly ConcurrentDictionary> _pools = new(); - private readonly ILogger _logger; - private readonly Timer _cleanupTimer; - private bool _disposed; - - /// - /// Gets or sets the maximum time a connection can remain idle in the pool before being removed. - /// Default is 2 minutes. - /// - public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromMinutes(2); - - /// - /// Gets or sets the maximum lifetime of a connection, regardless of activity. - /// Default is 10 minutes. - /// - public TimeSpan ConnectionLifetime { get; set; } = TimeSpan.FromMinutes(10); - - /// - /// Gets or sets the maximum number of connections per host. - /// Default is 10. - /// - public int MaxConnectionsPerHost { get; set; } = 10; - - public ConnectionPool(ILogger logger) - { - _logger = logger; - // Run cleanup every 30 seconds - _cleanupTimer = new Timer(CleanupCallback, null, TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(30)); - } - - /// - /// Attempts to get a pooled connection for the specified host and port. - /// - /// The host name. - /// The port number. - /// Whether the connection uses HTTPS. - /// The pooled connection if found. - /// True if a valid pooled connection was found; otherwise, false. - public bool TryGetConnection(string host, int port, bool isHttps, out PooledConnection connection) - { - connection = null; - - if (_disposed) - { - return false; - } - - var key = GetPoolKey(host, port, isHttps); - - if (!_pools.TryGetValue(key, out var pool)) - { - return false; - } - - while (pool.TryDequeue(out var candidate)) - { - // Check if connection is still valid - if (IsConnectionValid(candidate)) - { - connection = candidate; - _logger.LogDebug("Reusing pooled connection to {Host}:{Port}", host, port); - return true; - } - - // Connection is no longer valid, dispose it - try - { - candidate.Dispose(); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error disposing invalid pooled connection"); - } - } - - return false; - } - - /// - /// Returns a connection to the pool for reuse. - /// - /// The connection to return. - /// True if the connection was returned to the pool; otherwise, false. - public bool ReturnConnection(PooledConnection connection) - { - if (_disposed || connection == null) - { - connection?.Dispose(); - return false; - } - - // Check if connection is still valid for reuse - if (!IsConnectionValid(connection)) - { - _logger.LogDebug("Connection not valid for pooling, disposing"); - connection.Dispose(); - return false; - } - - var key = connection.PoolKey; - var pool = _pools.GetOrAdd(key, _ => new ConcurrentQueue()); - - // Check pool size limit - if (pool.Count >= MaxConnectionsPerHost) - { - _logger.LogDebug("Pool for {Key} is full ({Count}/{Max}), disposing connection", - key, pool.Count, MaxConnectionsPerHost); - connection.Dispose(); - return false; - } - - connection.LastUsed = DateTime.UtcNow; - pool.Enqueue(connection); - _logger.LogDebug("Returned connection to pool for {Key}", key); - return true; - } - - /// - /// Creates a new pooled connection wrapper. - /// - public PooledConnection CreatePooledConnection( - string host, - int port, - bool isHttps, - Socket socket, - Stream transport, - BufferedReadStream bufferedStream) - { - var key = GetPoolKey(host, port, isHttps); - return new PooledConnection(key, socket, transport, bufferedStream); - } - - private bool IsConnectionValid(PooledConnection connection) - { - // Check lifetime - if (DateTime.UtcNow - connection.Created > ConnectionLifetime) - { - return false; - } - - // Check idle timeout - if (DateTime.UtcNow - connection.LastUsed > IdleTimeout) - { - return false; - } - - // Check if socket is still connected - if (connection.Socket != null) - { - try - { - // Poll with zero timeout to check socket state - // SelectRead with available=0 means the connection was closed gracefully - if (connection.Socket.Poll(0, SelectMode.SelectRead)) - { - if (connection.Socket.Available == 0) - { - return false; // Connection was closed - } - } - } - catch - { - return false; - } - } - - return true; - } - - private static string GetPoolKey(string host, int port, bool isHttps) - { - return $"{(isHttps ? "https" : "http")}://{host}:{port}"; - } - - private void CleanupCallback(object state) - { - if (_disposed) - { - return; - } - - foreach (var kvp in _pools) - { - var pool = kvp.Value; - var validConnections = new List(); - - while (pool.TryDequeue(out var connection)) - { - if (IsConnectionValid(connection)) - { - validConnections.Add(connection); - } - else - { - try - { - connection.Dispose(); - _logger.LogDebug("Cleaned up expired connection from pool {Key}", kvp.Key); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error disposing expired pooled connection"); - } - } - } - - // Re-add valid connections - foreach (var conn in validConnections) - { - pool.Enqueue(conn); - } - } - } - - public void Dispose() - { - if (_disposed) - { - return; - } - - _disposed = true; - _cleanupTimer.Dispose(); - - foreach (var kvp in _pools) - { - while (kvp.Value.TryDequeue(out var connection)) - { - try - { - connection.Dispose(); - } - catch - { - // Ignore cleanup errors - } - } - } - - _pools.Clear(); - } -} - -/// -/// Represents a pooled connection that can be returned to the pool for reuse. -/// -internal sealed class PooledConnection : IDisposable -{ - private bool _disposed; - - public PooledConnection(string poolKey, Socket socket, Stream transport, BufferedReadStream bufferedStream) - { - PoolKey = poolKey; - Socket = socket; - Transport = transport; - BufferedStream = bufferedStream; - Created = DateTime.UtcNow; - LastUsed = DateTime.UtcNow; - } - - public string PoolKey { get; } - public Socket Socket { get; } - public Stream Transport { get; } - public BufferedReadStream BufferedStream { get; } - public DateTime Created { get; } - public DateTime LastUsed { get; set; } - - public void Dispose() - { - if (_disposed) - { - return; - } - - _disposed = true; - - try - { - BufferedStream?.Dispose(); - } - catch - { - // Ignore - } - } -} -#endif diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs index f4c3cb24..9d4435c3 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnectionResponseContent.cs @@ -4,33 +4,12 @@ public class HttpConnectionResponseContent : HttpContent { private readonly HttpConnection _connection; private Stream _responseStream; - private bool _hijacked; - -#if NET5_0_OR_GREATER - private PooledConnection _pooledConnection; - private Action _poolReturnCallback; -#endif internal HttpConnectionResponseContent(HttpConnection connection) { _connection = connection; } -#if NET5_0_OR_GREATER - internal HttpConnectionResponseContent(HttpConnection connection, PooledConnection pooledConnection, Action poolReturnCallback) - { - _connection = connection; - _pooledConnection = pooledConnection; - _poolReturnCallback = poolReturnCallback; - } - - internal void SetPoolReturnCallback(PooledConnection pooledConnection, Action poolReturnCallback) - { - _pooledConnection = pooledConnection; - _poolReturnCallback = poolReturnCallback; - } -#endif - internal void ResolveResponseStream(bool chunked, bool isConnectionUpgrade) { if (_responseStream != null) @@ -58,14 +37,6 @@ public WriteClosableStream HijackStream() throw new InvalidOperationException("cannot hijack chunked or content length stream"); } - _hijacked = true; - -#if NET5_0_OR_GREATER - // Hijacked connections cannot be returned to the pool - _poolReturnCallback = null; - _pooledConnection = null; -#endif - return _connection.Transport; } @@ -91,30 +62,6 @@ protected override void Dispose(bool disposing) { if (disposing) { -#if NET5_0_OR_GREATER - // Try to return connection to pool if not hijacked - if (!_hijacked && _poolReturnCallback != null && _pooledConnection != null) - { - try - { - // Drain any remaining response data before returning to pool - // Only attempt for content-length streams where we know the remaining bytes - if (_responseStream is ContentLengthReadStream) - { - // Pool return callback will handle disposal if pooling fails - _poolReturnCallback(_pooledConnection); - _pooledConnection = null; - _poolReturnCallback = null; - return; // Don't dispose the connection, it's returned to pool - } - } - catch - { - // Fall through to normal disposal - } - } -#endif - _responseStream?.Dispose(); _connection.Dispose(); } @@ -124,4 +71,4 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } } -} \ No newline at end of file +} diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index 224c5d1e..7e12a741 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -15,11 +15,6 @@ public class ManagedHandler : HttpMessageHandler private IWebProxy _proxy; -#if NET5_0_OR_GREATER - private readonly ConnectionPool _connectionPool; - private bool _enableConnectionPooling = true; -#endif - public delegate Task StreamOpener(string host, int port, CancellationToken cancellationToken); public delegate Task SocketOpener(string host, int port, CancellationToken cancellationToken); @@ -34,9 +29,6 @@ public ManagedHandler(ILogger logger, SocketConfiguration socketConfiguration) _logger = logger; _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = TcpSocketOpenerAsync; -#if NET5_0_OR_GREATER - _connectionPool = new ConnectionPool(logger); -#endif } public ManagedHandler(StreamOpener opener, ILogger logger) @@ -44,9 +36,6 @@ public ManagedHandler(StreamOpener opener, ILogger logger) _logger = logger; _socketConfiguration = SocketConfiguration.Default; _streamOpener = opener ?? throw new ArgumentNullException(nameof(opener)); -#if NET5_0_OR_GREATER - _connectionPool = new ConnectionPool(logger); -#endif } public ManagedHandler(SocketOpener opener, ILogger logger) @@ -54,9 +43,6 @@ public ManagedHandler(SocketOpener opener, ILogger logger) _logger = logger; _socketConfiguration = SocketConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); -#if NET5_0_OR_GREATER - _connectionPool = new ConnectionPool(logger); -#endif } public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration socketConfiguration) @@ -64,9 +50,6 @@ public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration s _logger = logger; _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); -#if NET5_0_OR_GREATER - _connectionPool = new ConnectionPool(logger); -#endif } public IWebProxy Proxy @@ -107,24 +90,6 @@ public IWebProxy Proxy /// public TimeSpan ConnectTimeout { get; set; } = TimeSpan.FromSeconds(30); -#if NET5_0_OR_GREATER - /// - /// Gets or sets whether connection pooling is enabled. - /// Default is true. When enabled, connections are reused for subsequent requests. - /// Connection pooling is automatically disabled for hijacking requests (attach/exec). - /// - public bool EnableConnectionPooling - { - get => _enableConnectionPooling; - set => _enableConnectionPooling = value; - } - - /// - /// Gets the connection pool used by this handler. - /// - internal ConnectionPool ConnectionPool => _connectionPool; -#endif - protected override async Task SendAsync(HttpRequestMessage httpRequestMessage, CancellationToken cancellationToken) { if (httpRequestMessage == null) @@ -205,184 +170,61 @@ private async Task ProcessRequestAsync(HttpRequestMessage r ProcessUrl(request); ProcessHostHeader(request); - - // Check if this is a hijacking request (attach/exec) - these cannot use pooled connections - var isHijackingRequest = IsHijackingRequest(request); - -#if NET5_0_OR_GREATER - // For non-hijacking requests with pooling enabled, try to get a pooled connection - if (_enableConnectionPooling && !isHijackingRequest) - { - request.Headers.ConnectionClose = false; // Keep connection alive for pooling - } - else - { - request.Headers.ConnectionClose = !request.Headers.Contains("Connection"); - } -#else request.Headers.ConnectionClose = !request.Headers.Contains("Connection"); // TODO: Connection reuse is not supported. -#endif ProxyMode proxyMode = DetermineProxyModeAndAddressLine(request); Socket socket = null; Stream transport = null; BufferedReadStream bufferedReadStream = null; -#if NET5_0_OR_GREATER - // Try to get a pooled connection first - bool usePooledConnection = false; - PooledConnection pooledConnection = null; + // Create linked cancellation token for connection timeout + using var timeoutCts = ConnectTimeout != Timeout.InfiniteTimeSpan + ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken) + : null; - if (_enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None) + if (timeoutCts != null) { - try - { - if (_connectionPool.TryGetConnection( - request.GetConnectionHostProperty(), - request.GetConnectionPortProperty().Value, - request.IsHttps(), - out pooledConnection)) - { - socket = pooledConnection.Socket; - transport = pooledConnection.Transport; - bufferedReadStream = pooledConnection.BufferedStream; - usePooledConnection = true; - _logger.LogDebug("Using pooled connection for {Host}:{Port}", - request.GetConnectionHostProperty(), request.GetConnectionPortProperty()); - } - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error getting pooled connection, creating new connection"); - } + timeoutCts.CancelAfter(ConnectTimeout); } - if (!usePooledConnection) - { -#endif - // Create linked cancellation token for connection timeout - using var timeoutCts = ConnectTimeout != Timeout.InfiniteTimeSpan - ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken) - : null; - - if (timeoutCts != null) - { - timeoutCts.CancelAfter(ConnectTimeout); - } - - var effectiveCancellationToken = timeoutCts?.Token ?? cancellationToken; + var effectiveCancellationToken = timeoutCts?.Token ?? cancellationToken; - try - { - if (_socketOpener != null) - { - socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); - transport = new NetworkStream(socket, true); - } - else - { - socket = null; - transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); - } - } - catch (OperationCanceledException) when (timeoutCts?.IsCancellationRequested == true && !cancellationToken.IsCancellationRequested) - { - throw new TimeoutException($"Connection to {request.GetConnectionHostProperty()}:{request.GetConnectionPortProperty()} timed out after {ConnectTimeout.TotalSeconds} seconds."); - } - catch (SocketException e) - { - throw new HttpRequestException("Connection failed.", e); - } - - if (proxyMode == ProxyMode.Tunnel) + try + { + if (_socketOpener != null) { - await TunnelThroughProxyAsync(request, transport, cancellationToken); + socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); + transport = new NetworkStream(socket, true); } - - if (request.IsHttps()) + else { - transport = await EstablishSslAsync(transport, request.GetHostProperty(), cancellationToken).ConfigureAwait(false); + socket = null; + transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); } - - bufferedReadStream = new BufferedReadStream(transport, socket, _logger); -#if NET5_0_OR_GREATER } -#endif - - var connection = new HttpConnection(bufferedReadStream); + catch (OperationCanceledException) when (timeoutCts?.IsCancellationRequested == true && !cancellationToken.IsCancellationRequested) + { + throw new TimeoutException($"Connection to {request.GetConnectionHostProperty()}:{request.GetConnectionPortProperty()} timed out after {ConnectTimeout.TotalSeconds} seconds."); + } + catch (SocketException e) + { + throw new HttpRequestException("Connection failed.", e); + } -#if NET5_0_OR_GREATER - // Create pool return callback for non-hijacking requests - Action poolReturnCallback = null; - if (_enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None) + if (proxyMode == ProxyMode.Tunnel) { - var host = request.GetConnectionHostProperty(); - var port = request.GetConnectionPortProperty().Value; - var isHttps = request.IsHttps(); + await TunnelThroughProxyAsync(request, transport, cancellationToken); + } - poolReturnCallback = pooledConn => - { - try - { - if (!_connectionPool.ReturnConnection(pooledConn)) - { - pooledConn.Dispose(); - } - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error returning connection to pool"); - pooledConn?.Dispose(); - } - }; + if (request.IsHttps()) + { + transport = await EstablishSslAsync(transport, request.GetHostProperty(), cancellationToken).ConfigureAwait(false); } - var responseContent = new HttpConnectionResponseContent( - connection, - _enableConnectionPooling && !isHijackingRequest && proxyMode == ProxyMode.None - ? _connectionPool.CreatePooledConnection( - request.GetConnectionHostProperty(), - request.GetConnectionPortProperty().Value, - request.IsHttps(), - socket, - transport, - bufferedReadStream) - : null, - poolReturnCallback); - - var response = await connection.SendAsync(request, cancellationToken); - - // Replace the content with our pooling-aware content - var originalContent = response.Content as HttpConnectionResponseContent; - if (originalContent != null && poolReturnCallback != null) - { - originalContent.SetPoolReturnCallback( - _connectionPool.CreatePooledConnection( - request.GetConnectionHostProperty(), - request.GetConnectionPortProperty().Value, - request.IsHttps(), - socket, - transport, - bufferedReadStream), - poolReturnCallback); - } - - return response; -#else - return await connection.SendAsync(request, cancellationToken); -#endif - } + bufferedReadStream = new BufferedReadStream(transport, socket, _logger); - private static bool IsHijackingRequest(HttpRequestMessage request) - { - // Check for requests that require connection hijacking - // These are typically attach/exec operations that upgrade the connection - var pathAndQuery = request.GetPathAndQueryProperty() ?? request.RequestUri?.PathAndQuery ?? string.Empty; - - return pathAndQuery.Contains("/attach") || - pathAndQuery.Contains("/exec/") || - request.Headers.TryGetValues("Upgrade", out var upgradeValues) && - upgradeValues.Any(v => "tcp".Equals(v, StringComparison.OrdinalIgnoreCase)); + var connection = new HttpConnection(bufferedReadStream); + return await connection.SendAsync(request, cancellationToken); } private async Task EstablishSslAsync(Stream transport, string targetHost, CancellationToken cancellationToken) @@ -787,12 +629,6 @@ private async Task TunnelThroughProxyAsync(HttpRequestMessage request, Stream tr protected override void Dispose(bool disposing) { - if (disposing) - { -#if NET5_0_OR_GREATER - _connectionPool?.Dispose(); -#endif - } base.Dispose(disposing); } } \ No newline at end of file diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index 8b8d3b1f..ac0c5343 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -161,67 +161,6 @@ public void SocketConfiguration_HappyEyeballsDelay_CanBeCustomized() #endregion - #region Phase 5: Connection Pooling Tests - -#if NET5_0_OR_GREATER - [Fact] - public void ManagedHandler_ConnectionPooling_DefaultEnabled() - { - // Arrange & Act - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Assert - Assert.True(handler.EnableConnectionPooling); - } - - [Fact] - public void ManagedHandler_ConnectionPooling_CanBeDisabled() - { - // Arrange - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Act - handler.EnableConnectionPooling = false; - - // Assert - Assert.False(handler.EnableConnectionPooling); - } - - [Fact] - public void ConnectionPool_Default_HasSensibleDefaults() - { - // Arrange & Act - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - var pool = new ConnectionPool(logger); - - // Assert - Assert.Equal(TimeSpan.FromMinutes(2), pool.IdleTimeout); - Assert.Equal(TimeSpan.FromMinutes(10), pool.ConnectionLifetime); - Assert.Equal(10, pool.MaxConnectionsPerHost); - - pool.Dispose(); - } - - [Fact] - public void ConnectionPool_TryGetConnection_ReturnsFalseWhenEmpty() - { - // Arrange - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var pool = new ConnectionPool(logger); - - // Act - var result = pool.TryGetConnection("localhost", 2375, false, out var connection); - - // Assert - Assert.False(result); - Assert.Null(connection); - } -#endif - - #endregion - #region Phase 6: Modern Proxy Resolution Tests [Fact] @@ -291,9 +230,6 @@ public void ManagedHandler_FullConfiguration_AllPropertiesAccessible() Assert.Null(handler.ServerCertificateValidationCallback); Assert.NotNull(handler.ClientCertificates); Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); -#if NET5_0_OR_GREATER - Assert.True(handler.EnableConnectionPooling); -#endif } [Fact] From c5d2b500e9c8734cefa95cb077de18752e010d91 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 13:09:41 +0000 Subject: [PATCH 13/18] refactor: Remove Memory APIs and IAsyncDisposable from stream classes Remove the Memory/ReadOnlyMemory overloads and IAsyncDisposable implementations as they are not directly related to improving proxy functionality: - BufferedReadStream.cs: Remove IAsyncDisposable, DisposeAsync, Memory APIs - ChunkedReadStream.cs: Remove IAsyncDisposable, DisposeAsync, Memory APIs - ChunkedWriteStream.cs: Remove IAsyncDisposable, DisposeAsync, Memory APIs - ContentLengthReadStream.cs: Remove IAsyncDisposable, DisposeAsync, Memory APIs - HttpConnection.cs: Remove IAsyncDisposable, DisposeAsync, Memory API usage - ManagedHandlerTests.cs: Remove Phase 1 Memory API tests Co-Authored-By: Claude Opus 4.5 --- .../BufferedReadStream.cs | 56 -------------- .../ChunkedReadStream.cs | 76 ------------------- .../ChunkedWriteStream.cs | 43 ----------- .../ContentLengthReadStream.cs | 36 --------- .../HttpConnection.cs | 14 ---- .../ManagedHandlerTests.cs | 68 +---------------- 6 files changed, 2 insertions(+), 291 deletions(-) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs index e52fe7da..bfdffdf3 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs @@ -1,9 +1,6 @@ namespace Microsoft.Net.Http.Client; internal sealed class BufferedReadStream : WriteClosableStream, IPeekableStream -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - , IAsyncDisposable -#endif { private readonly Stream _inner; private readonly Socket _socket; @@ -83,25 +80,6 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public new async ValueTask DisposeAsync() - { - if (_disposed) - { - return; - } - - _disposed = true; - if (Interlocked.Exchange(ref _bufferRefCount, 0) == 1) - { - ArrayPool.Shared.Return(_buffer); - } - - await _inner.DisposeAsync().ConfigureAwait(false); - GC.SuppressFinalize(this); - } -#endif - public override long Seek(long offset, SeekOrigin origin) { throw new NotSupportedException(); @@ -132,13 +110,6 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati return _inner.WriteAsync(buffer, offset, count, cancellationToken); } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - return _inner.WriteAsync(buffer, cancellationToken); - } -#endif - public override int Read(byte[] buffer, int offset, int count) { int read = ReadBuffer(buffer, offset, count); @@ -161,33 +132,6 @@ public override Task ReadAsync(byte[] buffer, int offset, int count, Cancel return _inner.ReadAsync(buffer, offset, count, cancellationToken); } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - int read = ReadBufferMemory(buffer.Span); - if (read > 0) - { - return new ValueTask(read); - } - - return _inner.ReadAsync(buffer, cancellationToken); - } - - private int ReadBufferMemory(Span destination) - { - if (_bufferCount > 0) - { - int toCopy = Math.Min(_bufferCount, destination.Length); - _buffer.AsSpan(_bufferOffset, toCopy).CopyTo(destination); - _bufferOffset += toCopy; - _bufferCount -= toCopy; - return toCopy; - } - - return 0; - } -#endif - public override void CloseWrite() { if (_socket != null) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs index 3bfdf8af..495b8c6e 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs @@ -1,9 +1,6 @@ namespace Microsoft.Net.Http.Client; internal sealed class ChunkedReadStream : Stream -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - , IAsyncDisposable -#endif { private readonly BufferedReadStream _inner; private int _chunkBytesRemaining; @@ -126,59 +123,6 @@ public override async Task ReadAsync(byte[] buffer, int offset, int count, return readBytesCount; } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - if (_done) - { - return 0; - } - - if (_chunkBytesRemaining == 0) - { - var headerLine = await _inner.ReadLineAsync(cancellationToken) - .ConfigureAwait(false); - - if (!int.TryParse(headerLine, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out _chunkBytesRemaining)) - { - throw new IOException($"Invalid chunk header encountered: '{headerLine}'."); - } - } - - var readBytesCount = 0; - - if (_chunkBytesRemaining > 0) - { - var remainingBytesCount = Math.Min(_chunkBytesRemaining, buffer.Length); - - readBytesCount = await _inner.ReadAsync(buffer.Slice(0, remainingBytesCount), cancellationToken) - .ConfigureAwait(false); - - if (readBytesCount == 0) - { - throw new EndOfStreamException(); - } - - _chunkBytesRemaining -= readBytesCount; - } - - if (_chunkBytesRemaining == 0) - { - var emptyLine = await _inner.ReadLineAsync(cancellationToken) - .ConfigureAwait(false); - - if (!string.IsNullOrEmpty(emptyLine)) - { - throw new IOException($"Expected an empty line, but received: '{emptyLine}'."); - } - - _done = readBytesCount == 0; - } - - return readBytesCount; - } -#endif - public override void Write(byte[] buffer, int offset, int count) { _inner.Write(buffer, offset, count); @@ -189,13 +133,6 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati return _inner.WriteAsync(buffer, offset, count, cancellationToken); } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - return _inner.WriteAsync(buffer, cancellationToken); - } -#endif - public override long Seek(long offset, SeekOrigin origin) { throw new NotSupportedException(); @@ -220,17 +157,4 @@ protected override void Dispose(bool disposing) } base.Dispose(disposing); } - -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public new ValueTask DisposeAsync() - { - if (!_disposed) - { - _disposed = true; - // Note: We don't dispose _inner here as it's owned by HttpConnection - } - GC.SuppressFinalize(this); - return default; - } -#endif } \ No newline at end of file diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs index f46e8071..c231ab51 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs @@ -1,9 +1,6 @@ namespace Microsoft.Net.Http.Client; internal sealed class ChunkedWriteStream : Stream -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - , IAsyncDisposable -#endif { private static readonly byte[] EndOfContentBytes = Encoding.ASCII.GetBytes("0\r\n\r\n"); @@ -92,33 +89,6 @@ public Task EndContentAsync(CancellationToken cancellationToken) return _inner.WriteAsync(EndOfContentBytes, 0, EndOfContentBytes.Length, cancellationToken); } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override async ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) - { - if (buffer.Length == 0) - { - return; - } - - const string crlf = "\r\n"; - - var chunkHeader = buffer.Length.ToString("X") + crlf; - var headerBytes = Encoding.ASCII.GetBytes(chunkHeader); - - // Write the chunk header - await _inner.WriteAsync(headerBytes.AsMemory(), cancellationToken) - .ConfigureAwait(false); - - // Write the chunk data - await _inner.WriteAsync(buffer, cancellationToken) - .ConfigureAwait(false); - - // Write the chunk footer (CRLF) - await _inner.WriteAsync(headerBytes.AsMemory(headerBytes.Length - 2, 2), cancellationToken) - .ConfigureAwait(false); - } -#endif - protected override void Dispose(bool disposing) { if (!_disposed && disposing) @@ -128,17 +98,4 @@ protected override void Dispose(bool disposing) } base.Dispose(disposing); } - -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public new ValueTask DisposeAsync() - { - if (!_disposed) - { - _disposed = true; - // Note: We don't dispose _inner here as it's owned by the caller - } - GC.SuppressFinalize(this); - return default; - } -#endif } \ No newline at end of file diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs index a030a7f6..321110fb 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ContentLengthReadStream.cs @@ -1,9 +1,6 @@ namespace Microsoft.Net.Http.Client; internal class ContentLengthReadStream : Stream -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - , IAsyncDisposable -#endif { private readonly Stream _inner; private long _bytesRemaining; @@ -123,27 +120,6 @@ public override async Task ReadAsync(byte[] buffer, int offset, int count, return read; } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - if (_disposed) - { - return 0; - } - - if (_bytesRemaining == 0) - { - return 0; - } - - cancellationToken.ThrowIfCancellationRequested(); - int toRead = (int)Math.Min(buffer.Length, _bytesRemaining); - int read = await _inner.ReadAsync(buffer.Slice(0, toRead), cancellationToken).ConfigureAwait(false); - UpdateBytesRemaining(read); - return read; - } -#endif - protected override void Dispose(bool disposing) { if (disposing && !_disposed) @@ -154,18 +130,6 @@ protected override void Dispose(bool disposing) } } -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public new async ValueTask DisposeAsync() - { - if (!_disposed) - { - _disposed = true; - await _inner.DisposeAsync().ConfigureAwait(false); - } - GC.SuppressFinalize(this); - } -#endif - private void CheckDisposed() { if (_disposed) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs index 6b246bd1..f4ee563a 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/HttpConnection.cs @@ -1,9 +1,6 @@ namespace Microsoft.Net.Http.Client; internal sealed class HttpConnection : IDisposable -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - , IAsyncDisposable -#endif { private static readonly ISet DockerStreamHeaders = new HashSet{ "application/vnd.docker.raw-stream", "application/vnd.docker.multiplexed-stream" }; @@ -21,11 +18,7 @@ public async Task SendAsync(HttpRequestMessage request, Can // Serialize headers & send string rawRequest = SerializeRequest(request); byte[] requestBytes = Encoding.ASCII.GetBytes(rawRequest); -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - await Transport.WriteAsync(requestBytes.AsMemory(), cancellationToken).ConfigureAwait(false); -#else await Transport.WriteAsync(requestBytes, 0, requestBytes.Length, cancellationToken).ConfigureAwait(false); -#endif if (request.Content != null) { @@ -205,11 +198,4 @@ public void Dispose() { Transport.Dispose(); } - -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - public ValueTask DisposeAsync() - { - return Transport.DisposeAsync(); - } -#endif } \ No newline at end of file diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index ac0c5343..722d81ee 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -11,71 +11,7 @@ namespace Docker.DotNet.Tests; /// public class ManagedHandlerTests { - #region Phase 1: Memory-Efficient APIs & IAsyncDisposable Tests - - [Fact] - public async Task BufferedReadStream_ImplementsIAsyncDisposable() - { -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - // Arrange - var mockStream = new MemoryStream(Encoding.ASCII.GetBytes("Hello, World!")); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - var bufferedStream = new BufferedReadStream(mockStream, null, logger); - - // Act & Assert - Assert.True(bufferedStream is IAsyncDisposable); - await ((IAsyncDisposable)bufferedStream).DisposeAsync(); -#else - await Task.CompletedTask; -#endif - } - -#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER - [Fact] - public async Task BufferedReadStream_ReadAsyncMemory_ReadsFromBuffer() - { - // Arrange - var testData = "Hello, World!"u8.ToArray(); - var mockStream = new MemoryStream(testData); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - var bufferedStream = new BufferedReadStream(mockStream, null, logger); - - // Act - var buffer = new byte[5]; - var bytesRead = await bufferedStream.ReadAsync(buffer.AsMemory()); - - // Assert - Assert.Equal(5, bytesRead); - Assert.Equal("Hello", Encoding.ASCII.GetString(buffer)); - - await bufferedStream.DisposeAsync(); - } - - [Fact] - public async Task BufferedReadStream_WriteAsyncMemory_WritesToInner() - { - // Arrange - var outputStream = new MemoryStream(); - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - var bufferedStream = new BufferedReadStream(outputStream, null, logger); - - // Act - var data = "Test Data"u8.ToArray(); - await bufferedStream.WriteAsync(data.AsMemory()); - - // Assert - outputStream.Position = 0; - var result = new byte[data.Length]; - await outputStream.ReadAsync(result.AsMemory()); - Assert.Equal("Test Data", Encoding.ASCII.GetString(result)); - - await bufferedStream.DisposeAsync(); - } -#endif - - #endregion - - #region Phase 2: Connection Timeout Tests + #region Connection Timeout Tests [Fact] public void ManagedHandler_ConnectTimeout_HasDefaultValue() @@ -201,7 +137,7 @@ public async Task BufferedReadStream_ReadLineAsync_ReadsLine() var testData = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; var mockStream = new MemoryStream(Encoding.ASCII.GetBytes(testData)); var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - await using var bufferedStream = new BufferedReadStream(mockStream, null, logger); + using var bufferedStream = new BufferedReadStream(mockStream, null, logger); // Act var line = await bufferedStream.ReadLineAsync(CancellationToken.None); From e27268b1158fe706bdbc351adf8dcae0bd4eeb6d Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 13:15:41 +0000 Subject: [PATCH 14/18] fix: Disable proxy resolution for Unix socket and named pipe connections Local Docker connections via Unix sockets and named pipes should not use proxy resolution as they connect directly to the local Docker daemon. This change explicitly sets UseProxy = false for these connection types, avoiding unnecessary proxy resolution overhead and potential issues in restricted environments like Docker-in-Docker GitHub Actions runners. Co-Authored-By: Claude Opus 4.5 --- src/Docker.DotNet/DockerClient.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 2d568351..99e45530 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -60,7 +60,7 @@ internal DockerClient(DockerClientConfiguration configuration, Version requested var pipeName = uri.Segments[2]; uri = new UriBuilder("http", pipeName).Uri; - handler = new ManagedHandler(async (host, port, cancellationToken) => + var pipeHandler = new ManagedHandler(async (host, port, cancellationToken) => { var timeout = (int)Configuration.NamedPipeConnectTimeout.TotalMilliseconds; var stream = new NamedPipeClientStream(serverName, pipeName, PipeDirection.InOut, PipeOptions.Asynchronous); @@ -71,6 +71,9 @@ await stream.ConnectAsync(timeout, cancellationToken) return dockerStream; }, logger); + // Named pipes are local connections - disable proxy resolution + pipeHandler.UseProxy = false; + handler = pipeHandler; break; case "tcp": @@ -97,7 +100,7 @@ await stream.ConnectAsync(timeout, cancellationToken) // ManagedHandler is required for hijacked stream operations (attach/exec/logs) // as it provides HttpConnectionResponseContent needed for connection hijacking. // SocketsHttpHandler cannot support hijacking because it encapsulates the transport stream. - handler = new ManagedHandler(async (_, _, cancellationToken) => + var unixHandler = new ManagedHandler(async (_, _, cancellationToken) => { var endpoint = new Microsoft.Net.Http.Client.UnixDomainSocketEndPoint(pipeString); var socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); @@ -145,6 +148,9 @@ await socket.ConnectAsync(endpoint, timeoutCts.Token) throw; } }, logger, socketConfig); + // Unix sockets are local connections - disable proxy resolution + unixHandler.UseProxy = false; + handler = unixHandler; break; default: From 018aa5a4085cb27f8393d799dea7aab5f43aee4c Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 20 Jan 2026 14:22:02 +0000 Subject: [PATCH 15/18] refactor: Rename SocketConfiguration to SocketConnectionConfiguration Rename for clarity - the class configures socket connection behavior, not sockets themselves. This is a new class introduced in this PR, so no breaking change concerns. Co-Authored-By: Claude Opus 4.5 --- src/Docker.DotNet/DockerClient.cs | 6 +++--- src/Docker.DotNet/DockerClientConfiguration.cs | 8 ++++---- .../Microsoft.Net.Http.Client/ManagedHandler.cs | 16 ++++++++-------- ...ation.cs => SocketConnectionConfiguration.cs} | 6 +++--- test/Docker.DotNet.Tests/ManagedHandlerTests.cs | 16 ++++++++-------- 5 files changed, 26 insertions(+), 26 deletions(-) rename src/Docker.DotNet/{SocketConfiguration.cs => SocketConnectionConfiguration.cs} (96%) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 99e45530..500db25e 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -83,17 +83,17 @@ await stream.ConnectAsync(timeout, cancellationToken) Scheme = configuration.Credentials.IsTlsCredentials() ? "https" : "http" }; uri = builder.Uri; - handler = new ManagedHandler(logger, Configuration.SocketConfiguration); + handler = new ManagedHandler(logger, Configuration.SocketConnectionConfiguration); break; case "https": - handler = new ManagedHandler(logger, Configuration.SocketConfiguration); + handler = new ManagedHandler(logger, Configuration.SocketConnectionConfiguration); break; case "unix": var pipeString = uri.LocalPath; var socketTimeout = Configuration.SocketConnectTimeout; - var socketConfig = Configuration.SocketConfiguration; + var socketConfig = Configuration.SocketConnectionConfiguration; uri = new UriBuilder("http", uri.Segments.Last()).Uri; // Use ManagedHandler for Unix socket connections. diff --git a/src/Docker.DotNet/DockerClientConfiguration.cs b/src/Docker.DotNet/DockerClientConfiguration.cs index b3f22bf7..9e6b12d3 100644 --- a/src/Docker.DotNet/DockerClientConfiguration.cs +++ b/src/Docker.DotNet/DockerClientConfiguration.cs @@ -10,7 +10,7 @@ public DockerClientConfiguration( TimeSpan namedPipeConnectTimeout = default, TimeSpan socketConnectTimeout = default, IReadOnlyDictionary defaultHttpRequestHeaders = null, - SocketConfiguration socketConfiguration = null) + SocketConnectionConfiguration socketConfiguration = null) : this(GetLocalDockerEndpoint(), credentials, defaultTimeout, namedPipeConnectTimeout, socketConnectTimeout, defaultHttpRequestHeaders, socketConfiguration) { } @@ -22,7 +22,7 @@ public DockerClientConfiguration( TimeSpan namedPipeConnectTimeout = default, TimeSpan socketConnectTimeout = default, IReadOnlyDictionary defaultHttpRequestHeaders = null, - SocketConfiguration socketConfiguration = null) + SocketConnectionConfiguration socketConfiguration = null) { if (endpoint == null) { @@ -40,7 +40,7 @@ public DockerClientConfiguration( NamedPipeConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, namedPipeConnectTimeout) ? TimeSpan.FromMilliseconds(100) : namedPipeConnectTimeout; SocketConnectTimeout = TimeSpan.Equals(TimeSpan.Zero, socketConnectTimeout) ? TimeSpan.FromSeconds(30) : socketConnectTimeout; DefaultHttpRequestHeaders = defaultHttpRequestHeaders ?? new Dictionary(); - SocketConfiguration = socketConfiguration ?? SocketConfiguration.Default; + SocketConnectionConfiguration = socketConfiguration ?? SocketConnectionConfiguration.Default; } /// @@ -72,7 +72,7 @@ public DockerClientConfiguration( /// Gets the socket configuration options for connection handling. /// These settings help improve proxy compatibility and connection reliability. /// - public SocketConfiguration SocketConfiguration { get; } + public SocketConnectionConfiguration SocketConnectionConfiguration { get; } /// /// Gets the collection of default HTTP request headers. diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index 7e12a741..f595310c 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -11,7 +11,7 @@ public class ManagedHandler : HttpMessageHandler private readonly SocketOpener _socketOpener; - private readonly SocketConfiguration _socketConfiguration; + private readonly SocketConnectionConfiguration _socketConfiguration; private IWebProxy _proxy; @@ -20,35 +20,35 @@ public class ManagedHandler : HttpMessageHandler public delegate Task SocketOpener(string host, int port, CancellationToken cancellationToken); public ManagedHandler(ILogger logger) - : this(logger, SocketConfiguration.Default) + : this(logger, SocketConnectionConfiguration.Default) { } - public ManagedHandler(ILogger logger, SocketConfiguration socketConfiguration) + public ManagedHandler(ILogger logger, SocketConnectionConfiguration socketConfiguration) { _logger = logger; - _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; + _socketConfiguration = socketConfiguration ?? SocketConnectionConfiguration.Default; _socketOpener = TcpSocketOpenerAsync; } public ManagedHandler(StreamOpener opener, ILogger logger) { _logger = logger; - _socketConfiguration = SocketConfiguration.Default; + _socketConfiguration = SocketConnectionConfiguration.Default; _streamOpener = opener ?? throw new ArgumentNullException(nameof(opener)); } public ManagedHandler(SocketOpener opener, ILogger logger) { _logger = logger; - _socketConfiguration = SocketConfiguration.Default; + _socketConfiguration = SocketConnectionConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); } - public ManagedHandler(SocketOpener opener, ILogger logger, SocketConfiguration socketConfiguration) + public ManagedHandler(SocketOpener opener, ILogger logger, SocketConnectionConfiguration socketConfiguration) { _logger = logger; - _socketConfiguration = socketConfiguration ?? SocketConfiguration.Default; + _socketConfiguration = socketConfiguration ?? SocketConnectionConfiguration.Default; _socketOpener = opener ?? throw new ArgumentNullException(nameof(opener)); } diff --git a/src/Docker.DotNet/SocketConfiguration.cs b/src/Docker.DotNet/SocketConnectionConfiguration.cs similarity index 96% rename from src/Docker.DotNet/SocketConfiguration.cs rename to src/Docker.DotNet/SocketConnectionConfiguration.cs index 627b2be0..1d185392 100644 --- a/src/Docker.DotNet/SocketConfiguration.cs +++ b/src/Docker.DotNet/SocketConnectionConfiguration.cs @@ -6,12 +6,12 @@ namespace Docker.DotNet; /// Configuration options for socket connections. /// These settings help improve proxy compatibility and connection reliability. /// -public sealed class SocketConfiguration +public sealed class SocketConnectionConfiguration { /// - /// Default socket configuration with sensible defaults for Docker socket connections. + /// Default socket connection configuration with sensible defaults for Docker socket connections. /// - public static SocketConfiguration Default { get; } = new SocketConfiguration(); + public static SocketConnectionConfiguration Default { get; } = new SocketConnectionConfiguration(); /// /// Gets or sets whether TCP keep-alive is enabled. diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index 722d81ee..d9c92f20 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -58,10 +58,10 @@ public void ManagedHandler_ConnectTimeout_CanBeDisabled() #if NET6_0_OR_GREATER [Fact] - public void SocketConfiguration_HappyEyeballs_DefaultEnabled() + public void SocketConnectionConfiguration_HappyEyeballs_DefaultEnabled() { // Arrange & Act - var config = new SocketConfiguration(); + var config = new SocketConnectionConfiguration(); // Assert Assert.True(config.EnableHappyEyeballs); @@ -69,10 +69,10 @@ public void SocketConfiguration_HappyEyeballs_DefaultEnabled() } [Fact] - public void SocketConfiguration_HappyEyeballs_CanBeDisabled() + public void SocketConnectionConfiguration_HappyEyeballs_CanBeDisabled() { // Arrange - var config = new SocketConfiguration(); + var config = new SocketConnectionConfiguration(); // Act config.EnableHappyEyeballs = false; @@ -82,10 +82,10 @@ public void SocketConfiguration_HappyEyeballs_CanBeDisabled() } [Fact] - public void SocketConfiguration_HappyEyeballsDelay_CanBeCustomized() + public void SocketConnectionConfiguration_HappyEyeballsDelay_CanBeCustomized() { // Arrange - var config = new SocketConfiguration(); + var config = new SocketConnectionConfiguration(); // Act config.HappyEyeballsDelay = TimeSpan.FromMilliseconds(500); @@ -169,10 +169,10 @@ public void ManagedHandler_FullConfiguration_AllPropertiesAccessible() } [Fact] - public void SocketConfiguration_FullConfiguration_AllPropertiesAccessible() + public void SocketConnectionConfiguration_FullConfiguration_AllPropertiesAccessible() { // Arrange & Act - var config = new SocketConfiguration(); + var config = new SocketConnectionConfiguration(); // Assert - All properties should be accessible Assert.True(config.KeepAlive); From 104f89a467159404072c63a5c0cfeb515064ac6b Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 25 Jan 2026 13:47:47 +0000 Subject: [PATCH 16/18] fix: Address PR feedback for socket handling improvements - Fix Happy Eyeballs cleanup to properly dispose non-winning sockets - Make SocketConnectionConfiguration.Default return new instance each time - Apply full socket configuration to Unix sockets via ApplyTo() - Fix formatting: remove double blank line, add trailing newlines Co-Authored-By: Claude Opus 4.5 --- src/Docker.DotNet/DockerClient.cs | 5 +-- .../BufferedReadStream.cs | 1 - .../ChunkedReadStream.cs | 2 +- .../ChunkedWriteStream.cs | 2 +- .../ManagedHandler.cs | 33 +++++++++++-------- .../SocketConnectionConfiguration.cs | 3 +- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/Docker.DotNet/DockerClient.cs b/src/Docker.DotNet/DockerClient.cs index 500db25e..2dd1637f 100644 --- a/src/Docker.DotNet/DockerClient.cs +++ b/src/Docker.DotNet/DockerClient.cs @@ -108,10 +108,7 @@ await stream.ConnectAsync(timeout, cancellationToken) try { // Apply socket configuration for better proxy compatibility - if (socketConfig.KeepAlive) - { - socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); - } + socketConfig.ApplyTo(socket); using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); timeoutCts.CancelAfter(socketTimeout); diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs index bfdffdf3..f69e45bd 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/BufferedReadStream.cs @@ -11,7 +11,6 @@ internal sealed class BufferedReadStream : WriteClosableStream, IPeekableStream private int _bufferCount; private bool _disposed; - public BufferedReadStream(Stream inner, Socket socket, ILogger logger) : this(inner, socket, 8192, logger) { diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs index 495b8c6e..0ed59f71 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedReadStream.cs @@ -157,4 +157,4 @@ protected override void Dispose(bool disposing) } base.Dispose(disposing); } -} \ No newline at end of file +} diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs index c231ab51..2bf147b7 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ChunkedWriteStream.cs @@ -98,4 +98,4 @@ protected override void Dispose(bool disposing) } base.Dispose(disposing); } -} \ No newline at end of file +} diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index f595310c..dde9bf25 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -511,13 +511,14 @@ private async Task ConnectWithHappyEyeballsAsync( } }, cts.Token); - // Wait for either to complete - var completedTasks = new List> { ipv6Task, ipv4Task }; + // Keep track of all tasks for cleanup + var allTasks = new List> { ipv6Task, ipv4Task }; + var pendingTasks = new List>(allTasks); - while (completedTasks.Count > 0) + while (pendingTasks.Count > 0) { - var completedTask = await Task.WhenAny(completedTasks).ConfigureAwait(false); - completedTasks.Remove(completedTask); + var completedTask = await Task.WhenAny(pendingTasks).ConfigureAwait(false); + pendingTasks.Remove(completedTask); try { @@ -536,19 +537,23 @@ private async Task ConnectWithHappyEyeballsAsync( } // Clean up any remaining sockets from tasks that didn't win - foreach (var task in completedTasks) + foreach (var task in allTasks) { - try + // Skip the winning task + if (winningSocket != null && task.IsCompletedSuccessfully) { - if (task.IsCompletedSuccessfully) + try { var socket = await task.ConfigureAwait(false); - socket?.Dispose(); + if (socket != null && socket != winningSocket) + { + socket.Dispose(); + } + } + catch + { + // Ignore cleanup errors } - } - catch - { - // Ignore cleanup errors } } @@ -631,4 +636,4 @@ protected override void Dispose(bool disposing) { base.Dispose(disposing); } -} \ No newline at end of file +} diff --git a/src/Docker.DotNet/SocketConnectionConfiguration.cs b/src/Docker.DotNet/SocketConnectionConfiguration.cs index 1d185392..4bd7cdbd 100644 --- a/src/Docker.DotNet/SocketConnectionConfiguration.cs +++ b/src/Docker.DotNet/SocketConnectionConfiguration.cs @@ -10,8 +10,9 @@ public sealed class SocketConnectionConfiguration { /// /// Default socket connection configuration with sensible defaults for Docker socket connections. + /// Returns a new instance each time to prevent shared state mutation. /// - public static SocketConnectionConfiguration Default { get; } = new SocketConnectionConfiguration(); + public static SocketConnectionConfiguration Default => new SocketConnectionConfiguration(); /// /// Gets or sets whether TCP keep-alive is enabled. From d9a0654719fca0114f66a8d033c762b385035843 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 25 Jan 2026 13:52:31 +0000 Subject: [PATCH 17/18] revert: Keep SocketConnectionConfiguration.Default as singleton Reverts the change to return a new instance each time - keeping PR focused. Co-Authored-By: Claude Opus 4.5 --- src/Docker.DotNet/SocketConnectionConfiguration.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Docker.DotNet/SocketConnectionConfiguration.cs b/src/Docker.DotNet/SocketConnectionConfiguration.cs index 4bd7cdbd..1d185392 100644 --- a/src/Docker.DotNet/SocketConnectionConfiguration.cs +++ b/src/Docker.DotNet/SocketConnectionConfiguration.cs @@ -10,9 +10,8 @@ public sealed class SocketConnectionConfiguration { /// /// Default socket connection configuration with sensible defaults for Docker socket connections. - /// Returns a new instance each time to prevent shared state mutation. /// - public static SocketConnectionConfiguration Default => new SocketConnectionConfiguration(); + public static SocketConnectionConfiguration Default { get; } = new SocketConnectionConfiguration(); /// /// Gets or sets whether TCP keep-alive is enabled. From 31772e05a198739fce0be492a1e9ddc1d2172c87 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 25 Jan 2026 13:58:56 +0000 Subject: [PATCH 18/18] refactor: Remove ConnectTimeout to keep PR focused Remove ConnectTimeout feature and related tests to keep the PR focused on proxy compatibility improvements (SocketConnectionConfiguration, Happy Eyeballs, Unix socket handling). Co-Authored-By: Claude Opus 4.5 --- .../ManagedHandler.cs | 34 ++------------ .../ManagedHandlerTests.cs | 46 +------------------ 2 files changed, 6 insertions(+), 74 deletions(-) diff --git a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs index dde9bf25..12983917 100644 --- a/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs +++ b/src/Docker.DotNet/Microsoft.Net.Http.Client/ManagedHandler.cs @@ -84,12 +84,6 @@ public IWebProxy Proxy public X509CertificateCollection ClientCertificates { get; set; } = new X509Certificate2Collection(); - /// - /// Gets or sets the connection timeout. Default is 30 seconds. - /// Set to to disable connection timeout. - /// - public TimeSpan ConnectTimeout { get; set; } = TimeSpan.FromSeconds(30); - protected override async Task SendAsync(HttpRequestMessage httpRequestMessage, CancellationToken cancellationToken) { if (httpRequestMessage == null) @@ -173,39 +167,22 @@ private async Task ProcessRequestAsync(HttpRequestMessage r request.Headers.ConnectionClose = !request.Headers.Contains("Connection"); // TODO: Connection reuse is not supported. ProxyMode proxyMode = DetermineProxyModeAndAddressLine(request); - Socket socket = null; - Stream transport = null; - BufferedReadStream bufferedReadStream = null; - - // Create linked cancellation token for connection timeout - using var timeoutCts = ConnectTimeout != Timeout.InfiniteTimeSpan - ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken) - : null; - - if (timeoutCts != null) - { - timeoutCts.CancelAfter(ConnectTimeout); - } - - var effectiveCancellationToken = timeoutCts?.Token ?? cancellationToken; + Socket socket; + Stream transport; try { if (_socketOpener != null) { - socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); + socket = await _socketOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, cancellationToken).ConfigureAwait(false); transport = new NetworkStream(socket, true); } else { socket = null; - transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, effectiveCancellationToken).ConfigureAwait(false); + transport = await _streamOpener(request.GetConnectionHostProperty(), request.GetConnectionPortProperty().Value, cancellationToken).ConfigureAwait(false); } } - catch (OperationCanceledException) when (timeoutCts?.IsCancellationRequested == true && !cancellationToken.IsCancellationRequested) - { - throw new TimeoutException($"Connection to {request.GetConnectionHostProperty()}:{request.GetConnectionPortProperty()} timed out after {ConnectTimeout.TotalSeconds} seconds."); - } catch (SocketException e) { throw new HttpRequestException("Connection failed.", e); @@ -221,8 +198,7 @@ private async Task ProcessRequestAsync(HttpRequestMessage r transport = await EstablishSslAsync(transport, request.GetHostProperty(), cancellationToken).ConfigureAwait(false); } - bufferedReadStream = new BufferedReadStream(transport, socket, _logger); - + var bufferedReadStream = new BufferedReadStream(transport, socket, _logger); var connection = new HttpConnection(bufferedReadStream); return await connection.SendAsync(request, cancellationToken); } diff --git a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs index d9c92f20..43854395 100644 --- a/test/Docker.DotNet.Tests/ManagedHandlerTests.cs +++ b/test/Docker.DotNet.Tests/ManagedHandlerTests.cs @@ -11,50 +11,7 @@ namespace Docker.DotNet.Tests; /// public class ManagedHandlerTests { - #region Connection Timeout Tests - - [Fact] - public void ManagedHandler_ConnectTimeout_HasDefaultValue() - { - // Arrange & Act - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Assert - Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); - } - - [Fact] - public void ManagedHandler_ConnectTimeout_CanBeSet() - { - // Arrange - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Act - handler.ConnectTimeout = TimeSpan.FromSeconds(60); - - // Assert - Assert.Equal(TimeSpan.FromSeconds(60), handler.ConnectTimeout); - } - - [Fact] - public void ManagedHandler_ConnectTimeout_CanBeDisabled() - { - // Arrange - var logger = new Microsoft.Extensions.Logging.Abstractions.NullLogger(); - using var handler = new ManagedHandler(logger); - - // Act - handler.ConnectTimeout = Timeout.InfiniteTimeSpan; - - // Assert - Assert.Equal(Timeout.InfiniteTimeSpan, handler.ConnectTimeout); - } - - #endregion - - #region Phase 4: Happy Eyeballs Tests + #region Happy Eyeballs Tests #if NET6_0_OR_GREATER [Fact] @@ -165,7 +122,6 @@ public void ManagedHandler_FullConfiguration_AllPropertiesAccessible() Assert.Equal(RedirectMode.NoDowngrade, handler.RedirectMode); Assert.Null(handler.ServerCertificateValidationCallback); Assert.NotNull(handler.ClientCertificates); - Assert.Equal(TimeSpan.FromSeconds(30), handler.ConnectTimeout); } [Fact]