Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpClient.SendAsync Throws Two Exceptions When QUIC Is Not Supported #49918

Closed
brianrob opened this issue Mar 19, 2021 · 6 comments · Fixed by #49973
Closed

HttpClient.SendAsync Throws Two Exceptions When QUIC Is Not Supported #49918

brianrob opened this issue Mar 19, 2021 · 6 comments · Fixed by #49973
Labels
area-System.Net tenet-performance Performance related issue
Milestone

Comments

@brianrob
Copy link
Member

brianrob commented Mar 19, 2021

On first use of HttpClient, there is a check to see if QUIC is supported. On any machine where QUIC hasn't been setup, this code throws two exceptions - a DLLNotFoundException, which is caught, and translated into a NotSupportedException. The NotSupportedException is also caught and translated into IsSupported = true. This is the stack where this happens:

module coreclr   <<coreclr!ProcessCLRException>>
module ntdll   <<ntdll!RtlRaiseException>>
module   kernelbase <<kernelbase!RaiseException>>
module coreclr <<coreclr!IL_Throw>>
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..ctor()
module coreclr <<coreclr!ProcessCLRException>>
module ntdll <<ntdll!RtlUnwind>>
module coreclr <<coreclr!ProcessCLRException>>
module ntdll <<ntdll!RtlRaiseException>>
module kernelbase <<kernelbase!RaiseException>>
module coreclr <<coreclr!ThePreStub>>
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..ctor()
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..cctor()
module coreclr <<coreclr!DelayLoad_Helper>>
system.net.quic!MsQuicImplementationProvider.get_IsSupported
system.net.http!System.Net.Http.HttpConnectionPool..ctor(System.Net.Http.HttpConnectionPoolManager,   System.Net.Http.HttpConnectionKind, System.String, Int32, System.String,   System.Uri, Int32)
system.net.http!HttpConnectionPoolManager.SendAsyncCore
system.net.http!HttpConnectionPoolManager.SendAsync
system.net.http!HttpConnectionHandler.SendAsync
system.net.http!System.Net.Http.RedirectHandler+<SendAsync>d__4.MoveNext()
module System.Private.CoreLib.il   <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!RedirectHandler.SendAsync
system.net.http!HttpMessageHandlerStage.SendAsync
system.net.http!SocketsHttpHandler.SendAsync
system.net.http!DelegatingHandler.SendAsync
system.net.http!System.Net.Http.DiagnosticsHandler+<SendAsyncCore>d__5.MoveNext()
module System.Private.CoreLib.il   <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!DiagnosticsHandler.SendAsyncCore
system.net.http!DiagnosticsHandler.SendAsync
system.net.http!HttpClientHandler.SendAsync
system.net.http!HttpMessageInvoker.SendAsync
system.net.http!System.Net.Http.HttpClient+<<SendAsync>g__Core\|83_0>d.MoveNext()
module System.Private.CoreLib.il <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!System.Net.Http.HttpClient.<SendAsync>g__Core\|83_0(System.Net.Http.HttpRequestMessage,   System.Net.Http.HttpCompletionOption,   System.Threading.CancellationTokenSource, Boolean,   System.Threading.CancellationTokenSource, System.Threading.CancellationToken)
system.net.http!HttpClient.SendAsync

Given that this is a light-up scenario, and in most cases, QUIC won't be supported, we should change this behavior to not throw, but instead, to detect the existence of the native component (e.g. msquic.dll) before attempting to load it.

@brianrob brianrob added the tenet-performance Performance related issue label Mar 19, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net untriaged New issue has not been triaged by the area owner labels Mar 19, 2021
@ghost
Copy link

ghost commented Mar 19, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

On first use of HttpClient, there is a check to see if QUIC is supported. On any machine where QUIC hasn't been setup, this code throws two exceptions - a DLLNotFoundException, which is caught, and translated into a NotSupportedException. The NotSupportedException is also caught and translated into IsSupported = true. This is the stack where this happens:

module coreclr   <<coreclr!ProcessCLRException>>
module ntdll   <<ntdll!RtlRaiseException>>
module   kernelbase <<kernelbase!RaiseException>>
module coreclr <<coreclr!IL_Throw>>
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..ctor()
module coreclr <<coreclr!ProcessCLRException>>
module ntdll <<ntdll!RtlUnwind>>
module coreclr <<coreclr!ProcessCLRException>>
module ntdll <<ntdll!RtlRaiseException>>
module kernelbase <<kernelbase!RaiseException>>
module coreclr <<coreclr!ThePreStub>>
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..ctor()
system.net.quic!System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi..cctor()
module coreclr <<coreclr!DelayLoad_Helper>>
system.net.quic!MsQuicImplementationProvider.get_IsSupported
system.net.http!System.Net.Http.HttpConnectionPool..ctor(System.Net.Http.HttpConnectionPoolManager,   System.Net.Http.HttpConnectionKind, System.String, Int32, System.String,   System.Uri, Int32)
system.net.http!HttpConnectionPoolManager.SendAsyncCore
system.net.http!HttpConnectionPoolManager.SendAsync
system.net.http!HttpConnectionHandler.SendAsync
system.net.http!System.Net.Http.RedirectHandler+<SendAsync>d__4.MoveNext()
module System.Private.CoreLib.il   <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!RedirectHandler.SendAsync
system.net.http!HttpMessageHandlerStage.SendAsync
system.net.http!SocketsHttpHandler.SendAsync
system.net.http!DelegatingHandler.SendAsync
system.net.http!System.Net.Http.DiagnosticsHandler+<SendAsyncCore>d__5.MoveNext()
module System.Private.CoreLib.il   <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!DiagnosticsHandler.SendAsyncCore
system.net.http!DiagnosticsHandler.SendAsync
system.net.http!HttpClientHandler.SendAsync
system.net.http!HttpMessageInvoker.SendAsync
system.net.http!System.Net.Http.HttpClient+<<SendAsync>g__Core\|83_0>d.MoveNext()
module System.Private.CoreLib.il <<System.Private.CoreLib.il!System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[System.__Canon].Start(!!0&)>>
system.net.http!System.Net.Http.HttpClient.<SendAsync>g__Core\|83_0(System.Net.Http.HttpRequestMessage,   System.Net.Http.HttpCompletionOption,   System.Threading.CancellationTokenSource, Boolean,   System.Threading.CancellationTokenSource, System.Threading.CancellationToken)
system.net.http!HttpClient.SendAsync

Given that this is a light-up scenario, and in most cases, QUIC won't be supported, we should change this behavior to not throw, but instead, to detect the existence of the native component (e.g. msquic.dll) before attempting to load it.

Author: brianrob
Assignees: -
Labels:

area-System.Net, tenet-performance, untriaged

Milestone: -

@stephentoub stephentoub added this to the 6.0.0 milestone Mar 20, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 22, 2021
@ManickaP
Copy link
Member

ManickaP commented Mar 22, 2021

@brianrob: Did you have System.Net.SocketsHttpHandler.Http3DraftSupport app context switch on? What is your scenario? Are you running some tests locally (which ones) or is this CI or when/where did you see this? Is this blocking something/someone? What's the priority?

I'm trying to understand this since me and @CarnaViire are taking over H3 and QUIC so that we do not make the same mistake in the future.

@stephentoub
Copy link
Member

Did you have System.Net.SocketsHttpHandler.Http3DraftSupport app context switch on?

@ManickaP, it's defaulting to on:

@brianrob
Copy link
Member Author

Right, per @stephentoub's comment, I did not have the switch on, but it defaults to true. The scenario where I ran into this is running dotnet build, which uses HttpClient for sending telemetry, but this will occur in any app that is using HttpClient.

I would describe the priority as not urgent, but something that we should fix in .NET 6, as this is extra overhead in any app using HttpClient. Throwing and catching exceptions are expensive, and given that all of this detection code is internal, and we don't want the exception to leak, we can do this without exceptions much more inexpensively.

@ManickaP
Copy link
Member

I completely missed the PR that changed that and thought it was disabled by default. Thanks for pointing that out. And thanks for the additional info and use case.

@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Mar 22, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 24, 2021
@brianrob
Copy link
Member Author

Thank you @stephentoub!

@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants