Skip to content

Commit 1729ca6

Browse files
authored
Tear down pending HTTP connection when the originating request completes (#71785)
Resolves #66297
1 parent cc7ccfd commit 1729ca6

File tree

9 files changed

+381
-125
lines changed

9 files changed

+381
-125
lines changed

src/libraries/Common/src/System/Threading/Tasks/TaskCompletionSourceWithCancellation.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace System.Threading.Tasks
88
/// <seealso cref="OperationCanceledException"/>s contain the relevant <see cref="CancellationToken"/>,
99
/// while also avoiding unnecessary allocations for closure captures.
1010
/// </summary>
11-
internal sealed class TaskCompletionSourceWithCancellation<T> : TaskCompletionSource<T>
11+
internal class TaskCompletionSourceWithCancellation<T> : TaskCompletionSource<T>
1212
{
1313
public TaskCompletionSourceWithCancellation() : base(TaskCreationOptions.RunContinuationsAsynchronously)
1414
{

src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ public abstract class GenericLoopbackConnection : IAsyncDisposable
152152
/// <summary>Waits for the client to signal cancellation.</summary>
153153
public abstract Task WaitForCloseAsync(CancellationToken cancellationToken);
154154

155+
/// <summary>Reset the connection's internal state so it can process further requests.</summary>
156+
public virtual void CompleteRequestProcessing() { }
157+
155158
/// <summary>Helper function to make it easier to convert old test with strings.</summary>
156159
public async Task SendResponseBodyAsync(string content, bool isFinal = true)
157160
{

src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ public override async Task<Byte[]> ReadRequestBodyAsync()
873873
return buffer;
874874
}
875875

876-
public void CompleteRequestProcessing()
876+
public override void CompleteRequestProcessing()
877877
{
878878
_contentLength = 0;
879879
_bodyRead = false;

src/libraries/System.Net.Http/src/System.Net.Http.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,6 @@
138138
Link="Common\System\Net\HttpDateParser.cs" />
139139
<Compile Include="$(CommonPath)System\Text\SimpleRegex.cs"
140140
Link="Common\System\Text\SimpleRegex.cs" />
141-
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
142-
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
143141
<Compile Include="$(CommonPath)System\HexConverter.cs"
144142
Link="Common\System\HexConverter.cs" />
145143
<Compile Include="$(CommonPath)System\Net\ArrayBuffer.cs"
@@ -226,6 +224,8 @@
226224
Link="Common\System\Net\DebugSafeHandle.cs" />
227225
<Compile Include="$(CommonPath)System\Net\DebugSafeHandleZeroOrMinusOneIsInvalid.cs"
228226
Link="Common\System\Net\DebugSafeHandleZeroOrMinusOneIsInvalid.cs" />
227+
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
228+
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
229229
<!-- Header support -->
230230
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs">
231231
<Link>Common\System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs</Link>

src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ internal static class SocketsHttpHandler
4545
// Defaults to 1.0. Higher values result in shorter window, but slower downloads.
4646
public static double Http2StreamWindowScaleThresholdMultiplier { get; } = GetHttp2StreamWindowScaleThresholdMultiplier();
4747

48+
public static int PendingConnectionTimeoutOnRequestCompletion { get; } = RuntimeSettingParser.QueryRuntimeSettingInt32(
49+
"System.Net.SocketsHttpHandler.PendingConnectionTimeoutOnRequestCompletion",
50+
"DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_PENDINGCONNECTIONTIMEOUTONREQUESTCOMPLETION", 5000);
51+
4852
public const int DefaultHttp2MaxStreamWindowSize = 16 * 1024 * 1024;
4953
public const double DefaultHttp2StreamWindowScaleThresholdMultiplier = 1.0;
5054

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2074,7 +2074,7 @@ private void ReturnConnectionToPool()
20742074
_idleSinceTickCount = Environment.TickCount64;
20752075

20762076
// Put connection back in the pool.
2077-
_pool.ReturnHttp11Connection(this, isNewConnection: false);
2077+
_pool.RecycleHttp11Connection(this);
20782078
}
20792079
}
20802080

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

Lines changed: 227 additions & 119 deletions
Large diffs are not rendered by default.
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.IO;
5+
using System.Net.Sockets;
6+
using System.Net.Test.Common;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
using Microsoft.DotNet.RemoteExecutor;
10+
using Xunit;
11+
using Xunit.Abstractions;
12+
13+
namespace System.Net.Http.Functional.Tests
14+
{
15+
[Collection(nameof(DisableParallelization))] // Reduces chance of timing-related issues
16+
[ConditionalClass(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))]
17+
public class SocketsHttpHandler_Cancellation_Test_NonParallel : HttpClientHandlerTestBase
18+
{
19+
public SocketsHttpHandler_Cancellation_Test_NonParallel(ITestOutputHelper output) : base(output)
20+
{
21+
}
22+
23+
[OuterLoop("Incurs significant delay.")]
24+
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
25+
[InlineData("1.1", 10_000, 1_000, 100)]
26+
[InlineData("2.0", 10_000, 1_000, 100)]
27+
[InlineData("1.1", 20_000, 10_000, null)]
28+
[InlineData("2.0", 20_000, 10_000, null)]
29+
public static void CancelPendingRequest_DropsStalledConnectionAttempt(string versionString, int firstConnectionDelayMs, int requestTimeoutMs, int? pendingConnectionTimeoutOnRequestCompletion)
30+
{
31+
RemoteInvokeOptions options = new RemoteInvokeOptions();
32+
if (pendingConnectionTimeoutOnRequestCompletion is not null)
33+
{
34+
options.StartInfo.EnvironmentVariables["DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_PENDINGCONNECTIONTIMEOUTONREQUESTCOMPLETION"] = pendingConnectionTimeoutOnRequestCompletion.ToString();
35+
}
36+
37+
RemoteExecutor.Invoke(CancelPendingRequest_DropsStalledConnectionAttempt_Impl, versionString, firstConnectionDelayMs.ToString(), requestTimeoutMs.ToString(), options).Dispose();
38+
}
39+
40+
private static async Task CancelPendingRequest_DropsStalledConnectionAttempt_Impl(string versionString, string firstConnectionDelayMsString, string requestTimeoutMsString)
41+
{
42+
var version = Version.Parse(versionString);
43+
LoopbackServerFactory factory = GetFactoryForVersion(version);
44+
45+
const int AttemptCount = 3;
46+
int firstConnectionDelayMs = int.Parse(firstConnectionDelayMsString);
47+
int requestTimeoutMs = int.Parse(requestTimeoutMsString);
48+
bool firstConnection = true;
49+
50+
using CancellationTokenSource cts0 = new CancellationTokenSource(requestTimeoutMs);
51+
52+
await factory.CreateClientAndServerAsync(async uri =>
53+
{
54+
using var handler = CreateHttpClientHandler(version);
55+
GetUnderlyingSocketsHttpHandler(handler).ConnectCallback = DoConnect;
56+
using var client = new HttpClient(handler) { DefaultRequestVersion = version };
57+
58+
await Assert.ThrowsAnyAsync<TaskCanceledException>(async () =>
59+
{
60+
await client.GetAsync(uri, cts0.Token);
61+
});
62+
63+
for (int i = 0; i < AttemptCount; i++)
64+
{
65+
using var cts1 = new CancellationTokenSource(requestTimeoutMs);
66+
using var response = await client.GetAsync(uri, cts1.Token);
67+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
68+
}
69+
}, async server =>
70+
{
71+
await server.AcceptConnectionAsync(async connection =>
72+
{
73+
for (int i = 0; i < AttemptCount; i++)
74+
{
75+
await connection.ReadRequestDataAsync();
76+
await connection.SendResponseAsync();
77+
connection.CompleteRequestProcessing();
78+
}
79+
});
80+
});
81+
82+
async ValueTask<Stream> DoConnect(SocketsHttpConnectionContext ctx, CancellationToken cancellationToken)
83+
{
84+
if (firstConnection)
85+
{
86+
firstConnection = false;
87+
await Task.Delay(100, cancellationToken); // Wait for the request to be pushed to the queue
88+
cts0.Cancel(); // cancel the first request faster than RequestTimeoutMs
89+
await Task.Delay(firstConnectionDelayMs, cancellationToken); // Simulate stalled connection
90+
}
91+
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
92+
await s.ConnectAsync(ctx.DnsEndPoint, cancellationToken);
93+
94+
return new NetworkStream(s, ownsSocket: true);
95+
}
96+
}
97+
98+
[OuterLoop("Incurs significant delay.")]
99+
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
100+
[InlineData(20_000)]
101+
[InlineData(Timeout.Infinite)]
102+
public void PendingConnectionTimeout_HighValue_PendingConnectionIsNotCancelled(int timeout)
103+
{
104+
RemoteExecutor.Invoke(async timoutStr =>
105+
{
106+
// Setup "infinite" timeout of int.MaxValue milliseconds
107+
AppContext.SetData("System.Net.SocketsHttpHandler.PendingConnectionTimeoutOnRequestCompletion", int.Parse(timoutStr));
108+
109+
bool connected = false;
110+
CancellationTokenSource cts = new CancellationTokenSource();
111+
112+
await new Http11LoopbackServerFactory().CreateClientAndServerAsync(async uri =>
113+
{
114+
using var handler = CreateHttpClientHandler(HttpVersion.Version11);
115+
GetUnderlyingSocketsHttpHandler(handler).ConnectCallback = DoConnect;
116+
using var client = new HttpClient(handler) { DefaultRequestVersion = HttpVersion.Version11 };
117+
118+
await Assert.ThrowsAnyAsync<TaskCanceledException>(() => client.GetAsync(uri, cts.Token));
119+
},
120+
async server => {
121+
await server.AcceptConnectionAsync(_ => Task.CompletedTask).WaitAsync(30_000);
122+
});
123+
124+
async ValueTask<Stream> DoConnect(SocketsHttpConnectionContext ctx, CancellationToken cancellationToken)
125+
{
126+
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
127+
await Task.Delay(100, cancellationToken); // Wait for the request to be pushed to the queue
128+
cts.Cancel();
129+
130+
await Task.Delay(10_000, cancellationToken);
131+
await s.ConnectAsync(ctx.DnsEndPoint, cancellationToken);
132+
connected = true;
133+
return new NetworkStream(s, ownsSocket: true);
134+
}
135+
136+
Assert.True(connected);
137+
}, timeout.ToString()).Dispose();
138+
}
139+
}
140+
}

src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<StringResourcesPath>../../src/Resources/Strings.resx</StringResourcesPath>
44
<DefineConstants>$(DefineConstants);SYSNETHTTP_NO_OPENSSL;HTTP3</DefineConstants>
@@ -159,6 +159,7 @@
159159
Link="Common\TestUtilities\System\DisableParallelization.cs" />
160160
<Compile Include="HttpClientHandlerTest.AltSvc.cs" />
161161
<Compile Include="SocketsHttpHandlerTest.Cancellation.cs" />
162+
<Compile Include="SocketsHttpHandlerTest.Cancellation.NonParallel.cs" />
162163
<Compile Include="SocketsHttpHandlerTest.Http2FlowControl.cs" />
163164
<Compile Include="SocketsHttpHandlerTest.Http2KeepAlivePing.cs" />
164165
<Compile Include="HttpClientHandlerTest.Connect.cs" />

0 commit comments

Comments
 (0)