Simplify ConnectHelper.ConnectAsync to just return Stream (dotnet/corefx#35411)

* Simplify ConnectHelper.ConnectAsync to just return Stream

SocketsHttpHandler's ConnectHelper.ConnectAsync currently returns a tuple of (Socket, Stream).  We can instead just have it return the Stream and allow the call site to fish out the Socket from the Stream if it can.  This is slightly more efficient memory-wise, in that the state machine involved will be smaller, but it also allows for a potential extensibility point that's simpler (abstracting out the ConnectAsync as a callback) and potentially allows us to in the future get a Socket in more cases, e.g. any connection helper handing back a NetworkStream.

Unfortunately, NetworkStream.Socket is protected rather than public.  Until such time that there's a public property for this, there are two options here:
1. Use reflection to create a delegate we can then use to get at the property; "Socket" is part of the exposed surface area, so the only dependency here is on documented surface area.
2. Create a custom NetworkStream-derived type that exposes the protected as a public.

I started with (2), which works fine for our own internal needs currently, but shifted to (1), as it's more flexible.

* Don't use reflection

Commit migrated from https://github.com/dotnet/corefx/commit/d9cdbc3cca0f75e5d7692f9ae03978dd0ed9b2e9

diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj
index e5c45b0..9b800cf 100644 (file)
--- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj
+++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj
@@ -139,6 +139,7 @@
     <Compile Include="System\Net\Http\SocketsHttpHandler\CreditManager.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\DecompressionHandler.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\EmptyReadStream.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\ExposedSocketNetworkStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2Connection.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2Stream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpAuthenticatedConnectionHandler.cs" />

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
index 50a44b5..b2e6f6b 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
@@ -38,7 +38,7 @@ namespace System.Net.Http
             }
         }
 
-        public static async ValueTask<(Socket, Stream)> ConnectAsync(string host, int port, CancellationToken cancellationToken)
+        public static async ValueTask<Stream> ConnectAsync(string host, int port, CancellationToken cancellationToken)
         {
             // Rather than creating a new Socket and calling ConnectAsync on it, we use the static
             // Socket.ConnectAsync with a SocketAsyncEventArgs, as we can then use Socket.CancelConnectAsync
@@ -77,7 +77,7 @@ namespace System.Net.Http
                 // Configure the socket and return a stream for it.
                 Socket socket = saea.ConnectSocket;
                 socket.NoDelay = true;
-                return (socket, new NetworkStream(socket, ownsSocket: true));
+                return new ExposedSocketNetworkStream(socket, ownsSocket: true);
             }
             catch (Exception error)
             {

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ExposedSocketNetworkStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ExposedSocketNetworkStream.cs
new file mode 100644 (file)
index 0000000..80f9948
--- /dev/null
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ExposedSocketNetworkStream.cs
@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Net.Sockets
+{
+    // TODO: Delete once https://github.com/dotnet/corefx/issues/35410 is available.
+    internal sealed class ExposedSocketNetworkStream : NetworkStream
+    {
+        public ExposedSocketNetworkStream(Socket socket, bool ownsSocket) : base(socket, ownsSocket) { }
+
+        public new Socket Socket => base.Socket;
+    }
+}

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
index f41038f..f25c0cd 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
@@ -582,18 +582,17 @@ namespace System.Net.Http
 
             try
             {
-                Socket socket = null;
                 Stream stream = null;
                 switch (_kind)
                 {
                     case HttpConnectionKind.Http:
                     case HttpConnectionKind.Https:
                     case HttpConnectionKind.ProxyConnect:
-                        (socket, stream) = await ConnectHelper.ConnectAsync(_host, _port, cancellationToken).ConfigureAwait(false);
+                        stream = await ConnectHelper.ConnectAsync(_host, _port, cancellationToken).ConfigureAwait(false);
                         break;
 
                     case HttpConnectionKind.Proxy:
-                        (socket, stream) = await ConnectHelper.ConnectAsync(_proxyUri.IdnHost, _proxyUri.Port, cancellationToken).ConfigureAwait(false);
+                        stream = await ConnectHelper.ConnectAsync(_proxyUri.IdnHost, _proxyUri.Port, cancellationToken).ConfigureAwait(false);
                         break;
 
                     case HttpConnectionKind.ProxyTunnel:
@@ -609,6 +608,8 @@ namespace System.Net.Http
                         break;
                 }
 
+                Socket socket = (stream as ExposedSocketNetworkStream)?.Socket; // TODO: Use NetworkStream when https://github.com/dotnet/corefx/issues/35410 is available.
+
                 TransportContext transportContext = null;
                 if (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel)
                 {