Conversation
Wait for buffered client completion and disposal before closing the HTTP/2 loopback connection. Cancel client work before server cleanup on failure, without changing shared teardown defaults or suppressing native errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
ManickaP
approved these changes
Sep 15, 2026
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified, and the targeted change was validated.
Pull request overview
Updates the Latin1 header test to prevent premature WinHTTP HTTP/2 connection teardown.
Changes:
- Coordinates client completion with server shutdown.
- Adds cancellation and explicit HTTP/2 cleanup for WinHTTP.
- Preserves existing protocol paths and header assertions.
File summaries
| File | Description |
|---|---|
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs |
Adds coordinated WinHTTP HTTP/2 client/server lifetime handling. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keep the HTTP/2 loopback connection alive until the Latin1 header test's buffered
HttpClient.SendAsyncand client disposal finish. The previous helper starts server-side send shutdown immediately after writing the final response, potentially racing pending WinHTTP native I/O.The new path is limited to WinHTTP HTTP/2 in this one header-encoding theory. It preserves request-body consumption, GOAWAY-before-response ordering, the empty 200/END_STREAM response, and all raw Latin1/Huffman header assertions. Client completion is signaled in
finally; server failure cancels the request before asynchronous connection disposal. HTTP/1 and Sockets retain the existing server helper.There is no production change, retry, native-error suppression, timeout increase, test disabling, or change to global loopback teardown defaults.
Evidence and limits
The supplied CoreCLR build 1594333, revision
6ed2ba318dcaa936da60f7d6bc9eb30cad54fc0c, failed the Content/0x7f case with native 12030 during response buffering: same-run console.API_QUERY_DATA_AVAILABLE / WINHTTP_CONNECTION_ERROR(12030)at call 686.This is a test-lifetime correction for header encoding, not a change to how WinHTTP handles arbitrary server disconnects. Cookie/client-certificate 12030s and badssl 12152/12002 diagnostics remain separate.
Local validation
Windows 11 build 26100, x64 CoreCLR Release with locally built Debug libraries/tests; actual runtime 12.0.0, test TFM net11.0-windows. Reused the successful clr+libs baseline through a subst path.
The Sockets async failure is tracked by #132004; its sync counterpart had the same signature. Both were reproduced with the unmodified HEAD test compiled via an artifact-only source override. Neither failure enters the new WinHTTP HTTP/2 branch, and neither was suppressed.
Artifact-only failure injection into the retained method also exercised success, client assertion failure, server failure before/after the response, cancellation with an established connection, client timeout, and completion-gate timeout. All seven preserved their expected outcomes and completed both cleanup paths, including awaited server disposal, within 0.7 seconds. The final uninstrumented source was then explicitly rebuilt and the full suites and 10,000-call replay run again. Final WinHTTP build: zero warnings/errors.
No x86, NativeAOT, older-Windows, or non-Windows execution is claimed. Framework coverage compiles the shared change but does not execute the HTTP/2 gate. Existing graceful-disposal/factory timeout limitations are not globally hardened by this patch.
Resolves #130458
Note
This change and description were developed with GitHub Copilot; validation results were collected locally.