Skip to content

[release/11.0] Defer WinHTTP HTTP/2 loopback connection closure until server disposal - #134204

Open
github-actions[bot] wants to merge 1 commit into
release/11.0from
backport/pr-134181-to-release/11.0
Open

github-actions[bot] wants to merge 1 commit into
release/11.0from
backport/pr-134181-to-release/11.0

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Backport of #134181 to release/11.0

/cc @rzikm

Customer Impact

  • Customer reported
  • Found internally

[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.]

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

[How was the fix verified? How was the issue missed previously? What tests were added?]

Risk

[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]

IMPORTANT: If this backport is for a servicing release, please verify that:

  • For .NET 8 and .NET 9: The PR target branch is release/X.0-staging, not release/X.0.
  • For .NET 10+: The PR target branch is release/X.0 (no -staging suffix).

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

#134181)

<!-- -->
WinHTTP can discard buffered response data when the loopback server
sends FIN before the client consumes it, causing error 12030. This
affects more than cookie tests: the same failure signature also appeared
in `ClientCertificateTest.UseClientCertOnHttp2_OSSupportsIt_Success`.
Synchronizing individual tests leaves other paths exposed.

This change moves lifetime management into the shared HTTP/2 loopback
infrastructure. In WinHTTP tests, connection disposal sends GOAWAY but
retains the transport until the enclosing server is disposed, normally
after both client and server callbacks complete. Retired connections do
not prevent subsequent connections from being accepted. Automatic GOAWAY
respects earlier last-stream boundaries, and cleanup tolerates the
server closing a connection during asynchronous disposal.

The behavior is enabled by default only for WinHTTP tests. Explicit
shutdown methods remain immediate, and tests requiring the original
disposal behavior can set `DeferConnectionClose = false`. The HTTP/2
cookie tests quarantined under #126867 are re-enabled; no
cookie-specific synchronization or product code changes are needed.

### Validation

- WinHTTP functional suites: 1,749 tests on .NET 11 and 667 on .NET
Framework, with zero failures and 41/28 existing skips respectively.
- WinHTTP unit tests: 173 passed.
- SocketsHttpHandler HTTP/2 protocol and cookie tests: 212 passed.
- Ten repetitions of the affected cookie, certificate, and lifetime
tests: 800 passes, no failures or skips.
- Regression tests verify that responses remain readable after
connection disposal and that physical closure during deferred disposal
completes safely. Both regressions were checked against the
corresponding unfixed behavior. These verify the lifetime contract, not
a deterministic reproduction of native error 12030.

Fixes #133779
Fixes #130458
Fixes #126867

... and countless others that boil down to "Error 12030 calling
WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, 'The connection with the server
was terminated abnormally'."


> [!NOTE]
> This pull request description and changes were generated with GitHub
Copilot.

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

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.

1 participant