Added an additional yield point prior to possibly long running initialization (#115688)

* Added an additional yield point

* Enqueued setup logic to thread pool

Also added an explanation comment

* Changed validation order to reduce discrepancies

between synchronous and asynchronous calls

* Updated comment

Co-authored-by: Miha Zupan <[email protected]>

* Removed Volatile.Read to make the code more readable

Co-authored-by: Miha Zupan <[email protected]>

* Removed the test that is no longer viable

* Moved the nested method

to adhere to the code style

---------

Co-authored-by: Miha Zupan <[email protected]>

Author: 2 people

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

@@ -20,6 +20,7 @@ public sealed class SocketsHttpHandler : HttpMessageHandler
     {
         private readonly HttpConnectionSettings _settings = new HttpConnectionSettings();
         private HttpMessageHandlerStage? _handler;
+        private Task<HttpMessageHandlerStage>? _handlerChainSetupTask;
         private Func<HttpConnectionSettings, HttpMessageHandlerStage, HttpMessageHandlerStage>? _decompressionHandlerFactory;
         private bool _disposed;
 
@@ -598,14 +599,14 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request,
 
             cancellationToken.ThrowIfCancellationRequested();
 
-            HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain();
-
             Exception? error = ValidateAndNormalizeRequest(request);
             if (error != null)
             {
                 throw error;
             }
 
+            HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain();
+
             return handler.Send(request, cancellationToken);
         }
 
@@ -620,15 +621,25 @@ protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessa
                 return Task.FromCanceled<HttpResponseMessage>(cancellationToken);
             }
 
-            HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain();
-
             Exception? error = ValidateAndNormalizeRequest(request);
             if (error != null)
             {
                 return Task.FromException<HttpResponseMessage>(error);
             }
 
-            return handler.SendAsync(request, cancellationToken);
+            return _handler is { } handler
+                ? handler.SendAsync(request, cancellationToken)
+                : CreateHandlerAndSendAsync(request, cancellationToken);
+
+            // SetupHandlerChain may block for a few seconds in some environments.
+            // E.g. during the first access of HttpClient.DefaultProxy - https://github.com/dotnet/runtime/issues/115301.
+            // The setup procedure is enqueued to thread pool to prevent the caller from blocking.
+            async Task<HttpResponseMessage> CreateHandlerAndSendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+            {
+                _handlerChainSetupTask ??= Task.Run(SetupHandlerChain);
+                HttpMessageHandlerStage handler = await _handlerChainSetupTask.ConfigureAwait(false);
+                return await handler.SendAsync(request, cancellationToken).ConfigureAwait(false);
+            }
         }
 
         private static Exception? ValidateAndNormalizeRequest(HttpRequestMessage request)

src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs

@@ -1069,83 +1069,6 @@ await RemoteExecutor.Invoke(async (useVersion, testAsync) =>
             }, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync();
         }
 
-        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
-        public async Task SendAsync_ExpectedDiagnosticSynchronousExceptionActivityLogging()
-        {
-            await RemoteExecutor.Invoke(async (useVersion, testAsync) =>
-            {
-                Exception exceptionLogged = null;
-
-                TaskCompletionSource activityStopTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
-
-                var diagnosticListenerObserver = new FakeDiagnosticListenerObserver(kvp =>
-                {
-                    if (kvp.Key.Equals("System.Net.Http.HttpRequestOut.Stop"))
-                    {
-                        Assert.NotNull(kvp.Value);
-                        GetProperty<HttpRequestMessage>(kvp.Value, "Request");
-                        TaskStatus requestStatus = GetProperty<TaskStatus>(kvp.Value, "RequestTaskStatus");
-                        Assert.Equal(TaskStatus.Faulted, requestStatus);
-                        activityStopTcs.SetResult();
-                    }
-                    else if (kvp.Key.Equals("System.Net.Http.Exception"))
-                    {
-                        Assert.NotNull(kvp.Value);
-                        exceptionLogged = GetProperty<Exception>(kvp.Value, "Exception");
-                    }
-                });
-
-                using (DiagnosticListener.AllListeners.Subscribe(diagnosticListenerObserver))
-                {
-                    diagnosticListenerObserver.Enable();
-
-                    using (HttpClientHandler handler = CreateHttpClientHandler(useVersion))
-                    using (HttpClient client = CreateHttpClient(handler, useVersion))
-                    {
-                        // Set a ftp proxy.
-                        // Forces a synchronous exception for SocketsHttpHandler.
-                        // SocketsHttpHandler only allow http & https & socks scheme for proxies.
-                        handler.Proxy = new WebProxy($"ftp://foo.bar", false);
-                        var request = new HttpRequestMessage(HttpMethod.Get, InvalidUri)
-                        {
-                            Version = Version.Parse(useVersion)
-                        };
-
-                        // We cannot use Assert.Throws<Exception>(() => { SendAsync(...); }) to verify the
-                        // synchronous exception here, because DiagnosticsHandler SendAsync() method has async
-                        // modifier, and returns Task. If the call is not awaited, the current test method will continue
-                        // run before the call is completed, thus Assert.Throws() will not capture the exception.
-                        // We need to wait for the Task to complete synchronously, to validate the exception.
-
-                        Exception exception = null;
-                        if (bool.Parse(testAsync))
-                        {
-                            Task sendTask = client.SendAsync(request);
-                            Assert.True(sendTask.IsFaulted);
-                            exception = sendTask.Exception.InnerException;
-                        }
-                        else
-                        {
-                            try
-                            {
-                                client.Send(request);
-                            }
-                            catch (Exception ex)
-                            {
-                                exception = ex;
-                            }
-                            Assert.NotNull(exception);
-                        }
-
-                        await activityStopTcs.Task;
-
-                        Assert.IsType<NotSupportedException>(exception);
-                        Assert.Same(exceptionLogged, exception);
-                    }
-                }
-            }, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync();
-        }
-
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public async Task SendAsync_ExpectedDiagnosticSourceNewAndDeprecatedEventsLogging()
         {

src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs

@@ -2595,8 +2595,11 @@ public async Task AfterDisposeSendAsync_GettersUsable_SettersThrow(bool dispose)
                 else
                 {
                     using (var c = new HttpMessageInvoker(handler, disposeHandler: false))
+                    {
                         await Assert.ThrowsAnyAsync<Exception>(() =>
-                            c.SendAsync(new HttpRequestMessage(HttpMethod.Get, new Uri("/shouldquicklyfail", UriKind.Relative)), default));
+                            c.SendAsync(new HttpRequestMessage(HttpMethod.Get, new Uri("http://www.some.example/shouldquicklyfail", UriKind.Absolute)), default));
+                    }
+
                     expectedExceptionType = typeof(InvalidOperationException);
                 }