[ci-fix] Needs review: dual-stack accept-reset socket test fails on Windows/Linux (refs #133778) - #133925
Merged
wfurt merged 1 commit intoSep 16, 2026
Conversation
The AcceptDualStackResetTests test only wrapped ReceiveAsync in the SocketException catch, but on Windows and Linux the peer's immediate reset is surfaced by AcceptAsync itself (An existing connection was forcibly closed by the remote host). Move the accept inside the try so the reset from either accept or receive is tolerated and the healthy peer is accepted on a subsequent iteration. Refs #133778 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 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 |
wfurt
approved these changes
Sep 16, 2026
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 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
There was a problem hiding this comment.
🟡 Changes recommended
Filter handling to reset-specific errors so unrelated accept failures are not swallowed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates a dual-stack socket regression test to retry peer-reset errors surfaced during Accept.
Changes:
- Moves accept operations into the existing exception handler.
- Preserves the healthy-peer assertion and clarifies retry behavior.
File summaries
| File | Summary |
|---|---|
src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs |
Adjusts reset handling in the dual-stack accept test. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Member
|
@wfurt Should this be merged? This test is failing widely |
wfurt
deleted the
ci-fix/133778-dualstack-accept-reset-a1b2-492c233de43be1d4
branch
September 16, 2026 18:23
Member
|
/backport to release/11.0 |
Contributor
Author
|
Started backporting to |
Contributor
Author
|
@rzikm backporting to git am output$ git cherry-pick bb31474ef5f70cb926e3e7e10b99378cbb2811ea
Auto-merging src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
CONFLICT (content): Merge conflict in src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
error: could not apply bb31474ef5f... [ci-fix] Needs review: dual-stack accept-reset socket test fails on Windows/Linux (refs #133778) (#133925)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: [ci-fix] Guard AcceptAsync against peer RST in dual-stack reset test
Using index info to reconstruct a base tree...
M src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
CONFLICT (content): Merge conflict in src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 [ci-fix] Guard AcceptAsync against peer RST in dual-stack reset test
Error: The process '/usr/bin/git' failed with exit code 128 |
wfurt
pushed a commit
that referenced
this pull request
Sep 17, 2026
…indows/Linux (refs #133778) (#133925) Workflow artifact: ci-fix Artifact kind: help Linked KBE: #133778 > [!NOTE] > This is an AI/Copilot-generated **best-effort** fix attempt that I could not fully validate. It is a starting point for a maintainer, not a finished change. Please review the analysis below before merging. ## Root cause (best analysis) `System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(useAsync: True)` fails on `windows-x64`/`linux-x64` (`TestReadyToRun_Libraries` and the runtime pipeline): ``` System.Net.Sockets.SocketException : An existing connection was forcibly closed by the remote host. at System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(Boolean useAsync) in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs:line 527 ``` Line 527 is the `AcceptAsync`/`Accept` call, **not** the receive. The test (added by #131869) closes an IPv4 peer with `SO_LINGER=0` so the kernel sends an immediate RST, then expects the listener to still deliver the healthy IPv6 peer's byte. The `try { ... } catch (SocketException)` block only wraps the `ReceiveAsync`; the `accept` that precedes it sits **outside** the `try`. On Windows and Linux the reset is surfaced by `accept` itself (`WSAECONNRESET` / "connection forcibly closed"), which escapes the catch and fails the test, even though the very next accept iteration would return the healthy peer. macOS discards the reset at accept (which #131869 hardened `EndPoint.Create` for), so it doesn't hit this path — which is why the test passed there but not on Windows/Linux. The test comment already anticipates this ("Some platforms surface the reset connection from accept()"), but the code doesn't guard the accept accordingly. ## Attempted fix Move the `accept` inside the existing `try` block so a `SocketException` from either the accept or the following receive is tolerated, and the loop retries to accept the healthy peer on the next iteration (the loop already runs up to 2 accepts). No production code changes; the listener-stays-healthy assertion is preserved. This is **not** a test-disable — the test still runs and still asserts the healthy peer's message is received. ## What is unverified / where I need help - I could not build and run `System.Net.Sockets.Tests` in this environment, so I have not confirmed the test now passes on windows-x64/linux-x64 while still catching a genuine listener regression. - Please confirm the intended contract: on Windows/Linux, should the RST be observable at `accept` (making tolerating it here correct), or should the runtime itself suppress the reset connection from the accept loop the way macOS does? If the latter, the real fix belongs in the socket accept path rather than the test. ## Validation - Command: `not run because the sockets functional test suite could not be built/executed within the run budget` - Result: not run ## Evidence - Failing build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=1593787 - First build it occurred: after #131869 (merged 2026-09-09, commit 7c35b7d), which introduced this test (computed within the scanned window; may not be the true origin) - Suspected regressing change: #131869 (added the test with the accept outside the catch) ## Help wanted - Likely author: `@wfurt` (authored #131869 that added the test) - Area owners (`area-System.Net.Sockets`): `@dotnet/ncl` --- Filed by [`ci-failure-fix`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-fix.md). Comment here or on the workflow file to suggest changes; [`ci-failure-scan-feedback`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-scan-feedback.md) reads in-scope feedback daily and opens (or updates) a PR with prompt edits. Structured data: ```json { "artifact_kind": "help", "linked_kbe": 133778, "workflow_artifact": "ci-fix" } ``` > Generated by [CI Outer-Loop Failure Fixer](https://github.com/dotnet/runtime/actions/runs/34916254170) · opus48 · 764.8 AIC · ⌖ 20.8 AIC · ⊞ 19.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fruntime+%22gh-aw-workflow-id%3A+ci-failure-fix%22&type=pullrequests) <!-- gh-aw-agentic-workflow: CI Outer-Loop Failure Fixer, engine: copilot, model: claude-opus-4.8, id: 34916254170, workflow_id: ci-failure-fix, run: https://github.com/dotnet/runtime/actions/runs/34916254170 --> <!-- gh-aw-workflow-id: ci-failure-fix --> <!-- gh-aw-workflow-call-id: dotnet/runtime/ci-failure-fix --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wfurt
pushed a commit
that referenced
this pull request
Sep 17, 2026
…indows/Linux (refs #133778) (#133925) Workflow artifact: ci-fix Artifact kind: help Linked KBE: #133778 > [!NOTE] > This is an AI/Copilot-generated **best-effort** fix attempt that I could not fully validate. It is a starting point for a maintainer, not a finished change. Please review the analysis below before merging. ## Root cause (best analysis) `System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(useAsync: True)` fails on `windows-x64`/`linux-x64` (`TestReadyToRun_Libraries` and the runtime pipeline): ``` System.Net.Sockets.SocketException : An existing connection was forcibly closed by the remote host. at System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(Boolean useAsync) in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs:line 527 ``` Line 527 is the `AcceptAsync`/`Accept` call, **not** the receive. The test (added by #131869) closes an IPv4 peer with `SO_LINGER=0` so the kernel sends an immediate RST, then expects the listener to still deliver the healthy IPv6 peer's byte. The `try { ... } catch (SocketException)` block only wraps the `ReceiveAsync`; the `accept` that precedes it sits **outside** the `try`. On Windows and Linux the reset is surfaced by `accept` itself (`WSAECONNRESET` / "connection forcibly closed"), which escapes the catch and fails the test, even though the very next accept iteration would return the healthy peer. macOS discards the reset at accept (which #131869 hardened `EndPoint.Create` for), so it doesn't hit this path — which is why the test passed there but not on Windows/Linux. The test comment already anticipates this ("Some platforms surface the reset connection from accept()"), but the code doesn't guard the accept accordingly. ## Attempted fix Move the `accept` inside the existing `try` block so a `SocketException` from either the accept or the following receive is tolerated, and the loop retries to accept the healthy peer on the next iteration (the loop already runs up to 2 accepts). No production code changes; the listener-stays-healthy assertion is preserved. This is **not** a test-disable — the test still runs and still asserts the healthy peer's message is received. ## What is unverified / where I need help - I could not build and run `System.Net.Sockets.Tests` in this environment, so I have not confirmed the test now passes on windows-x64/linux-x64 while still catching a genuine listener regression. - Please confirm the intended contract: on Windows/Linux, should the RST be observable at `accept` (making tolerating it here correct), or should the runtime itself suppress the reset connection from the accept loop the way macOS does? If the latter, the real fix belongs in the socket accept path rather than the test. ## Validation - Command: `not run because the sockets functional test suite could not be built/executed within the run budget` - Result: not run ## Evidence - Failing build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=1593787 - First build it occurred: after #131869 (merged 2026-09-09, commit 7c35b7d), which introduced this test (computed within the scanned window; may not be the true origin) - Suspected regressing change: #131869 (added the test with the accept outside the catch) ## Help wanted - Likely author: `@wfurt` (authored #131869 that added the test) - Area owners (`area-System.Net.Sockets`): `@dotnet/ncl` --- Filed by [`ci-failure-fix`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-fix.md). Comment here or on the workflow file to suggest changes; [`ci-failure-scan-feedback`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-scan-feedback.md) reads in-scope feedback daily and opens (or updates) a PR with prompt edits. Structured data: ```json { "artifact_kind": "help", "linked_kbe": 133778, "workflow_artifact": "ci-fix" } ``` > Generated by [CI Outer-Loop Failure Fixer](https://github.com/dotnet/runtime/actions/runs/34916254170) · opus48 · 764.8 AIC · ⌖ 20.8 AIC · ⊞ 19.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fruntime+%22gh-aw-workflow-id%3A+ci-failure-fix%22&type=pullrequests) <!-- gh-aw-agentic-workflow: CI Outer-Loop Failure Fixer, engine: copilot, model: claude-opus-4.8, id: 34916254170, workflow_id: ci-failure-fix, run: https://github.com/dotnet/runtime/actions/runs/34916254170 --> <!-- gh-aw-workflow-id: ci-failure-fix --> <!-- gh-aw-workflow-call-id: dotnet/runtime/ci-failure-fix --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jtschuster
pushed a commit
to jtschuster/runtime
that referenced
this pull request
Sep 18, 2026
…indows/Linux (refs dotnet#133778) (dotnet#133925) Workflow artifact: ci-fix Artifact kind: help Linked KBE: dotnet#133778 > [!NOTE] > This is an AI/Copilot-generated **best-effort** fix attempt that I could not fully validate. It is a starting point for a maintainer, not a finished change. Please review the analysis below before merging. ## Root cause (best analysis) `System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(useAsync: True)` fails on `windows-x64`/`linux-x64` (`TestReadyToRun_Libraries` and the runtime pipeline): ``` System.Net.Sockets.SocketException : An existing connection was forcibly closed by the remote host. at System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(Boolean useAsync) in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs:line 527 ``` Line 527 is the `AcceptAsync`/`Accept` call, **not** the receive. The test (added by dotnet#131869) closes an IPv4 peer with `SO_LINGER=0` so the kernel sends an immediate RST, then expects the listener to still deliver the healthy IPv6 peer's byte. The `try { ... } catch (SocketException)` block only wraps the `ReceiveAsync`; the `accept` that precedes it sits **outside** the `try`. On Windows and Linux the reset is surfaced by `accept` itself (`WSAECONNRESET` / "connection forcibly closed"), which escapes the catch and fails the test, even though the very next accept iteration would return the healthy peer. macOS discards the reset at accept (which dotnet#131869 hardened `EndPoint.Create` for), so it doesn't hit this path — which is why the test passed there but not on Windows/Linux. The test comment already anticipates this ("Some platforms surface the reset connection from accept()"), but the code doesn't guard the accept accordingly. ## Attempted fix Move the `accept` inside the existing `try` block so a `SocketException` from either the accept or the following receive is tolerated, and the loop retries to accept the healthy peer on the next iteration (the loop already runs up to 2 accepts). No production code changes; the listener-stays-healthy assertion is preserved. This is **not** a test-disable — the test still runs and still asserts the healthy peer's message is received. ## What is unverified / where I need help - I could not build and run `System.Net.Sockets.Tests` in this environment, so I have not confirmed the test now passes on windows-x64/linux-x64 while still catching a genuine listener regression. - Please confirm the intended contract: on Windows/Linux, should the RST be observable at `accept` (making tolerating it here correct), or should the runtime itself suppress the reset connection from the accept loop the way macOS does? If the latter, the real fix belongs in the socket accept path rather than the test. ## Validation - Command: `not run because the sockets functional test suite could not be built/executed within the run budget` - Result: not run ## Evidence - Failing build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=1593787 - First build it occurred: after dotnet#131869 (merged 2026-09-09, commit 7c35b7d), which introduced this test (computed within the scanned window; may not be the true origin) - Suspected regressing change: dotnet#131869 (added the test with the accept outside the catch) ## Help wanted - Likely author: `@wfurt` (authored dotnet#131869 that added the test) - Area owners (`area-System.Net.Sockets`): `@dotnet/ncl` --- Filed by [`ci-failure-fix`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-fix.md). Comment here or on the workflow file to suggest changes; [`ci-failure-scan-feedback`](https://github.com/dotnet/runtime/blob/main/.github/workflows/ci-failure-scan-feedback.md) reads in-scope feedback daily and opens (or updates) a PR with prompt edits. Structured data: ```json { "artifact_kind": "help", "linked_kbe": 133778, "workflow_artifact": "ci-fix" } ``` > Generated by [CI Outer-Loop Failure Fixer](https://github.com/dotnet/runtime/actions/runs/34916254170) · opus48 · 764.8 AIC · ⌖ 20.8 AIC · ⊞ 19.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fruntime+%22gh-aw-workflow-id%3A+ci-failure-fix%22&type=pullrequests) <!-- gh-aw-agentic-workflow: CI Outer-Loop Failure Fixer, engine: copilot, model: claude-opus-4.8, id: 34916254170, workflow_id: ci-failure-fix, run: https://github.com/dotnet/runtime/actions/runs/34916254170 --> <!-- gh-aw-workflow-id: ci-failure-fix --> <!-- gh-aw-workflow-call-id: dotnet/runtime/ci-failure-fix --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Workflow artifact: ci-fix
Artifact kind: help
Linked KBE: #133778
Note
This is an AI/Copilot-generated best-effort fix attempt that I could not fully validate. It is a starting point for a maintainer, not a finished change. Please review the analysis below before merging.
Root cause (best analysis)
System.Net.Sockets.Tests.AcceptDualStackResetTests.Accept_DualStackListener_PeerImmediatelyResets_ListenerStaysHealthy(useAsync: True)fails onwindows-x64/linux-x64(TestReadyToRun_Librariesand the runtime pipeline):Line 527 is the
AcceptAsync/Acceptcall, not the receive. The test (added by #131869) closes an IPv4 peer withSO_LINGER=0so the kernel sends an immediate RST, then expects the listener to still deliver the healthy IPv6 peer's byte. Thetry { ... } catch (SocketException)block only wraps theReceiveAsync; theacceptthat precedes it sits outside thetry. On Windows and Linux the reset is surfaced byacceptitself (WSAECONNRESET/ "connection forcibly closed"), which escapes the catch and fails the test, even though the very next accept iteration would return the healthy peer. macOS discards the reset at accept (which #131869 hardenedEndPoint.Createfor), so it doesn't hit this path — which is why the test passed there but not on Windows/Linux.The test comment already anticipates this ("Some platforms surface the reset connection from accept()"), but the code doesn't guard the accept accordingly.
Attempted fix
Move the
acceptinside the existingtryblock so aSocketExceptionfrom either the accept or the following receive is tolerated, and the loop retries to accept the healthy peer on the next iteration (the loop already runs up to 2 accepts). No production code changes; the listener-stays-healthy assertion is preserved. This is not a test-disable — the test still runs and still asserts the healthy peer's message is received.What is unverified / where I need help
System.Net.Sockets.Testsin this environment, so I have not confirmed the test now passes on windows-x64/linux-x64 while still catching a genuine listener regression.accept(making tolerating it here correct), or should the runtime itself suppress the reset connection from the accept loop the way macOS does? If the latter, the real fix belongs in the socket accept path rather than the test.Validation
not run because the sockets functional test suite could not be built/executed within the run budgetEvidence
Help wanted
@wfurt(authored Guard against empty remote address on macOS accept #131869 that added the test)area-System.Net.Sockets):@dotnet/nclFiled by
ci-failure-fix. Comment here or on the workflow file to suggest changes;ci-failure-scan-feedbackreads in-scope feedback daily and opens (or updates) a PR with prompt edits.Structured data:
{ "artifact_kind": "help", "linked_kbe": 133778, "workflow_artifact": "ci-fix" }