Skip to content

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

Merged
rzikm merged 1 commit into
dotnet:mainfrom
rzikm:rzikm/winhttp-and-flow-control-failures
Sep 16, 2026
Merged

rzikm merged 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

@rzikm

rzikm commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

I will hold on backporting for a bit to first see if the test failure counts drop on main

@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 17, 2026
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 18, 2026
)

## 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](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1594333),
revision `6ed2ba318dcaa936da60f7d6bc9eb30cad54fc0c`, failed the
Content/0x7f case with native 12030 during response buffering: [same-run
console](https://helixr18s23ayyejvk1x8qcc.blob.core.windows.net/dotnet-runtime-refs-heads-main-a2037c901dc94f1d88/System.Net.Http.WinHttpHandler.Functional.Tests/1/console.55c334a6.log?helixlogtype=result).

- 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 dotnet#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 dotnet#130458

> [!NOTE]
> This change and description were developed with GitHub Copilot;
validation results were collected locally.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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