Skip to content

Commit

Permalink
Tear down pending HTTP connection when the originating request comple…
Browse files Browse the repository at this point in the history
…tes (#71785)

Resolves #66297
  • Loading branch information
antonfirsov authored Jul 26, 2022
1 parent cc7ccfd commit 1729ca6
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 125 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace System.Threading.Tasks
/// <seealso cref="OperationCanceledException"/>s contain the relevant <see cref="CancellationToken"/>,
/// while also avoiding unnecessary allocations for closure captures.
/// </summary>
internal sealed class TaskCompletionSourceWithCancellation<T> : TaskCompletionSource<T>
internal class TaskCompletionSourceWithCancellation<T> : TaskCompletionSource<T>

This comment has been minimized.

Copy link
@rhuijben

rhuijben Aug 8, 2022

Why did you remove the sealed here?

{
public TaskCompletionSourceWithCancellation() : base(TaskCreationOptions.RunContinuationsAsynchronously)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ public abstract class GenericLoopbackConnection : IAsyncDisposable
/// <summary>Waits for the client to signal cancellation.</summary>
public abstract Task WaitForCloseAsync(CancellationToken cancellationToken);

/// <summary>Reset the connection's internal state so it can process further requests.</summary>
public virtual void CompleteRequestProcessing() { }

/// <summary>Helper function to make it easier to convert old test with strings.</summary>
public async Task SendResponseBodyAsync(string content, bool isFinal = true)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ public override async Task<Byte[]> ReadRequestBodyAsync()
return buffer;
}

public void CompleteRequestProcessing()
public override void CompleteRequestProcessing()
{
_contentLength = 0;
_bodyRead = false;
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@
Link="Common\System\Net\HttpDateParser.cs" />
<Compile Include="$(CommonPath)System\Text\SimpleRegex.cs"
Link="Common\System\Text\SimpleRegex.cs" />
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
<Compile Include="$(CommonPath)System\HexConverter.cs"
Link="Common\System\HexConverter.cs" />
<Compile Include="$(CommonPath)System\Net\ArrayBuffer.cs"
Expand Down Expand Up @@ -226,6 +224,8 @@
Link="Common\System\Net\DebugSafeHandle.cs" />
<Compile Include="$(CommonPath)System\Net\DebugSafeHandleZeroOrMinusOneIsInvalid.cs"
Link="Common\System\Net\DebugSafeHandleZeroOrMinusOneIsInvalid.cs" />
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
<!-- Header support -->
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs">
<Link>Common\System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ internal static class SocketsHttpHandler
// Defaults to 1.0. Higher values result in shorter window, but slower downloads.
public static double Http2StreamWindowScaleThresholdMultiplier { get; } = GetHttp2StreamWindowScaleThresholdMultiplier();

public static int PendingConnectionTimeoutOnRequestCompletion { get; } = RuntimeSettingParser.QueryRuntimeSettingInt32(
"System.Net.SocketsHttpHandler.PendingConnectionTimeoutOnRequestCompletion",
"DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_PENDINGCONNECTIONTIMEOUTONREQUESTCOMPLETION", 5000);

public const int DefaultHttp2MaxStreamWindowSize = 16 * 1024 * 1024;
public const double DefaultHttp2StreamWindowScaleThresholdMultiplier = 1.0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2074,7 +2074,7 @@ private void ReturnConnectionToPool()
_idleSinceTickCount = Environment.TickCount64;

// Put connection back in the pool.
_pool.ReturnHttp11Connection(this, isNewConnection: false);
_pool.RecycleHttp11Connection(this);
}
}

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Net.Sockets;
using System.Net.Test.Common;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.DotNet.RemoteExecutor;
using Xunit;
using Xunit.Abstractions;

namespace System.Net.Http.Functional.Tests
{
[Collection(nameof(DisableParallelization))] // Reduces chance of timing-related issues
[ConditionalClass(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))]
public class SocketsHttpHandler_Cancellation_Test_NonParallel : HttpClientHandlerTestBase
{
public SocketsHttpHandler_Cancellation_Test_NonParallel(ITestOutputHelper output) : base(output)
{
}

[OuterLoop("Incurs significant delay.")]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData("1.1", 10_000, 1_000, 100)]
[InlineData("2.0", 10_000, 1_000, 100)]
[InlineData("1.1", 20_000, 10_000, null)]
[InlineData("2.0", 20_000, 10_000, null)]
public static void CancelPendingRequest_DropsStalledConnectionAttempt(string versionString, int firstConnectionDelayMs, int requestTimeoutMs, int? pendingConnectionTimeoutOnRequestCompletion)
{
RemoteInvokeOptions options = new RemoteInvokeOptions();
if (pendingConnectionTimeoutOnRequestCompletion is not null)
{
options.StartInfo.EnvironmentVariables["DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_PENDINGCONNECTIONTIMEOUTONREQUESTCOMPLETION"] = pendingConnectionTimeoutOnRequestCompletion.ToString();
}

RemoteExecutor.Invoke(CancelPendingRequest_DropsStalledConnectionAttempt_Impl, versionString, firstConnectionDelayMs.ToString(), requestTimeoutMs.ToString(), options).Dispose();
}

private static async Task CancelPendingRequest_DropsStalledConnectionAttempt_Impl(string versionString, string firstConnectionDelayMsString, string requestTimeoutMsString)
{
var version = Version.Parse(versionString);
LoopbackServerFactory factory = GetFactoryForVersion(version);

const int AttemptCount = 3;
int firstConnectionDelayMs = int.Parse(firstConnectionDelayMsString);
int requestTimeoutMs = int.Parse(requestTimeoutMsString);
bool firstConnection = true;

using CancellationTokenSource cts0 = new CancellationTokenSource(requestTimeoutMs);

await factory.CreateClientAndServerAsync(async uri =>
{
using var handler = CreateHttpClientHandler(version);
GetUnderlyingSocketsHttpHandler(handler).ConnectCallback = DoConnect;
using var client = new HttpClient(handler) { DefaultRequestVersion = version };
await Assert.ThrowsAnyAsync<TaskCanceledException>(async () =>
{
await client.GetAsync(uri, cts0.Token);
});
for (int i = 0; i < AttemptCount; i++)
{
using var cts1 = new CancellationTokenSource(requestTimeoutMs);
using var response = await client.GetAsync(uri, cts1.Token);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
}
}, async server =>
{
await server.AcceptConnectionAsync(async connection =>
{
for (int i = 0; i < AttemptCount; i++)
{
await connection.ReadRequestDataAsync();
await connection.SendResponseAsync();
connection.CompleteRequestProcessing();
}
});
});

async ValueTask<Stream> DoConnect(SocketsHttpConnectionContext ctx, CancellationToken cancellationToken)
{
if (firstConnection)
{
firstConnection = false;
await Task.Delay(100, cancellationToken); // Wait for the request to be pushed to the queue
cts0.Cancel(); // cancel the first request faster than RequestTimeoutMs
await Task.Delay(firstConnectionDelayMs, cancellationToken); // Simulate stalled connection
}
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
await s.ConnectAsync(ctx.DnsEndPoint, cancellationToken);

return new NetworkStream(s, ownsSocket: true);
}
}

[OuterLoop("Incurs significant delay.")]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData(20_000)]
[InlineData(Timeout.Infinite)]
public void PendingConnectionTimeout_HighValue_PendingConnectionIsNotCancelled(int timeout)
{
RemoteExecutor.Invoke(async timoutStr =>
{
// Setup "infinite" timeout of int.MaxValue milliseconds
AppContext.SetData("System.Net.SocketsHttpHandler.PendingConnectionTimeoutOnRequestCompletion", int.Parse(timoutStr));
bool connected = false;
CancellationTokenSource cts = new CancellationTokenSource();
await new Http11LoopbackServerFactory().CreateClientAndServerAsync(async uri =>
{
using var handler = CreateHttpClientHandler(HttpVersion.Version11);
GetUnderlyingSocketsHttpHandler(handler).ConnectCallback = DoConnect;
using var client = new HttpClient(handler) { DefaultRequestVersion = HttpVersion.Version11 };
await Assert.ThrowsAnyAsync<TaskCanceledException>(() => client.GetAsync(uri, cts.Token));
},
async server => {
await server.AcceptConnectionAsync(_ => Task.CompletedTask).WaitAsync(30_000);
});
async ValueTask<Stream> DoConnect(SocketsHttpConnectionContext ctx, CancellationToken cancellationToken)
{
var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
await Task.Delay(100, cancellationToken); // Wait for the request to be pushed to the queue
cts.Cancel();
await Task.Delay(10_000, cancellationToken);
await s.ConnectAsync(ctx.DnsEndPoint, cancellationToken);
connected = true;
return new NetworkStream(s, ownsSocket: true);
}
Assert.True(connected);
}, timeout.ToString()).Dispose();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<StringResourcesPath>../../src/Resources/Strings.resx</StringResourcesPath>
<DefineConstants>$(DefineConstants);SYSNETHTTP_NO_OPENSSL;HTTP3</DefineConstants>
Expand Down Expand Up @@ -159,6 +159,7 @@
Link="Common\TestUtilities\System\DisableParallelization.cs" />
<Compile Include="HttpClientHandlerTest.AltSvc.cs" />
<Compile Include="SocketsHttpHandlerTest.Cancellation.cs" />
<Compile Include="SocketsHttpHandlerTest.Cancellation.NonParallel.cs" />
<Compile Include="SocketsHttpHandlerTest.Http2FlowControl.cs" />
<Compile Include="SocketsHttpHandlerTest.Http2KeepAlivePing.cs" />
<Compile Include="HttpClientHandlerTest.Connect.cs" />
Expand Down

0 comments on commit 1729ca6

Please sign in to comment.