-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix HTTP/2 pings not working with zero connection lifetime #115735
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
Tagging subscribers to this area: @dotnet/ncl |
@copilot please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
The new test file will need to be added to the .csproj. Otherwise, it won't be included in the built tests. |
Co-authored-by: stephentoub <[email protected]>
I've added the test file to the System.Net.Http.Functional.Tests.csproj file so it will be included in the built tests (f1088d8). |
Please write the test(s) in the same style as the rest of the tests and include it or then in an existing test file rather than creating a new one. |
Co-authored-by: stephentoub <[email protected]>
I've moved the test methods into the existing SocketsHttpHandlerTest.Http2KeepAlivePing.cs file and removed the separate test file as requested. The tests follow the same style and patterns as the existing tests in that file (2fc0eb7). Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Issue
When
PooledConnectionLifetime
is set toTimeSpan.Zero
, HTTP/2 keep-alive pings don't work, even if explicitly configured. This is because the connection pool manager optimization that avoids storing connections (when lifetime is zero) skips creating the heartbeat timer needed for ping functionality.Root Cause
In
HttpConnectionPoolManager.cs
, there's this optimization condition:When this condition is true, the code skips creating the heartbeat timer, regardless of whether the user has configured HTTP/2 keep-alive pings.
Fix
I've modified the condition to also consider whether keep-alive pings are enabled:
This ensures that if pings are configured (
keepAlivePingDelay
is notInfiniteTimeSpan
), connections will be stored and the heartbeat timer will be created, even if the connection lifetime is zero.I've also added test cases to verify this fix:
PooledConnectionLifetime = TimeSpan.Zero
and ping settings are configuredFixes #113532.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
dc.services.visualstudio.com
/home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt
(dns block)pkgs.dev.azure.com
/home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.