add HttpConnectionBase and refactor some connection logic, in preparation for HTTP2...
authorGeoff Kizer <geoffrek>
Wed, 25 Apr 2018 10:12:42 +0000 (03:12 -0700)
committerGeoff Kizer <geoffrek>
Tue, 17 Jul 2018 18:49:15 +0000 (11:49 -0700)
Commit migrated from https://github.com/dotnet/corefx/commit/8595970e24d5f52c0777ef2b4a5f0d88397fe7bb

src/libraries/System.Net.Http/src/System.Net.Http.csproj
src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs [new file with mode: 0644]
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

index f27b2b7..1a8a851 100644 (file)
     <Compile Include="System\Net\Http\SocketsHttpHandler\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpAuthenticatedConnectionHandler.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnection.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionBase.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionHandler.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionKind.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionPool.cs" />
index a24a0d4..b68d559 100644 (file)
@@ -3,12 +3,15 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Diagnostics.CodeAnalysis;
+using System.IO;
 
 namespace System.Net.Http
 {
     [SuppressMessage("Microsoft.Serialization", "CA2229")]
     public class HttpRequestException : Exception
     {
+        private bool _allowRetry;
+
         public HttpRequestException()
             : this(null, null)
         { }
@@ -25,5 +28,16 @@ namespace System.Net.Http
                 HResult = inner.HResult;
             }
         }
+
+        // This constructor is used internally to indicate that a request was not successfully sent due to an IOException,
+        // and the exception occurred early enough so that the request may be retried on another connection.
+        // occurred before the request
+        internal HttpRequestException(string message, IOException inner, bool allowRetry)
+            : this(message, inner)
+        {
+            _allowRetry = allowRetry;
+        }
+
+        internal bool AllowRetry => _allowRetry;
     }
 }
index e719fa2..7327e59 100644 (file)
@@ -17,7 +17,7 @@ using System.Threading.Tasks;
 
 namespace System.Net.Http
 {
-    internal partial class HttpConnection : IDisposable
+    internal partial class HttpConnection : HttpConnectionBase, IDisposable
     {
         /// <summary>Default size of the read buffer used for the connection.</summary>
         private const int InitialReadBufferSize =
@@ -198,26 +198,6 @@ namespace System.Net.Http
             return null;
         }
 
-        public bool IsNewConnection
-        {
-            get
-            {
-                // This is only valid when we are not actually processing a request.
-                Debug.Assert(_currentRequest == null);
-                return (_readAheadTask == null);
-            }
-        }
-
-        public bool CanRetry
-        {
-            get
-            {
-                // Should only be called when we have been disposed.
-                Debug.Assert(_disposed != 0);
-                return _canRetry;
-            }
-        }
-
         public DateTimeOffset CreationTime { get; } = DateTimeOffset.UtcNow;
 
         public TransportContext TransportContext => _transportContext;
@@ -683,12 +663,17 @@ namespace System.Net.Http
                     // original information as the inner exception, for diagnostic purposes.
                     throw CancellationHelper.CreateOperationCanceledException(error, cancellationToken);
                 }
-                else if (error is InvalidOperationException || error is IOException)
+                else if (error is InvalidOperationException)
                 {
-                    // If it's an InvalidOperationException or an IOException, for consistency
-                    // with other handlers we wrap the exception in an HttpRequestException.
+                    // For consistency with other handlers we wrap the exception in an HttpRequestException.
                     throw new HttpRequestException(SR.net_http_client_execution_error, error);
                 }
+                else if (error is IOException ioe)
+                {
+                    // For consistency with other handlers we wrap the exception in an HttpRequestException.
+                    // If the request is retryable, indicate that on the exception.
+                    throw new HttpRequestException(SR.net_http_client_execution_error, ioe, _canRetry);
+                }
                 else
                 {
                     // Otherwise, just allow the original exception to propagate.
@@ -707,7 +692,7 @@ namespace System.Net.Http
             return SendAsyncCore(request, cancellationToken);
         }
 
-        public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool doRequestAuth, CancellationToken cancellationToken)
+        private Task<HttpResponseMessage> SendAsyncInternal(HttpRequestMessage request, bool doRequestAuth, CancellationToken cancellationToken)
         {
             if (doRequestAuth && _pool.Settings._credentials != null)
             {
@@ -717,6 +702,19 @@ namespace System.Net.Http
             return SendWithNtProxyAuthAsync(request, cancellationToken);
         }
 
+        public sealed override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool doRequestAuth, CancellationToken cancellationToken)
+        {
+            Acquire();
+            try
+            {
+                return await SendAsyncInternal(request, doRequestAuth, cancellationToken).ConfigureAwait(false);
+            }
+            finally
+            {
+                Release();
+            }
+        }
+
         private HttpContentWriteStream CreateRequestContentStream(HttpRequestMessage request)
         {
             bool requestTransferEncodingChunked = request.HasHeaders && request.Headers.TransferEncodingChunked == true;
@@ -1472,7 +1470,7 @@ namespace System.Net.Http
             }
         }
 
-        public void Acquire()
+        private void Acquire()
         {
             Debug.Assert(_currentRequest == null);
             Debug.Assert(!_inUse);
@@ -1480,7 +1478,7 @@ namespace System.Net.Http
             _inUse = true;
         }
 
-        public void Release()
+        private void Release()
         {
             Debug.Assert(_inUse);
 
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs
new file mode 100644 (file)
index 0000000..08cb510
--- /dev/null
@@ -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.
+
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace System.Net.Http
+{
+    internal abstract class HttpConnectionBase
+    {
+        public abstract Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool doRequestAuth, CancellationToken cancellationToken);
+    }
+}
index 97c76ba..1d871cf 100644 (file)
@@ -175,11 +175,18 @@ namespace System.Net.Http
         /// <summary>Object used to synchronize access to state in the pool.</summary>
         private object SyncObj => _idleConnections;
 
-        private ValueTask<(HttpConnection, HttpResponseMessage)> GetConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        private async ValueTask<(HttpConnectionBase connection, bool isNewConnection, HttpResponseMessage failureResponse)> 
+            GetConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        {
+            (HttpConnection connection, bool isNewConnection, HttpResponseMessage failureResponse) = await GetHttp11ConnectionAsync(request, cancellationToken);
+            return ((HttpConnectionBase)connection, isNewConnection, failureResponse);
+        }
+
+        private ValueTask<(HttpConnection connection, bool isNewConnection, HttpResponseMessage failureResponse)> GetHttp11ConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
             if (cancellationToken.IsCancellationRequested)
             {
-                return new ValueTask<(HttpConnection, HttpResponseMessage)>(Task.FromCanceled<(HttpConnection, HttpResponseMessage)>(cancellationToken));
+                return new ValueTask<(HttpConnection, bool, HttpResponseMessage)>(Task.FromCanceled<(HttpConnection, bool, HttpResponseMessage)>(cancellationToken));
             }
 
             TimeSpan pooledConnectionLifetime = _poolManager.Settings._pooledConnectionLifetime;
@@ -201,7 +208,7 @@ namespace System.Net.Http
                     {
                         // We found a valid connection.  Return it.
                         if (NetEventSource.IsEnabled) conn.Trace("Found usable connection in pool.");
-                        return new ValueTask<(HttpConnection, HttpResponseMessage)>((conn, null));
+                        return new ValueTask<(HttpConnection, bool, HttpResponseMessage)>((conn, false, null));
                     }
 
                     // We got a connection, but it was already closed by the server or the
@@ -252,7 +259,7 @@ namespace System.Net.Http
                             }
                         }, waiter);
                     }
-                    return new ValueTask<(HttpConnection, HttpResponseMessage)>(waiter.Task);
+                    return new ValueTask<(HttpConnection, bool, HttpResponseMessage)>(waiter.Task);
                 }
 
                 // Note that we don't check for _disposed.  We may end up disposing the
@@ -272,28 +279,23 @@ namespace System.Net.Http
             { 
                 // Loop on connection failures and retry if possible.
 
-                (HttpConnection connection, HttpResponseMessage response) = await GetConnectionAsync(request, cancellationToken).ConfigureAwait(false);
-                if (response != null)
+                (HttpConnectionBase connection, bool isNewConnection, HttpResponseMessage failureResponse) = await GetConnectionAsync(request, cancellationToken).ConfigureAwait(false);
+                if (failureResponse != null)
                 {
                     // Proxy tunnel failure; return proxy response
-                    return response;
+                    Debug.Assert(isNewConnection);
+                    Debug.Assert(connection == null);
+                    return failureResponse;
                 }
 
-                bool isNewConnection = connection.IsNewConnection;
-
-                connection.Acquire();
                 try
                 {
                     return await connection.SendAsync(request, doRequestAuth, cancellationToken).ConfigureAwait(false);
                 }
-                catch (HttpRequestException e) when (!isNewConnection && e.InnerException is IOException && connection.CanRetry)
+                catch (HttpRequestException e) when (!isNewConnection && e.AllowRetry)
                 {
                     // Eat exception and try again.
                 }
-                finally
-                {
-                    connection.Release();
-                }
             }
         }
 
@@ -477,7 +479,7 @@ namespace System.Net.Http
         }
 
         /// <summary>Waits for and returns the created connection, decrementing the associated connection count if it fails.</summary>
-        private async ValueTask<(HttpConnection, HttpResponseMessage)> WaitForCreatedConnectionAsync(ValueTask<(HttpConnection, HttpResponseMessage)> creationTask)
+        private async ValueTask<(HttpConnection connection, bool isNewConnection, HttpResponseMessage failureResponse)> WaitForCreatedConnectionAsync(ValueTask<(HttpConnection, HttpResponseMessage)> creationTask)
         {
             try
             {
@@ -486,7 +488,7 @@ namespace System.Net.Http
                 {
                     DecrementConnectionCount();
                 }
-                return (connection, response);
+                return (connection, true, response);
             }
             catch
             {
@@ -549,7 +551,8 @@ namespace System.Net.Http
                         // Transfer the connection to the waiter.  Since we already have a count
                         // that's inflated due to the connection being disassociated, we don't
                         // need to change the count here.
-                        waiter.SetResult(connectionTask.Result);
+                        (HttpConnection connection, HttpResponseMessage failureResponse) = connectionTask.Result;
+                        waiter.SetResult((connection, true, failureResponse));
                     }
                     else
                     {
@@ -560,10 +563,12 @@ namespace System.Net.Http
                             try
                             {
                                 // Get the resulting connection.
-                                (HttpConnection result, HttpResponseMessage response) = innerConnectionTask.GetAwaiter().GetResult();
+                                (HttpConnection connection, HttpResponseMessage failureResponse) = innerConnectionTask.GetAwaiter().GetResult();
 
-                                if (response != null)
+                                if (failureResponse != null)
                                 {
+                                    Debug.Assert(connection == null);
+
                                     // Proxy tunnel connect failed, so decrement the connection count.
                                     innerWaiter._pool.DecrementConnectionCount();
                                 }
@@ -571,7 +576,7 @@ namespace System.Net.Http
                                 // Store the resulting connection into the waiter. As in the synchronous case,
                                 // since we already have a count that's inflated due to the connection being
                                 // disassociated, we don't need to change the count here.
-                                innerWaiter.SetResult(innerConnectionTask.Result);
+                                innerWaiter.SetResult((connection, true, failureResponse));
                             }
                             catch (Exception e)
                             {
@@ -876,7 +881,7 @@ namespace System.Net.Http
         /// into the waiter as a result, and if no connection is available from the pool,
         /// this waiter's logic is used to create the connection.
         /// </summary>
-        private class ConnectionWaiter : TaskCompletionSource<(HttpConnection, HttpResponseMessage)>
+        private class ConnectionWaiter : TaskCompletionSource<(HttpConnection connection, bool isNewConnection, HttpResponseMessage failureResponse)>
         {
             /// <summary>The pool with which this waiter is associated.</summary>
             internal readonly HttpConnectionPool _pool;