Avoid creating cleaning timer if no connections will ever be added to pool

Author: stephentoub

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs

@@ -5,7 +5,6 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Net.Http.Headers;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -62,17 +61,42 @@ public HttpConnectionPoolManager(HttpConnectionSettings settings)
 
             // Start out with the timer not running, since we have no pools.
             // When it does run, run it with a frequency based on the idle timeout.
-            if (settings._pooledConnectionIdleTimeout == Timeout.InfiniteTimeSpan)
+            if (settings._pooledConnectionLifetime != TimeSpan.Zero &&
+                settings._pooledConnectionIdleTimeout != TimeSpan.Zero)
             {
-                const int DefaultScavengeSeconds = 30;
-                _cleanPoolTimeout = TimeSpan.FromSeconds(DefaultScavengeSeconds);
-            }
-            else
-            {
-                const int ScavengesPerIdle = 4;
-                const int MinScavengeSeconds = 1;
-                TimeSpan timerPeriod = settings._pooledConnectionIdleTimeout / ScavengesPerIdle;
-                _cleanPoolTimeout = timerPeriod.TotalSeconds >= MinScavengeSeconds ? timerPeriod : TimeSpan.FromSeconds(MinScavengeSeconds);
+                if (settings._pooledConnectionIdleTimeout == Timeout.InfiniteTimeSpan)
+                {
+                    const int DefaultScavengeSeconds = 30;
+                    _cleanPoolTimeout = TimeSpan.FromSeconds(DefaultScavengeSeconds);
+                }
+                else
+                {
+                    const int ScavengesPerIdle = 4;
+                    const int MinScavengeSeconds = 1;
+                    TimeSpan timerPeriod = settings._pooledConnectionIdleTimeout / ScavengesPerIdle;
+                    _cleanPoolTimeout = timerPeriod.TotalSeconds >= MinScavengeSeconds ? timerPeriod : TimeSpan.FromSeconds(MinScavengeSeconds);
+                }
+
+                bool restoreFlow = false;
+                try
+                {
+                    // Don't capture the current ExecutionContext and its AsyncLocals onto the timer causing them to live forever
+                    if (!ExecutionContext.IsFlowSuppressed())
+                    {
+                        ExecutionContext.SuppressFlow();
+                        restoreFlow = true;
+                    }
+
+                    _cleaningTimer = new Timer(s => ((HttpConnectionPoolManager)s).RemoveStalePools(), this, Timeout.Infinite, Timeout.Infinite);
+                }
+                finally
+                {
+                    // Restore the current ExecutionContext
+                    if (restoreFlow)
+                    {
+                        ExecutionContext.RestoreFlow();
+                    }
+                }
             }
 
             // Figure out proxy stuff.
@@ -84,25 +108,6 @@ public HttpConnectionPoolManager(HttpConnectionSettings settings)
                     _proxyCredentials = _proxy.Credentials ?? settings._defaultProxyCredentials;
                 }
             }
-
-            // Don't capture the current ExecutionContext and its AsyncLocals onto the timer causing them to live forever
-            bool restoreFlow = false;
-            try
-            {
-                if (!ExecutionContext.IsFlowSuppressed())
-                {
-                    ExecutionContext.SuppressFlow();
-                    restoreFlow = true;
-                }
-
-                _cleaningTimer = new Timer(s => ((HttpConnectionPoolManager)s).RemoveStalePools(), this, Timeout.Infinite, Timeout.Infinite);
-            }
-            finally
-            {
-                // Restore the current ExecutionContext
-                if (restoreFlow)
-                    ExecutionContext.RestoreFlow();
-            }
         }
 
         public HttpConnectionSettings Settings => _settings;
@@ -204,7 +209,7 @@ public Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request, Uri p
             while (!_pools.TryGetValue(key, out pool))
             {
                 pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri, _maxConnectionsPerServer);
-                if (_pools.TryAdd(key, pool))
+                if (_pools.TryAdd(key, pool) && _cleaningTimer != null)
                 {
                     // We need to ensure the cleanup timer is running if it isn't
                     // already now that we added a new connection pool.
@@ -263,7 +268,8 @@ public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool doRe
         /// <summary>Disposes of the pools, disposing of each individual pool.</summary>
         public void Dispose()
         {
-            _cleaningTimer.Dispose();
+            _cleaningTimer?.Dispose();
+
             foreach (KeyValuePair<HttpConnectionKey, HttpConnectionPool> pool in _pools)
             {
                 pool.Value.Dispose();
@@ -278,6 +284,8 @@ public void Dispose()
         /// <summary>Removes unusable connections from each pool, and removes stale pools entirely.</summary>
         private void RemoveStalePools()
         {
+            Debug.Assert(_cleaningTimer != null);
+
             // Iterate through each pool in the set of pools.  For each, ask it to clear out
             // any unusable connections (e.g. those which have expired, those which have been closed, etc.)
             // The pool may detect that it's empty and long unused, in which case it'll dispose of itself,