Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix polling on https connections in HttpConnectionPool #49474

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Fix polling on https connections in HttpConnectionPool #49474

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e478eb
Fix polling on https connections in HttpConnectionPool
stephentoub Mar 11, 2021
dfc1588
Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpH…
stephentoub Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ internal partial class HttpConnection : HttpConnectionBase, IDisposable

public HttpConnection(
HttpConnectionPool pool,
Socket? socket,
Stream stream,
TransportContext? transportContext)
{
Expand All @@ -79,10 +80,7 @@ public HttpConnection(

_pool = pool;
_stream = stream;
if (stream is NetworkStream networkStream)
{
_socket = networkStream.Socket;
}
_socket = socket;

_transportContext = transportContext;

Expand Down
46 changes: 31 additions & 15 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ private static bool IsUsableHttp11Connection(HttpConnection connection, long now
}

// Try to establish an HTTP2 connection
Socket? socket = null;
Stream? stream = null;
SslStream? sslStream = null;
TransportContext? transportContext = null;
Expand All @@ -591,7 +592,7 @@ private static bool IsUsableHttp11Connection(HttpConnection connection, long now

HttpResponseMessage? failureResponse;

(stream, transportContext, failureResponse) =
(socket, stream, transportContext, failureResponse) =
await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false);

if (failureResponse != null)
Expand Down Expand Up @@ -681,7 +682,7 @@ private static bool IsUsableHttp11Connection(HttpConnection connection, long now

if (canUse)
{
return (await ConstructHttp11ConnectionAsync(async, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true, null);
return (await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true, null);
}
else
{
Expand Down Expand Up @@ -1213,7 +1214,7 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
return SendWithProxyAuthAsync(request, async, doRequestAuth, cancellationToken);
}

private async ValueTask<(Stream?, TransportContext?, HttpResponseMessage?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
private async ValueTask<(Socket?, Stream?, TransportContext?, HttpResponseMessage?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
{
// If a non-infinite connect timeout has been set, create and use a new CancellationToken that will be canceled
// when either the original token is canceled or a connect timeout occurs.
Expand All @@ -1228,17 +1229,18 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
try
{
Stream? stream = null;
Socket? socket = null;
switch (_kind)
{
case HttpConnectionKind.Http:
case HttpConnectionKind.Https:
case HttpConnectionKind.ProxyConnect:
Debug.Assert(_originAuthority != null);
stream = await ConnectToTcpHostAsync(_originAuthority.IdnHost, _originAuthority.Port, request, async, cancellationToken).ConfigureAwait(false);
(socket, stream) = await ConnectToTcpHostAsync(_originAuthority.IdnHost, _originAuthority.Port, request, async, cancellationToken).ConfigureAwait(false);
break;

case HttpConnectionKind.Proxy:
stream = await ConnectToTcpHostAsync(_proxyUri!.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
(socket, stream) = await ConnectToTcpHostAsync(_proxyUri!.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
break;

case HttpConnectionKind.ProxyTunnel:
Expand All @@ -1249,12 +1251,18 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
{
// Return non-success response from proxy.
response.RequestMessage = request;
return (null, null, response);
return (null, null, null, response);
}
break;
}

Debug.Assert(stream != null);
if (socket is null && stream is NetworkStream ns)
{
// We weren't handed a socket directly. But if we're able to extract one, do so.
// Most likely case here is a ConnectCallback was used and returned a NetworkStream.
socket = ns.Socket;
}

TransportContext? transportContext = null;
if (IsSecure)
Expand All @@ -1264,20 +1272,21 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
stream = sslStream;
}

return (stream, transportContext, null);
return (socket, stream, transportContext, null);
}
finally
{
cancellationWithConnectTimeout?.Dispose();
}
}

private async ValueTask<Stream> ConnectToTcpHostAsync(string host, int port, HttpRequestMessage initialRequest, bool async, CancellationToken cancellationToken)
private async ValueTask<(Socket?, Stream)> ConnectToTcpHostAsync(string host, int port, HttpRequestMessage initialRequest, bool async, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

var endPoint = new DnsEndPoint(host, port);
Socket? socket = null;
Stream? stream = null;
try
{
// If a ConnectCallback was supplied, use that to establish the connection.
Expand All @@ -1294,7 +1303,7 @@ private async ValueTask<Stream> ConnectToTcpHostAsync(string host, int port, Htt
Trace($"{nameof(SocketsHttpHandler.ConnectCallback)} completing asynchronously for a synchronous request.");
}

return await streamTask.ConfigureAwait(false) ?? throw new HttpRequestException(SR.net_http_null_from_connect_callback);
stream = await streamTask.ConfigureAwait(false) ?? throw new HttpRequestException(SR.net_http_null_from_connect_callback);
}
else
{
Expand All @@ -1313,8 +1322,10 @@ private async ValueTask<Stream> ConnectToTcpHostAsync(string host, int port, Htt
}
}

return new NetworkStream(socket, ownsSocket: true);
stream = new NetworkStream(socket, ownsSocket: true);
}

return (socket, stream);
}
catch (Exception ex)
{
Expand All @@ -1327,15 +1338,15 @@ private async ValueTask<Stream> ConnectToTcpHostAsync(string host, int port, Htt

internal async ValueTask<(HttpConnection?, HttpResponseMessage?)> CreateHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
{
(Stream? stream, TransportContext? transportContext, HttpResponseMessage? failureResponse) =
(Socket? socket, Stream? stream, TransportContext? transportContext, HttpResponseMessage? failureResponse) =
await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false);

if (failureResponse != null)
{
return (null, failureResponse);
}

return (await ConstructHttp11ConnectionAsync(async, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), null);
return (await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), null);
}

private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessage request)
Expand Down Expand Up @@ -1393,10 +1404,15 @@ private async ValueTask<Stream> ApplyPlaintextFilterAsync(bool async, Stream str
return newStream;
}

private async ValueTask<HttpConnection> ConstructHttp11ConnectionAsync(bool async, Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken)
private async ValueTask<HttpConnection> ConstructHttp11ConnectionAsync(bool async, Socket? socket, Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken)
{
stream = await ApplyPlaintextFilterAsync(async, stream, HttpVersion.Version11, request, cancellationToken).ConfigureAwait(false);
return new HttpConnection(this, stream, transportContext);
Stream newStream = await ApplyPlaintextFilterAsync(async, stream, HttpVersion.Version11, request, cancellationToken).ConfigureAwait(false);
if (newStream != stream)
{
// If a plaintext filter created a new stream, we can't trust that the socket is still applicable, so rely on it.
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
socket = null;
}
return new HttpConnection(this, socket, newStream, transportContext);
}

private async ValueTask<Http2Connection> ConstructHttp2ConnectionAsync(Stream stream, HttpRequestMessage request, CancellationToken cancellationToken)
Expand Down