feat(iot-service, shared): Add support for configuring http connection lease timeouts

common/src/service/HttpClientHelper.cs

@@ -40,53 +40,31 @@ internal sealed class HttpClientHelper : IHttpClientHelper
         // IDisposables
 
         private readonly HttpClient _httpClientWithDefaultTimeout;
-
         private readonly HttpClient _httpClientWithNoTimeout;
-        private readonly HttpClientHandler _httpClientHandler;
 
         public HttpClientHelper(
             Uri baseAddress,
             IAuthorizationHeaderProvider authenticationHeaderProvider,
             IDictionary<HttpStatusCode, Func<HttpResponseMessage, Task<Exception>>> defaultErrorMapping,
             TimeSpan timeout,
-            IWebProxy customHttpProxy)
+            IWebProxy customHttpProxy,
+            int connectionLeaseTimeoutMilliseconds)
         {
             _baseAddress = baseAddress;
             _authenticationHeaderProvider = authenticationHeaderProvider;
             _defaultErrorMapping = new ReadOnlyDictionary<HttpStatusCode, Func<HttpResponseMessage, Task<Exception>>>(defaultErrorMapping);
             _defaultOperationTimeout = timeout;
 
-            // HttpClientHandler is IDisposable, so save onto it for disposing.
-            _httpClientHandler = new HttpClientHandler
-            {
-#if !NET451
-                SslProtocols = TlsVersions.Instance.Preferred,
-                CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck,
-#endif
-            };
-
-            if (customHttpProxy != DefaultWebProxySettings.Instance)
-            {
-                _httpClientHandler.UseProxy = customHttpProxy != null;
-                _httpClientHandler.Proxy = customHttpProxy;
-            }
-
             // We need two types of HttpClients, one with our default operation timeout, and one without. The one without will rely on
             // a cancellation token.
 
-            _httpClientWithDefaultTimeout = new HttpClient(_httpClientHandler, false)
-            {
-                BaseAddress = _baseAddress,
-                Timeout = _defaultOperationTimeout,
-            };
+            _httpClientWithDefaultTimeout = CreateDefaultClient(customHttpProxy, _baseAddress, connectionLeaseTimeoutMilliseconds);
+            _httpClientWithDefaultTimeout.Timeout = _defaultOperationTimeout;
             _httpClientWithDefaultTimeout.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(CommonConstants.MediaTypeForDeviceManagementApis));
             _httpClientWithDefaultTimeout.DefaultRequestHeaders.ExpectContinue = false;
 
-            _httpClientWithNoTimeout = new HttpClient(_httpClientHandler, false)
-            {
-                BaseAddress = _baseAddress,
-                Timeout = Timeout.InfiniteTimeSpan,
-            };
+            _httpClientWithNoTimeout = CreateDefaultClient(customHttpProxy, _baseAddress, connectionLeaseTimeoutMilliseconds);
+            _httpClientWithNoTimeout.Timeout = Timeout.InfiniteTimeSpan;
             _httpClientWithNoTimeout.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(CommonConstants.MediaTypeForDeviceManagementApis));
             _httpClientWithNoTimeout.DefaultRequestHeaders.ExpectContinue = false;
 
@@ -788,9 +766,6 @@ public void Dispose()
         {
             _httpClientWithDefaultTimeout.Dispose();
             _httpClientWithNoTimeout.Dispose();
-
-            // Since HttpClientHandler was passed to the 2 HttpClients above, but told them not to dispose it, we want to dispose this after
-            _httpClientHandler.Dispose();
         }
 
         private async Task ExecuteAsync(
@@ -919,5 +894,46 @@ await ExceptionHandlingHelper.GetExceptionMessageAsync(response).ConfigureAwait(
             Task<Exception> exception = mapToExceptionFunc(response);
             return await exception.ConfigureAwait(false);
         }
+
+        internal static HttpClient CreateDefaultClient(IWebProxy webProxy, Uri baseUri, int connectionLeaseTimeoutMilliseconds)
+        {
+            // This http client will dispose of this httpMessageHandler when the http client is disposed, so no need to save a local copy
+#pragma warning disable CA2000 // Dispose objects before losing scope (httpClient and messageHandler are disposed outside of this scope)
+            return new HttpClient(CreateDefaultHttpMessageHandler(webProxy, baseUri, connectionLeaseTimeoutMilliseconds))
+#pragma warning restore CA2000 // Dispose objects before losing scope
+            {
+                // Timeouts are handled by the pipeline
+                Timeout = Timeout.InfiniteTimeSpan,
+                BaseAddress = baseUri
+            };
+        }
+
+        internal static HttpMessageHandler CreateDefaultHttpMessageHandler(IWebProxy webProxy, Uri baseUri, int connectionLeaseTimeoutMilliseconds)
+        {
+#pragma warning disable CA2000 // Dispose objects before losing scope (object is returned by this method, so the caller is responsible for disposing it)
+#if NETCOREAPP && !NETCOREAPP2_0 && !NETCOREAPP1_0 && !NETCOREAPP1_1
+            // SocketsHttpHandler is only available in netcoreapp2.1 and onwards
+            SocketsHttpHandler httpMessageHandler = new SocketsHttpHandler();
+            httpMessageHandler.SslOptions.EnabledSslProtocols = TlsVersions.Instance.Preferred;
+#else
+            HttpClientHandler httpMessageHandler = new HttpClientHandler();
+#if !NET451
+            httpMessageHandler.SslProtocols = TlsVersions.Instance.Preferred;
+            httpMessageHandler.CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck;
+#endif
+#endif
+#pragma warning restore CA2000 // Dispose objects before losing scope
+
+
+            if (webProxy != DefaultWebProxySettings.Instance)
+            {
+                httpMessageHandler.UseProxy = webProxy != null;
+                httpMessageHandler.Proxy = webProxy;
+            }
+
+            ServicePointHelpers.SetLimits(httpMessageHandler, baseUri, connectionLeaseTimeoutMilliseconds);
+
+            return httpMessageHandler;
+        }
     }
 }

iothub/service/src/AmqpServiceClient.cs

@@ -54,7 +54,8 @@ public AmqpServiceClient(IotHubConnectionString iotHubConnectionString, bool use
                 iotHubConnectionString,
                 ExceptionHandlingHelper.GetDefaultErrorMapping(),
                 s_defaultOperationTimeout,
-                transportSettings.HttpProxy);
+                transportSettings.HttpProxy,
+                transportSettings.ConnectionLeaseTimeoutMilliseconds);
 
             // Set the trace provider for the AMQP library.
             AmqpTrace.Provider = new AmqpTransportLog();

iothub/service/src/DigitalTwin/DigitalTwinClient.cs

@@ -34,12 +34,62 @@ public static DigitalTwinClient CreateFromConnectionString(string connectionStri
             return new DigitalTwinClient(iotHubConnectionString.HttpsEndpoint, sharedAccessKeyCredential, handlers);
         }
 
+
         private DigitalTwinClient(Uri uri, IotServiceClientCredentials credentials, params DelegatingHandler[] handlers)
         {
-            _client = new IotHubGatewayServiceAPIs(uri, credentials, handlers);
+            var httpMessageHandler = HttpClientHelper.CreateDefaultHttpMessageHandler(null, uri, ServicePointHelpers.DefaultConnectionLeaseTimeout);
+#pragma warning disable CA2000 // Dispose objects before losing scope (httpMessageHandlerWithDelegatingHandlers is disposed when the http client owning it is disposed)
+            HttpMessageHandler httpMessageHandlerWithDelegatingHandlers = CreateHttpHandlerPipeline(httpMessageHandler, handlers);
+#pragma warning restore CA2000 // Dispose objects before losing scope
+
+#pragma warning disable CA2000 // Dispose objects before losing scope (httpClient is disposed when the protocol layer client owning it is disposed)
+            var httpClient = new HttpClient(httpMessageHandlerWithDelegatingHandlers, true)
+            {
+                BaseAddress = uri
+            };
+#pragma warning restore CA2000 // Dispose objects before losing scope
+
+#pragma warning restore CA2000 // Dispose objects before losing scope
+
+            // When this client is disposed, all the http message handlers and delegating handlers will be disposed automatically
+            _client = new IotHubGatewayServiceAPIs(credentials, httpClient, true);
+            _client.BaseUri = uri;
             _protocolLayer = new DigitalTwin(_client);
         }
 
+        // Creates a single HttpMessageHandler to construct a HttpClient with from a base httpMessageHandler and some number of custom delegating handlers
+        // This is almost a copy of the Microsoft.Rest.ClientRuntime library's implementation, but with the return and parameter type HttpClientHandler replaced
+        // with the more abstract HttpMessageHandler in order for us to set the base handler as either a SocketsHttpHandler for .net core or an HttpClientHandler otherwise
+        // https://github.com/Azure/azure-sdk-for-net/blob/99f4da88ab0aa01c79aa291c6c101ab94c4ac940/sdk/mgmtcommon/ClientRuntime/ClientRuntime/ServiceClient.cs#L376
+        private static HttpMessageHandler CreateHttpHandlerPipeline(HttpMessageHandler httpMessageHandler, params DelegatingHandler[] handlers)
+        {
+            // The RetryAfterDelegatingHandler should be the absoulte outermost handler
+            // because it's extremely lightweight and non-interfering
+            HttpMessageHandler currentHandler =
+#pragma warning disable CA2000 // Dispose objects before losing scope (delegating handler is disposed when the http client that uses it is disposed)
+                new RetryDelegatingHandler(new RetryAfterDelegatingHandler { InnerHandler = httpMessageHandler });
+#pragma warning restore CA2000 // Dispose objects before losing scope
+
+            if (handlers != null)
+            {
+                for (int i = handlers.Length - 1; i >= 0; --i)
+                {
+                    DelegatingHandler handler = handlers[i];
+                    // Non-delegating handlers are ignored since we always
+                    // have RetryDelegatingHandler as the outer-most handler
+                    while (handler.InnerHandler is DelegatingHandler)
+                    {
+                        handler = handler.InnerHandler as DelegatingHandler;
+                    }
+
+                    handler.InnerHandler = currentHandler;
+                    currentHandler = handlers[i];
+                }
+            }
+
+            return currentHandler;
+        }
+
         /// <summary>
         /// Gets a strongly-typed digital twin.
         /// </summary>

iothub/service/src/HttpRegistryManager.cs

@@ -62,7 +62,8 @@ internal HttpRegistryManager(IotHubConnectionString connectionString, HttpTransp
                 connectionString,
                 ExceptionHandlingHelper.GetDefaultErrorMapping(),
                 s_defaultOperationTimeout,
-                transportSettings.Proxy);
+                transportSettings.Proxy,
+                transportSettings.ConnectionLeaseTimeoutMilliseconds);
         }
 
         // internal test helper

iothub/service/src/HttpTransportSettings.cs

@@ -27,5 +27,19 @@ public HttpTransportSettings()
         /// especially when MQTT and AMQP ports are disallowed to the internet.
         /// </remarks>
         public IWebProxy Proxy { get; set; }
+
+        /// <summary>
+        /// How long, in milliseconds, a given cached TCP connection created by this client's HTTP layer will live before being closed. 
+        /// If this value is set to any negative value, the connection lease will be infinite. If this value is set to 0, then the TCP connection will close after
+        /// each HTTP request and a new TCP connection will be opened upon the next request.
+        /// </summary>
+        /// <remarks>
+        /// By closing cached TCP connections and opening a new one upon the next request, the underlying HTTP client has a chance to do a DNS lookup 
+        /// to validate that it will send the requests to the correct IP address. While it is atypical for a given IoT Hub to change its IP address, it does
+        /// happen when a given IoT Hub fails over into a different region. Because of that, users who expect to failover their IoT Hub at any point
+        /// are advised to set this value to a value of 0 or greater. Larger values will make better use of caching to save network resources over time,
+        /// but smaller values will make the client respond more quickly to failed over IoT Hubs.
+        /// </remarks>
+        public int ConnectionLeaseTimeoutMilliseconds { get; set; } = ServicePointHelpers.DefaultConnectionLeaseTimeout;
     }
 }

iothub/service/src/JobClient/HttpJobClient.cs

@@ -35,7 +35,8 @@ internal HttpJobClient(IotHubConnectionString connectionString, HttpTransportSet
                 connectionString,
                 ExceptionHandlingHelper.GetDefaultErrorMapping(),
                 s_defaultOperationTimeout,
-                transportSettings.Proxy);
+                transportSettings.Proxy,
+                transportSettings.ConnectionLeaseTimeoutMilliseconds);
         }
 
         // internal test helper

iothub/service/src/Microsoft.Azure.Devices.csproj

@@ -1,6 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFrameworks>netstandard2.1;netstandard2.0;net472;net451</TargetFrameworks>
+	<!--netcoreapp2.1 target framework here is solely so that this library can use the SocketsHttpHandler class to handle connection lease timeout issues.-->
+	<!--See #1874 for additional details-->
+    <TargetFrameworks>netstandard2.1;netstandard2.0;net472;net451;netcoreapp2.1</TargetFrameworks>
     <TargetFrameworks Condition="'$(OS)' != 'Windows_NT'">netstandard2.1;netstandard2.0</TargetFrameworks>
     <LangVersion>8.0</LangVersion>
     <!-- TODO #176: Enable warnings as errors. -->

iothub/service/src/ServiceClientTransportSettings.cs

@@ -29,5 +29,19 @@ public ServiceClientTransportSettings()
         /// The proxy settings to be used on the HTTP client.
         /// </summary>
         public IWebProxy HttpProxy { get; set; }
+
+        /// <summary>
+        /// How long, in milliseconds, a given cached TCP connection created by this client's HTTP layer will live before being closed. 
+        /// If this value is set to any negative value, the connection lease will be infinite. If this value is set to 0, then the TCP connection will close after
+        /// each HTTP request and a new TCP connection will be opened upon the next request.
+        /// </summary>
+        /// <remarks>
+        /// By closing cached TCP connections and opening a new one upon the next request, the underlying HTTP client has a chance to do a DNS lookup 
+        /// to validate that it will send the requests to the correct IP address. While it is atypical for a given IoT Hub to change its IP address, it does
+        /// happen when a given IoT Hub fails over into a different region. Because of that, users who expect to failover their IoT Hub at any point
+        /// are advised to set this value to a value of 0 or greater. Larger values will make better use of caching to save network resources over time,
+        /// but smaller values will make the client respond more quickly to failed over IoT Hubs.
+        /// </remarks>
+        public int ConnectionLeaseTimeoutMilliseconds { get; set; } = ServicePointHelpers.DefaultConnectionLeaseTimeout;
     }
 }

iothub/service/src/ServicePointHelpers.cs

@@ -0,0 +1,48 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Net;
+using System.Net.Http;
+
+namespace Microsoft.Azure.Devices
+{
+    // This type manages changing HttpClient defaults to more appropriate values
+    // There are two limits we target:
+    // - Per Server Connection Limit
+    // - Keep Alive Connection Timeout
+    // On .NET Core 2.1+ the HttpClient defaults to using the HttpSocketHandler so we adjust both limits on the client handler
+    //
+    // On .NET Standard & NET 4.51+ the HttpClient defaults to using the HttpClientHandler
+    //   and there is no easy way to set Keep Alive Connection Timeout but it's mitigated by setting the service point's connection lease timeout
+    internal static class ServicePointHelpers
+    {
+        // These default values are consistent with Azure.Core default values:
+        // https://github.com/Azure/azure-sdk-for-net/blob/7e3cf643977591e9041f4c628fd4d28237398e0b/sdk/core/Azure.Core/src/Pipeline/ServicePointHelpers.cs#L28
+        internal const int DefaultMaxConnectionsPerServer = 50;
+
+        internal const int DefaultConnectionLeaseTimeout = 300 * 1000; // 5 minutes
+
+        // messageHandler passed in is an HttpClientHandler for .NET Framework and .NET standard, and a SocketsHttpHandler for .NET core
+        public static void SetLimits(HttpMessageHandler messageHandler, Uri baseUri, int connectionLeaseTimeoutMilliseconds)
+        {
+            switch (messageHandler)
+            {
+                case HttpClientHandler httpClientHandler:
+#if !NET451
+                    httpClientHandler.MaxConnectionsPerServer = DefaultMaxConnectionsPerServer;
+#endif
+                    var servicePoint = ServicePointManager.FindServicePoint(baseUri);
+                    servicePoint.ConnectionLeaseTimeout = connectionLeaseTimeoutMilliseconds;
+                    break;
+#if NETCOREAPP && !NETCOREAPP2_0 && !NETCOREAPP1_0 && !NETCOREAPP1_1
+                // SocketsHttpHandler is only available in netcore2.1 and onwards
+                case SocketsHttpHandler socketsHttpHandler:
+                    socketsHttpHandler.MaxConnectionsPerServer = DefaultMaxConnectionsPerServer;
+                    socketsHttpHandler.PooledConnectionLifetime = TimeSpan.FromMilliseconds(connectionLeaseTimeoutMilliseconds);
+                    break;
+#endif
+            }
+        }
+    }
+}

shared/src/Microsoft.Azure.Devices.Shared.csproj

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">netstandard2.1;netstandard2.0;net472;net451</TargetFrameworks>
+    <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">netstandard2.1;netstandard2.0;net472;net451;netcoreapp2.1</TargetFrameworks>
     <TargetFrameworks Condition="'$(OS)' != 'Windows_NT'">netstandard2.1;netstandard2.0</TargetFrameworks>
     <LangVersion>8.0</LangVersion>
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>