Fix HttpClient object leak described in #5984 (no socket exhaustion)

Author: bgavrilMS

src/client/Microsoft.Identity.Client/PlatformsCommon/Shared/SimpleHttpClientFactory.cs

@@ -6,6 +6,7 @@
 using System.Net.Http;
 using System.Net.Security;
 using System.Security.Cryptography.X509Certificates;
+using System.Threading;
 using Microsoft.Identity.Client.Http;
 using Microsoft.Identity.Client.ManagedIdentity;
 
@@ -24,8 +25,12 @@ internal class SimpleHttpClientFactory : IMsalMtlsHttpClientFactory, IMsalSFHttp
         private static readonly ConcurrentDictionary<string, HttpClient> s_httpClientPool = new ConcurrentDictionary<string, HttpClient>();
         private static readonly object s_cacheLock = new object();
 
+        // referenced in unit tests
+        internal static int HttpClientCreationCount;
+
         private static HttpClient CreateHttpClient()
         {
+            Interlocked.Increment(ref HttpClientCreationCount);
             CheckAndManageCache();
 
             var httpClient = new HttpClient(new HttpClientHandler()
@@ -63,7 +68,7 @@ private static HttpClient CreateMtlsHttpClient(X509Certificate2 bindingCertifica
 
         public HttpClient GetHttpClient()
         {
-            return s_httpClientPool.GetOrAdd("non_mtls", CreateHttpClient());
+            return s_httpClientPool.GetOrAdd("non_mtls", _ => CreateHttpClient());
         }
 
         public HttpClient GetHttpClient(X509Certificate2 x509Certificate2)
@@ -74,7 +79,7 @@ public HttpClient GetHttpClient(X509Certificate2 x509Certificate2)
             }
 
             string key = x509Certificate2.Thumbprint;
-            return s_httpClientPool.GetOrAdd(key, CreateMtlsHttpClient(x509Certificate2));
+            return s_httpClientPool.GetOrAdd(key, _ => CreateMtlsHttpClient(x509Certificate2));
         }
 
         private static void CheckAndManageCache()
@@ -88,6 +93,13 @@ private static void CheckAndManageCache()
             }
         }
 
+        // referenced in unit tests
+        internal static void ResetForTest()
+        {
+            s_httpClientPool.Clear();
+            HttpClientCreationCount = 0;
+        }
+
         // This method is used for Service Fabric scenarios where a custom server certificate validation callback is required.
         // It allows the caller to provide a custom HttpClientHandler with the callback.
         // The server cert rotates so we need a new HttpClient for each call.

tests/Microsoft.Identity.Test.Unit/CoreTests/HttpTests/HttpClientFactoryTests.cs

@@ -77,5 +77,27 @@ public void TestHttpClientWithMtlsCertificateAndCustomHandler()
             Assert.AreNotSame(mtlsClient, handlerClient); // Should be different instances
         }
 
+        [TestMethod]
+        public void TestGetHttpClient_DoesNotLeakHttpClients()
+        {
+            // Arrange - reset static state so we start from a clean pool
+            SimpleHttpClientFactory.ResetForTest();
+            var factory = new SimpleHttpClientFactory();
+
+            // Act - call GetHttpClient multiple times with the same (default) key
+            factory.GetHttpClient();
+            factory.GetHttpClient();
+            factory.GetHttpClient();
+
+            int created = SimpleHttpClientFactory.HttpClientCreationCount;
+
+            // Assert - CreateHttpClient should be called exactly once.
+            // Before the fix, GetOrAdd(key, CreateHttpClient()) eagerly evaluates
+            // CreateHttpClient() on every call, leaking HttpClientHandler sockets.
+            Assert.AreEqual(1, created,
+                $"CreateHttpClient was called {created} times for 3 lookups. " +
+                "Use GetOrAdd(key, factory_delegate) to avoid creating throwaway HttpClient instances.");
+        }
+
     }
 }