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

Flow analysis regression causing failure in runtime repo #6833

Closed
buyaa-n opened this issue Aug 4, 2023 · 1 comment · Fixed by #6834
Closed

Flow analysis regression causing failure in runtime repo #6833

buyaa-n opened this issue Aug 4, 2023 · 1 comment · Fixed by #6834

Comments

@buyaa-n
Copy link
Contributor

buyaa-n commented Aug 4, 2023

A bug fix made with Perform exceptions path analysis for all flow analyses introduced regression in flow analyses and causing failures in runtime repo.

This call site is reachable on: 'freebsd'. 'HttpConnectionPool.TrySendUsingHttp3Async(HttpRequestMessage, CancellationToken)' is only supported on: 'linux', 'macOS/OSX', 'windows'.

The HttpConnectionPool.TrySendUsingHttp3Async(HttpRequestMessage, CancellationToken) was guarded with IsHttp3Supported(), but that not working anymore. I was not able to repro with a simpler code, so pasted the runtime code below, removed parts that not affecting the code analysis (like code in catch blocks etc, the full code could be found here). The warning disappears if remove the finally or 2 of the 3 catch blocks.

    public async ValueTask<HttpResponseMessage> SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken)
    {
        while (true)
        {
            HttpConnectionWaiter<Http2Connection?>? http2ConnectionWaiter = null;
            try
            {
                HttpResponseMessage? response = null;

                if (IsHttp3Supported() && // guard to enable trimming HTTP/3 support
                    _http3Enabled &&
                    (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure)) &&
                    !request.IsExtendedConnectRequest)
                {
                    if (_sslOptionsHttp3 == null)
                    {
                        // deferred creation. We use atomic exchange to be sure all threads point to single object to mimic ctor behavior.
                        SslClientAuthenticationOptions sslOptionsHttp3 = ConstructSslOptions(_poolManager, _sslOptionsHttp11!.TargetHost!);
                        sslOptionsHttp3.ApplicationProtocols = s_http3ApplicationProtocols;
                        Interlocked.CompareExchange(ref _sslOptionsHttp3, sslOptionsHttp3, null);
                    }

                    response = await TrySendUsingHttp3Async(request, cancellationToken).ConfigureAwait(false);
                }
                return response!;
            }
            catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnConnectionFailure)
            {    }
            catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnLowerHttpVersion)
            {     }
            catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnStreamLimitReached)
            {    }
            finally
            {
                CancelIfNecessary(http2ConnectionWaiter, cancellationToken.IsCancellationRequested);
            }
        }
    }

Blocking dotnet/runtime#89630.
@mavasani please take a look.

@mavasani
Copy link
Contributor

mavasani commented Aug 4, 2023

Thanks @buyaa-n looking.

mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Aug 4, 2023
mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Aug 4, 2023
mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants