Skip to content

Commit bc5e386

Browse files
geoffkizerGeoffrey Kizer
andauthored
Remove _socket field from HttpConnection (#66416)
* remove _socket from HttpConnection Co-authored-by: Geoffrey Kizer <geoffrek@windows.microsoft.com>
1 parent c28b13f commit bc5e386

File tree

2 files changed

+38
-49
lines changed

2 files changed

+38
-49
lines changed

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ internal sealed partial class HttpConnection : HttpConnectionBase
4343
private static readonly ulong s_http11Bytes = BitConverter.ToUInt64(Encoding.ASCII.GetBytes("HTTP/1.1"));
4444

4545
private readonly HttpConnectionPool _pool;
46-
private readonly Socket? _socket; // used for polling; _stream should be used for all reading/writing. _stream owns disposal.
4746
private readonly Stream _stream;
4847
private readonly TransportContext? _transportContext;
4948
private readonly WeakReference<HttpConnection> _weakThisRef;
@@ -74,7 +73,6 @@ internal sealed partial class HttpConnection : HttpConnectionBase
7473

7574
public HttpConnection(
7675
HttpConnectionPool pool,
77-
Socket? socket,
7876
Stream stream,
7977
TransportContext? transportContext)
8078
{
@@ -83,7 +81,6 @@ public HttpConnection(
8381

8482
_pool = pool;
8583
_stream = stream;
86-
_socket = socket;
8784

8885
_transportContext = transportContext;
8986

@@ -158,13 +155,13 @@ public bool PrepareForReuse(bool async)
158155
// Check to see if we've received anything on the connection; if we have, that's
159156
// either erroneous data (we shouldn't have received anything yet) or the connection
160157
// has been closed; either way, we can't use it.
161-
if (!async && _socket is not null)
158+
if (!async && _stream is NetworkStream networkStream)
162159
{
163160
// Directly poll the socket rather than doing an async read, so that we can
164161
// issue an appropriate sync read when we actually need it.
165162
try
166163
{
167-
return !_socket.Poll(0, SelectMode.SelectRead);
164+
return !networkStream.Socket.Poll(0, SelectMode.SelectRead);
168165
}
169166
catch (Exception e) when (e is SocketException || e is ObjectDisposedException)
170167
{

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

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ private async ValueTask<HttpConnection> GetHttp11ConnectionAsync(HttpRequestMess
555555
}
556556
}
557557

558-
private async Task HandleHttp11Downgrade(HttpRequestMessage request, Socket? socket, Stream stream, TransportContext? transportContext, CancellationToken cancellationToken)
558+
private async Task HandleHttp11Downgrade(HttpRequestMessage request, Stream stream, TransportContext? transportContext, CancellationToken cancellationToken)
559559
{
560560
if (NetEventSource.Log.IsEnabled()) Trace("Server does not support HTTP2; disabling HTTP2 use and proceeding with HTTP/1.1 connection");
561561

@@ -610,7 +610,7 @@ private async Task HandleHttp11Downgrade(HttpRequestMessage request, Socket? soc
610610
try
611611
{
612612
// Note, the same CancellationToken from the original HTTP2 connection establishment still applies here.
613-
http11Connection = await ConstructHttp11ConnectionAsync(true, socket, stream, transportContext, request, cancellationToken).ConfigureAwait(false);
613+
http11Connection = await ConstructHttp11ConnectionAsync(true, stream, transportContext, request, cancellationToken).ConfigureAwait(false);
614614
}
615615
catch (OperationCanceledException oce) when (oce.CancellationToken == cancellationToken)
616616
{
@@ -635,7 +635,7 @@ private async Task AddHttp2ConnectionAsync(HttpRequestMessage request)
635635
{
636636
try
637637
{
638-
(Socket? socket, Stream stream, TransportContext? transportContext) = await ConnectAsync(request, true, cts.Token).ConfigureAwait(false);
638+
(Stream stream, TransportContext? transportContext) = await ConnectAsync(request, true, cts.Token).ConfigureAwait(false);
639639

640640
if (IsSecure)
641641
{
@@ -656,7 +656,7 @@ private async Task AddHttp2ConnectionAsync(HttpRequestMessage request)
656656
else
657657
{
658658
// We established an SSL connection, but the server denied our request for HTTP2.
659-
await HandleHttp11Downgrade(request, socket, stream, transportContext, cts.Token).ConfigureAwait(false);
659+
await HandleHttp11Downgrade(request, stream, transportContext, cts.Token).ConfigureAwait(false);
660660
return;
661661
}
662662
}
@@ -1347,21 +1347,20 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
13471347

13481348
private CancellationTokenSource GetConnectTimeoutCancellationTokenSource() => new CancellationTokenSource(Settings._connectTimeout);
13491349

1350-
private async ValueTask<(Socket?, Stream, TransportContext?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
1350+
private async ValueTask<(Stream, TransportContext?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
13511351
{
13521352
Stream? stream = null;
1353-
Socket? socket = null;
13541353
switch (_kind)
13551354
{
13561355
case HttpConnectionKind.Http:
13571356
case HttpConnectionKind.Https:
13581357
case HttpConnectionKind.ProxyConnect:
13591358
Debug.Assert(_originAuthority != null);
1360-
(socket, stream) = await ConnectToTcpHostAsync(_originAuthority.IdnHost, _originAuthority.Port, request, async, cancellationToken).ConfigureAwait(false);
1359+
stream = await ConnectToTcpHostAsync(_originAuthority.IdnHost, _originAuthority.Port, request, async, cancellationToken).ConfigureAwait(false);
13611360
break;
13621361

13631362
case HttpConnectionKind.Proxy:
1364-
(socket, stream) = await ConnectToTcpHostAsync(_proxyUri!.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
1363+
stream = await ConnectToTcpHostAsync(_proxyUri!.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
13651364
break;
13661365

13671366
case HttpConnectionKind.ProxyTunnel:
@@ -1371,17 +1370,11 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
13711370

13721371
case HttpConnectionKind.SocksTunnel:
13731372
case HttpConnectionKind.SslSocksTunnel:
1374-
(socket, stream) = await EstablishSocksTunnel(request, async, cancellationToken).ConfigureAwait(false);
1373+
stream = await EstablishSocksTunnel(request, async, cancellationToken).ConfigureAwait(false);
13751374
break;
13761375
}
13771376

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

13861379
TransportContext? transportContext = null;
13871380
if (IsSecure)
@@ -1402,15 +1395,14 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
14021395
stream = sslStream;
14031396
}
14041397

1405-
return (socket, stream, transportContext);
1398+
return (stream, transportContext);
14061399
}
14071400

1408-
private async ValueTask<(Socket?, Stream)> ConnectToTcpHostAsync(string host, int port, HttpRequestMessage initialRequest, bool async, CancellationToken cancellationToken)
1401+
private async ValueTask<Stream> ConnectToTcpHostAsync(string host, int port, HttpRequestMessage initialRequest, bool async, CancellationToken cancellationToken)
14091402
{
14101403
cancellationToken.ThrowIfCancellationRequested();
14111404

14121405
var endPoint = new DnsEndPoint(host, port);
1413-
Socket? socket = null;
14141406
Stream? stream = null;
14151407
try
14161408
{
@@ -1433,28 +1425,34 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
14331425
else
14341426
{
14351427
// Otherwise, create and connect a socket using default settings.
1436-
socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
1437-
1438-
if (async)
1439-
{
1440-
await socket.ConnectAsync(endPoint, cancellationToken).ConfigureAwait(false);
1441-
}
1442-
else
1428+
Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
1429+
try
14431430
{
1444-
using (cancellationToken.UnsafeRegister(static s => ((Socket)s!).Dispose(), socket))
1431+
if (async)
14451432
{
1446-
socket.Connect(endPoint);
1433+
await socket.ConnectAsync(endPoint, cancellationToken).ConfigureAwait(false);
1434+
}
1435+
else
1436+
{
1437+
using (cancellationToken.UnsafeRegister(static s => ((Socket)s!).Dispose(), socket))
1438+
{
1439+
socket.Connect(endPoint);
1440+
}
14471441
}
1448-
}
14491442

1450-
stream = new NetworkStream(socket, ownsSocket: true);
1443+
stream = new NetworkStream(socket, ownsSocket: true);
1444+
}
1445+
catch
1446+
{
1447+
socket.Dispose();
1448+
throw;
1449+
}
14511450
}
14521451

1453-
return (socket, stream);
1452+
return stream;
14541453
}
14551454
catch (Exception ex)
14561455
{
1457-
socket?.Dispose();
14581456
throw ex is OperationCanceledException oce && oce.CancellationToken == cancellationToken ?
14591457
CancellationHelper.CreateOperationCanceledException(innerException: null, cancellationToken) :
14601458
ConnectHelper.CreateWrappedException(ex, endPoint.Host, endPoint.Port, cancellationToken);
@@ -1463,9 +1461,8 @@ public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool
14631461

14641462
internal async ValueTask<HttpConnection> CreateHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
14651463
{
1466-
(Socket? socket, Stream stream, TransportContext? transportContext) = await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false);
1467-
1468-
return await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false);
1464+
(Stream stream, TransportContext? transportContext) = await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false);
1465+
return await ConstructHttp11ConnectionAsync(async, stream, transportContext, request, cancellationToken).ConfigureAwait(false);
14691466
}
14701467

14711468
private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessage request)
@@ -1528,15 +1525,10 @@ private async ValueTask<Stream> ApplyPlaintextFilterAsync(bool async, Stream str
15281525
return newStream;
15291526
}
15301527

1531-
private async ValueTask<HttpConnection> ConstructHttp11ConnectionAsync(bool async, Socket? socket, Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken)
1528+
private async ValueTask<HttpConnection> ConstructHttp11ConnectionAsync(bool async, Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken)
15321529
{
15331530
Stream newStream = await ApplyPlaintextFilterAsync(async, stream, HttpVersion.Version11, request, cancellationToken).ConfigureAwait(false);
1534-
if (newStream != stream)
1535-
{
1536-
// If a plaintext filter created a new stream, we can't trust that the socket is still applicable.
1537-
socket = null;
1538-
}
1539-
return new HttpConnection(this, socket, newStream, transportContext);
1531+
return new HttpConnection(this, newStream, transportContext);
15401532
}
15411533

15421534
private async ValueTask<Http2Connection> ConstructHttp2ConnectionAsync(Stream stream, HttpRequestMessage request, CancellationToken cancellationToken)
@@ -1589,12 +1581,12 @@ private async ValueTask<Stream> EstablishProxyTunnelAsync(bool async, HttpReques
15891581
}
15901582
}
15911583

1592-
private async ValueTask<(Socket? socket, Stream stream)> EstablishSocksTunnel(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
1584+
private async ValueTask<Stream> EstablishSocksTunnel(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
15931585
{
15941586
Debug.Assert(_originAuthority != null);
15951587
Debug.Assert(_proxyUri != null);
15961588

1597-
(Socket? socket, Stream stream) = await ConnectToTcpHostAsync(_proxyUri.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
1589+
Stream stream = await ConnectToTcpHostAsync(_proxyUri.IdnHost, _proxyUri.Port, request, async, cancellationToken).ConfigureAwait(false);
15981590

15991591
try
16001592
{
@@ -1606,7 +1598,7 @@ private async ValueTask<Stream> EstablishProxyTunnelAsync(bool async, HttpReques
16061598
throw new HttpRequestException(SR.net_http_request_aborted, e);
16071599
}
16081600

1609-
return (socket, stream);
1601+
return stream;
16101602
}
16111603

16121604
private void HandleHttp11ConnectionFailure(HttpRequestMessage request, Exception e)

0 commit comments

Comments
 (0)