feedback

Author: ManickaP

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

@@ -1982,7 +1982,8 @@ public bool CleanCacheAndDisposeIfUnused()
             // Dispose them asynchronously to not to block the caller on closing the SslStream or NetworkStream.
             if (toDispose is not null)
             {
-                Task.Run(() => toDispose.ForEach(c => c.Dispose()));
+                Task.Factory.StartNew(static s => ((List<HttpConnectionBase>)s!).ForEach(c => c.Dispose()), toDispose,
+                    CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
             }
 
             // Pool is active.  Should not be removed.

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

@@ -51,12 +51,6 @@ internal sealed class HttpConnectionPoolManager : IDisposable
         /// <see cref="ConcurrentDictionary{TKey,TValue}.IsEmpty"/> call.
         /// </summary>
         private bool _timerIsRunning;
-
-        /// <summary>
-        /// Prevents parallel execution of RemoveStalePools in case the timer triggers faster than the method itself finishes.
-        /// </summary>
-        private int _removeStalePoolsIsRunning;
-
         /// <summary>Object used to synchronize access to state in the pool.</summary>
         private object SyncObj => _pools;
 
@@ -468,7 +462,7 @@ private void SetCleaningTimer(TimeSpan timeout)
         {
             try
             {
-                _cleaningTimer!.Change(timeout, timeout);
+                _cleaningTimer!.Change(timeout, Timeout.InfiniteTimeSpan);
                 _timerIsRunning = timeout != Timeout.InfiniteTimeSpan;
             }
             catch (ObjectDisposedException)
@@ -485,40 +479,23 @@ private void RemoveStalePools()
         {
             Debug.Assert(_cleaningTimer != null);
 
-            // Check whether the method is not already running and prevent parallel execution.
-            if (Interlocked.CompareExchange(ref _removeStalePoolsIsRunning, 1, 0) != 0)
-            {
-                return;
-            }
-
-            try
+            // 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,
+            // such that any connections returned to the pool to be cached will be disposed of.  In such
+            // a case, we also remove the pool from the set of pools to avoid a leak.
+            foreach (KeyValuePair<HttpConnectionKey, HttpConnectionPool> entry in _pools)
             {
-                // 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,
-                // such that any connections returned to the pool to be cached will be disposed of.  In such
-                // a case, we also remove the pool from the set of pools to avoid a leak.
-                foreach (KeyValuePair<HttpConnectionKey, HttpConnectionPool> entry in _pools)
+                if (entry.Value.CleanCacheAndDisposeIfUnused())
                 {
-                    if (entry.Value.CleanCacheAndDisposeIfUnused())
-                    {
-                        _pools.TryRemove(entry.Key, out HttpConnectionPool _);
-                    }
-                }
-
-                // Stop running the timer if we don't have any pools to clean up.
-                lock (SyncObj)
-                {
-                    if (_pools.IsEmpty)
-                    {
-                        SetCleaningTimer(Timeout.InfiniteTimeSpan);
-                    }
+                    _pools.TryRemove(entry.Key, out HttpConnectionPool _);
                 }
             }
-            finally
+
+            // Restart the timer if we have any pools to clean up.
+            lock (SyncObj)
             {
-                // Make sure the guard value gets always reset back to 0 and that it's visible to other threads.
-                Volatile.Write(ref _removeStalePoolsIsRunning, 0);
+                SetCleaningTimer(!_pools.IsEmpty ? _cleanPoolTimeout : Timeout.InfiniteTimeSpan);
             }
 
             // NOTE: There is a possible race condition with regards to a pool getting cleaned up at the same