Skip to content

Avoid early HTTP/2 teardown in WinHTTP Latin1 header test - #133955

Open
rzikm wants to merge 1 commit into
dotnet:mainfrom
rzikm:rzikm/winhttp-and-flow-control-failures
Open

rzikm wants to merge 1 commit into
dotnet:mainfrom
rzikm:rzikm/winhttp-and-flow-control-failures

Conversation

@rzikm

@rzikm rzikm commented Sep 15, 2026

Copy link
Copy Markdown
Member

Summary

Keep the HTTP/2 loopback connection alive until the Latin1 header test's buffered HttpClient.SendAsync and 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.

  • Directly invoking the unchanged archived CI test method reproduced API_QUERY_DATA_AVAILABLE / WINHTTP_CONNECTION_ERROR(12030) at call 686.
  • A controlled reconstruction using those archived assemblies compared immediate shutdown with waiting for buffered client completion: 1 native 12030 in 10,000 ungated calls; 0 in 10,000 gated calls. That controlled failure was at API_WRITE_DATA, not API_QUERY_DATA_AVAILABLE. Observer timestamps show server END_STREAM and send shutdown before the request-error callback.
  • The different native API phases and sparse failure count matter: this supports removing the test's unnecessary teardown race, but does not prove the internal native cause of the original CI failure or elimination of all WinHTTP 12030s. A separate bounded header-callback-delay experiment passed both arms and was inconclusive.

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.

Final-source validation Result
WinHTTP full functional suite, CoreCLR 1,631 passed, 41 existing skips, zero failures
WinHTTP full functional suite, .NET Framework net481 639 passed, 28 existing skips, zero failures
Actual fixed Content/0x7f method, filtered native tracing enabled 10,000/10,000 passed
Shared Sockets Latin1 theory, unmodified HEAD and final source Both 70 passed / same two HTTP/1 host-software-aborted failures

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.

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>
Copilot AI lite review requested due to automatic review settings September 15, 2026 14:57
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Test Failure] WinHTTP: System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandler_Http2_Test.SendAsync_RequestWithLatin1HeaderValue_Succeeds

3 participants