Repository navigation
fix: the stalled DNS connection test reads the close in real time - #738
Merged
Merged
Conversation
a_client_that_reads_no_answers_does_not_keep_its_connection failed in 15 of the 113 trunk-targeting CI runs since it landed, 11 of the 20 red runs since Oct 20, always as "the stalled connection was not closed" and on changes that did not touch the DNS listener (#678). The test is start_paused over a real loopback socket. Paused, tokio polls I/O once without waiting and then advances the clock to the next timer, so a read that is waiting on the kernel -- here for the close the server sends once its write times out -- finds nothing, the one-second timeout fires at once, and the panic blames a listener that did close. Nothing in virtual time waits for the kernel, so any real delay in delivering the close reads as the close not coming. The clock is resumed after the virtual idle sleep, so the final read loop's second is a real one. Everything that needed a paused clock is before that point: the fill loop's settle waits, and the server's ten second write timeout, which the sleep still advances past. Reproduced here by delaying only the TCP FIN and RST segments on loopback in a private network namespace (tc netem behind a flower filter on the flags). With the close 3 ms late the test failed with the CI panic, after the whole scenario had run, and again at 50 ms; with this change it passes at 3, 50 and 500 ms. CPU contention did not reproduce it on this 48-core machine: 120 runs, 12 at a time pinned to four cores, passed both before and after the change. With the server's write timeout removed the test still fails with the same message, so it still checks that a stalled connection is closed. the_fixed_listener_port_binds_again_after_a_server_first_close, the issue's lesser case, has not been seen in any CI log and is not touched. Closes #678
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.
a_client_that_reads_no_answers_does_not_keep_its_connection failed in 15 of the 113 trunk-targeting CI runs since it landed, 11 of the 20 red runs since Oct 20, always as "the stalled connection was not closed" and on changes that did not touch the DNS listener (#678).
The test is start_paused over a real loopback socket. Paused, tokio polls I/O once without waiting and then advances the clock to the next timer, so a read that is waiting on the kernel -- here for the close the server sends once its write times out -- finds nothing, the one-second timeout fires at once, and the panic blames a listener that did close. Nothing in virtual time waits for the kernel, so any real delay in delivering the close reads as the close not coming.
The clock is resumed after the virtual idle sleep, so the final read loop's second is a real one. Everything that needed a paused clock is before that point: the fill loop's settle waits, and the server's ten second write timeout, which the sleep still advances past.
Reproduced here by delaying only the TCP FIN and RST segments on loopback in a private network namespace (tc netem behind a flower filter on the flags). With the close 3 ms late the test failed with the CI panic, after the whole scenario had run, and again at 50 ms; with this change it passes at 3, 50 and 500 ms. CPU contention did not reproduce it on this 48-core machine: 120 runs, 12 at a time pinned to four cores, passed both before and after the change. With the server's write timeout removed the test still fails with the same message, so it still checks that a stalled connection is closed.
the_fixed_listener_port_binds_again_after_a_server_first_close, the issue's lesser case, has not been seen in any CI log and is not touched.
Closes #678
Summary
What does this change, and why?
Checklist
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningsis cleancargo test --workspacepassesdoc/updated if behavior changed (mdbook buildstill clean)Notes for reviewers
Anything worth calling out -- tricky bits, follow-ups, or intentionally out-of-scope items.