Skip to content

Serialize HTTP/2 loopback writes and join reader before disposal - #135941

Merged
rzikm merged 1 commit into
dotnet:mainfrom
rzikm:rzikm/http2-helper-write-serialization
Oct 19, 2026
Merged

rzikm merged 1 commit into
dotnet:mainfrom
rzikm:rzikm/http2-helper-write-serialization

Conversation

@rzikm

@rzikm rzikm commented Oct 16, 2026

Copy link
Copy Markdown
Member

HTTP/2 stream-limit and keepalive tests can issue concurrent writes on one loopback connection, causing SslStream to throw NotSupportedException: This method may not be called when another write operation is pending. A pending ReadFrameAsync can send a transparent PING ACK while the test sends response HEADERS; the keepalive reader can also observe client EOF and send disposal GOAWAY while final response DATA is still being written.

Enable the existing connection-scoped EnsureThreadSafeIO option for the stream-limit test and keepalive fixture. Move keepalive connection disposal to TerminateLoopbackConnectionAsync, after joining the incoming-frame task, so the reader does not independently initiate shutdown on EOF. Protocol assertions are unchanged; there are no product changes or changes to the shared helper implementation.

Validation

Windows x64, using a short subst path:

  • .\build.cmd clr+libs -rc release: passed. Existing MsQuic package advisory warnings remain.
  • .\dotnet.cmd build src\libraries\System.Net.Http\tests\FunctionalTests\System.Net.Http.Functional.Tests.csproj /t:Test /p:Outerloop=true "/p:XUnitOptions=-method System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.Http2_MaxConcurrentStreams_LimitEnforced -method System.Net.Http.Functional.Tests.SocketsHttpHandler_Http2KeepAlivePing_Test.KeepAlivePingDelay_Infinite_NoKeepAlivePingIsSent -method System.Net.Http.Functional.Tests.SocketsHttpHandler_Http2KeepAlivePing_Test.KeepAliveConfigured_KeepAlivePingsAreSentEvenWith0ConnectionLifetime -method System.Net.Http.Functional.Tests.SocketsHttpHandler_Http2KeepAlivePing_Test.KeepAliveConfigured_ConnectionAtMaxConcurrentStreams_KeepAlivePingsAreStillSent -method System.Net.Http.Functional.Tests.SocketsHttpHandler_Http2KeepAlivePing_Test.KeepAliveConfigured_HandlerDisposedWithActiveRequest_KeepAlivePingsAreStillSent": passed; repeated twice with /p:TestNoBuild=true, totaling 15 passed, 0 failed, 0 skipped after removing instrumentation.
  • .\dotnet.cmd build src\libraries\System.Net.Http\tests\FunctionalTests\System.Net.Http.Functional.Tests.csproj /t:Test /p:Outerloop=true /p:TestNoBuild=true "/p:XUnitOptions=-class System.Net.Http.Functional.Tests.SocketsHttpHandler_Http2KeepAlivePing_Test": passed, 10 tests, 0 skipped.

Temporary connection-scoped tracing and a 50-ms delay of transport write completion below SslStream reproduced competing writers in four of the five methods. Across three repetitions of all five tests, the original code had 12 failures; serialization alone passed 15/15, joining alone passed 9/15, and this fix passed 15/15 under the same delay. Traces confirmed queued ACK writes reached TLS only after the preceding response write completed. All instrumentation and delay controls were removed from this patch.

No spontaneous failure occurred in the unamplified baseline. The max-concurrent-streams keepalive variant did not reproduce locally, although it passed all fixed runs. Linux/ARM, NativeAOT, WinHTTP, and the full System.Net.Http suite were not run.

Note

This pull request description and code changes were generated with GitHub Copilot.

Enable existing connection-scoped I/O serialization for stream-limit and keepalive tests. Dispose keepalive connections after joining frame processing so EOF cleanup cannot race final response writes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3326d75e-588a-4534-9778-75e5812d65c0
@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.

@rzikm

rzikm commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

/ba-g mutex tests are unrelated

@rzikm
rzikm merged commit bd4312a into dotnet:main Oct 19, 2026
85 of 87 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 20, 2026
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.

2 participants