Repository navigation
fix(quic): a resumed client times out on its own idle timer - #8442
Conversation
Quest-Claim: a6f2e92e5ca52c6a
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 seconds. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 519277c.
No actionable findings identified. Checking the idle deadline at the shared connection event boundary is a focused fix: an expired connection is drained before queued packets can refresh its timer or RTT estimate, while packets read before expiry retain the existing ordering. The mock-time regression explicitly exercises buffered datagrams on resume and checks TimedOut.
Verification: GitHub-only static review of the full patch, timer and close paths, endpoint timestamps, and Tokio driver ordering. No builds, tests, or multi-client SIGSTOP field check were run.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Verified the idle-timeout fix and its regression test; no actionable findings.
- Idle deadline checked in
handle_event:Connection::handle_eventnow fireshandle_timeoutfor a datagram read afterTimer::Idleexpires, before the migration/remote check, instead of letting it restart the timer and fold the stall into the RTT. - Resumed-client regression test:
resumed_client_times_outstops a client past a 100 ms idle timeout, then reads its buffered datagrams at 4x the timeout and assertsDrained/TimedOut. - Quest completed:
quest/m1/quic-resume-idle.mdand itsquest/m1/README.mdbullet are removed.
Verification: cargo test --release -p moq-quic --lib resumed_client_times_out passes with the fix and fails at rs/moq-quic/src/tests/mod.rs:2221 (assertion failed: conn.is_drained()) with the guard removed; rustfmt --edition 2024 --check is clean on both changed Rust files.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
|
quest land watch: the merge queue removed this pull request at 519277c because its The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
… moq-relay web::tests::http_refused_subscribe_counts_no_request
PASS [ 0.032s] (5046/8420) moq-relay web::tests::http_fetch_meters_bytes_without_session_presence
PASS [ 0.020s] (5047/8420) moq-relay web::tests::https_settings_require_the_https_listener
PASS [ 0.018s] (5048/8420) moq-relay web::tests::set_mtls_extension_injects_marker
PASS [ 0.030s] (5049/8420) moq-relay web::tests::serve_captures_the_socket_on_every_listener
PASS [ 0.019s] (5050/8420) moq-relay web::tests::unreachable_timeout_waits_forever
PASS [ 0.093s] (5051/8420) moq-relay uring::tests::pending_admission_is_abandoned_at_the_deadline
PASS [ 0.020s] (5052/8420) moq-relay web::tests::web_acceptor_skips_capture_without_websockets
PASS [ 0.039s] (5053/8420) moq-relay web::tests::https_listener_verifies_tls_qmux_client_certificates
PASS [ 0.023s] (5054/8420) moq-relay web::tests::web_acceptor_surfaces_the_accepted_socket
PASS [ 0.017s] (5055/8420) moq-relay websocket::tests::axum_ws_negotiates_newest_moq_alpn
PASS [ 0.017s] (5056/8420) moq-relay websocket::tests::axum_ws_rejects_unsupported_subprotocols
PASS [ 0.055s] (5057/8420) moq-relay web::tests::https_listener_serves_tls_qmux
PASS [ 0.027s] (5058/8420) moq-relay web::tests::websocket_upgrade_advertises_the_configured_receive_windows
PASS [ 0.017s] (5059/8420) moq-relay websocket::tests::select_subprotocol_requires_a_match_when_any_are_offered
PASS [ 0.019s] (5060/8420) moq-relay websocket::tests::stalled_setup_is_closed_at_the_deadline
PASS [ 0.041s] (5061/8420) moq-relay web::tests::web_listeners_hand_rtmp_to_its_consumer
PASS [ 0.016s] (5062/8420) moq-relay websocket::tests::supported_subprotocols_only_lists_configured_alpns
PASS [ 0.015s] (5063/8420) moq-relay websocket::tests::supported_subprotocols_lists_full_matrix
PASS [ 0.027s] (5064/8420) moq-relay websocket::tests::keep_alive_reaps_a_silent_peer
PASS [ 0.015s] (5065/8420) moq-relay websocket::tests::supported_subprotocols_preserves_moq_preference_across_qmux_versions
PASS [ 1.666s] (5066/8420) moq-quic tests::rate::a_live_cap_change_takes_effect
PASS [ 0.038s] (5067/8420) moq-relay websocket::tests::every_advertised_pair_is_acceptable
PASS [ 0.113s] (5068/8420) moq-relay web::tests::drain::later_keep_alive_upgrade_enters_from_the_handler
PASS [ 0.123s] (5069/8420) moq-relay web::tests::server_name_is_the_sni_never_the_host
PASS [ 1.374s] (5070/8420) moq-quic tests::scream::smaller_queue_than_bbr3
PASS [ 0.223s] (5071/8420) moq-relay web::tests::an_unanswered_http_admission_is_refused_at_the_timeout
PASS [ 0.236s] (5072/8420) moq-relay web::tests::drain::accepted_web_connection_holds_drain_until_upgrade
PASS [ 0.222s] (5073/8420) moq-relay web::tests::slow_request_headers_are_timed_out
FAIL [ 0.494s] (5074/8420) moq-relay config::tests::repository_configs_load
stdout ───
running 1 test
test config::tests::repository_configs_load ... FAILED
failures:
failures:
config::tests::repository_configs_load
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 327 filtered out; finished in 0.48s
stderr ───
thread 'config::tests::repository_configs_load' (2023874) panicked at rs/moq-relay/src/config.rs:986:51:
/var/cache/github-runner-work/moq/moq/rs/moq-relay/../../test/audio-quality/congestion.toml: unknown field `down`, expected one of `limits`, `listen`, `server`, `connect`, `client`, `quic`, `log`, `runtime`, `cluster`, `auth`, `web`, `stats`, `cache`, `internal`, `drain_timeout`, `file`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Cancelling due to test failure: 23 tests still running
PASS [ 1.540s] (5075/8420) moq-quic tests::sim::priority_on_a_clean_path
PASS [ 4.579s] (5076/8420) moq-mux jitter::tests::a_wandering_track_sent_latest_does_not_move_the_rate
PASS [ 4.803s] (5077/8420) moq-mux jitter::tests::an_unmarked_step_reacquires_the_clock
PASS [ 1.083s] (5078/8420) moq-relay uring::tests::pending_admission_is_abandoned_at_the_handshake_deadline
PASS [ 2.412s] (5079/8420) moq-quic tests::sim::random_workloads
PASS [ 10.004s] (5080/8420) moq-mux container::ts::export_timing_test::two_legs_render_a_drifting_source_the_same_way
PASS [ 2.491s] (5081/8420) moq-relay::integration auth_lifetime::a_fixed_lease_still_expires
PASS [ 2.706s] (5082/8420) moq-relay::integration auth_lifetime::a_late_decision_within_the_deadline_admits
PASS [ 3.792s] (5083/8420) moq-relay::integration auth_lifetime::a_configured_token_is_the_connection_credential
PASS [ 4.217s] (5084/8420) moq-relay::integration auth_lifetime::a_certificate_admits_only_what_the_server_grants
PASS [ 5.177s] (5085/8420) moq-relay::integration auth_lifetime::a_moved_root_closes_live_sessions
PASS [ 5.246s] (5086/8420) moq-relay::integration auth_lifetime::a_moved_tier_retags_the_live_session
PASS [ 5.287s] (5087/8420) moq-relay::integration auth_lifetime::a_rechecked_grant_resizes_live_sessions
PASS [ 9.546s] (5088/8420) moq-mux jitter::tests::the_clock_follows_a_drifting_source_for_a_day
PASS [ 6.316s] (5089/8420) moq-relay::drills no_publisher_never_delivers::loopback
PASS [ 7.564s] (5090/8420) moq-relay::drills interrupted_publisher_republishes_new_content::loopback
PASS [ 7.801s] (5091/8420) moq-relay::drills cancel_under_backpressure_releases_the_reader::loopback
PASS [ 7.979s] (5092/8420) moq-relay::drills bursts_cross_a_cluster::loopback
PASS [ 7.974s] (5093/8420) moq-relay::drills bursts_cross_a_transport_peer::loopback
PASS [ 9.082s] (5094/8420) moq-relay::drills lite_subscriber_max_age_skips_bufferbloat
PASS [ 9.086s] (5095/8420) moq-relay::drills ietf_subscriber_max_age_skips_bufferbloat
PASS [ 9.793s] (5096/8420) moq-relay::drills bursts_cross_a_flapping_peer::loopback
PASS [ 10.748s] (5097/8420) moq-relay::drills relay_killed_mid_group_aborts_then_resumes::loopback
────────────
Summary [ 42.505s] 5097/8420 tests run: 5096 passed, 1 failed, 134 skipped
FAIL [ 0.494s] (5074/8420) moq-relay config::tests::repository_configs_load
stdout ───
running 1 test
test config::tests::repository_configs_load ... FAILED
failures:
failures:
config::tests::repository_configs_load
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 327 filtered out; finished in 0.48s
stderr ───
thread 'config::tests::repository_configs_load' (2023874) panicked at rs/moq-relay/src/config.rs:986:51:
/var/cache/github-runner-work/moq/moq/rs/moq-relay/../../test/audio-quality/congestion.toml: unknown field `down`, expected one of `limits`, `listen`, `server`, `connect`, `client`, `quic`, `log`, `runtime`, `cluster`, `auth`, `web`, `stats`, `cache`, `internal`, `drain_timeout`, `file`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: 3323/8420 tests were not run due to test failure (run with --no-fail-fast to run all tests, or run with --max-fail)
error: test run failed
mbx[cache]: object cache: 0 hits, 28 misses, 63 not looked up, 41 bypassed; 0 B downloaded, 0 B uploaded, 7.1 GiB stored locally
mbx[cache]: 8m 4s session; 0s estimated compiler time avoided (summed across compilations)
mbx[cache]: 63 compilations had no usable prior inputs or matching prediction for a lookup
mbx[cache]: bypass reasons: 41 unsupported-crate-type
mbx[cache]: Cargo artifact reuse and CI cache archive transfers are not included above
mbx[savings]: across 18320 builds, rustc has been excused from 15d 36m of its duties.
error: recipe `test` failed on line 375 with exit code 100
error: recipe `test-changed` failed on line 92 with exit code 100
##[endgroup]
##[error]ci: test rs failed
##[error]Process completed with exit code 100.
##[group]Run actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9
with:
name: cargo-timings
path: target/cargo-timings/cargo-timing-*.html
if-no-files-found: ignore
retention-days: 14
compression-level: 6
overwrite: false
include-hidden-files: false
archive: true
env:
BASE: d7699f81ae9fb7dde8a463ee49072a7974d47317
CHECK_FAIL_FAST: 1
NEXTEST_TEST_THREADS: 24
MBX_CACHE_LINKS: 1
MBX_REMOTE_URL:
##[endgroup]
With the provided path, there will be 2 files uploaded
Artifact name is valid!
Root directory input is valid!
Uploading artifact: cargo-timings.zip
Beginning upload of artifact content to blob storage
Uploaded bytes 175606
Finished uploading artifact content to blob storage!
SHA256 digest of uploaded artifact is 83527e3763f3cf880a9119ff903fe32a7270fb07d2657f720a4824f198c21a7c
Finalizing artifact upload
Artifact cargo-timings successfully finalized. Artifact ID 13815695385
Artifact cargo-timings has been successfully uploaded! Final size is 175606 bytes. Artifact ID is 13815695385
Artifact download URL: https://github.com/moq-dev/moq/actions/runs/42926770309/artifacts/13815695385
Post job cleanup.
##[start-action display=Run jdx/mr-boxington-action@88acf2bb50552c7c508cbac13e5f8c576b95b1de;id=__ebe18a1c-5391-4cb1-b68f-f8befa4daece.mbx]
##[end-action id=__ebe18a1c-5391-4cb1-b68f-f8befa4daece.mbx;outcome=skipped;conclusion=skipped;duration_ms=0]
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/a378a544-cd40-43db-a304-9a9251dde64a' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/moq/moq
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 1870 objects and 33 action results (7.1 GiB logical); 49.9 GiB logical remain
dropped 2 stale checkout records
dropped 3 session event streams
removed 0 learned incremental units and 2 checkout directories (3.7 KiB logical); 110 units (5.0 GiB logical) remain
removed 3 target directories (32.0 GiB logical, 2 abandoned and 1 live); 0 B logical remain
Cleaning up orphan processes
(written by quest land watch) |
|
codex, gpt-6.1-sol, medium effort land fixer note:
(written by codex, gpt-6.1-sol, medium effort) |
|
codex, gpt-6.1-sol, medium effort land fixer: python3 sh/js/audit.test.py
..
----------------------------------------------------------------------
Ran 2 tests in 0.200s
OK
python3 sh/gh/nightly.test.py
..............
exited 100 in 940013ms:
Switched to a new branch 'scratch-8442'
Auto-merging quest/m1/README.md
Merge made by the 'ort' strategy.
quest/m1/README.md | 1 -
quest/m1/quic-resume-idle.md | 46 ------------------------------
rs/moq-quic/src/connection/mod.rs | 7 +++++
rs/moq-quic/src/tests/mod.rs | 60 +++++++++++++++++++++++++++++++++++++++
4 files changed, 67 insertions(+), 47 deletions(-)
delete mode 100644 quest/m1/quic-resume-idle.md
exec
/bin/bash -lc 'tail -n 12 /tmp/tmp.b5QQ8BPZqm/bun.log; tail -n 3 /tmp/quest-8442-ci.log' in /work
succeeded in 0ms:
tail: cannot open '/tmp/tmp.b5QQ8BPZqm/bun.log' for reading: No such file or directory
0 0 check justfile
822 4151 whole run
error: recipe `ci` failed on line 119 with exit code 100
exec
/bin/bash -lc "rg -n 'ci: .* (failed|passed)|resumed_client_times_out|Summary|unknown field' /tmp/quest-8442-ci.log | tail -n 25; git diff --exit-code; git diff --cached --exit-code" in /work
succeeded in 1839ms:
46:ci: check nix passed in 2s
49:ci: check justfile passed in 0s
51:ci: check gh passed in 14s
53:ci: check rs passed in 263s
55:ci: test rs failed in 386s
6062: PASS [ 0.010s] (4655/8420) moq-quic tests::resumed_client_times_out
6373: /work/rs/moq-relay/../../test/audio-quality/congestion.toml: unknown field `down`, expected one of `limits`, `listen`, `server`, `connect`, `client`, `quic`, `log`, `runtime`, `cluster`, `auth`, `web`, `stats`, `cache`, `internal`, `drain_timeout`, `file`
6399: Summary [ 74.188s] 4953/8420 tests run: 4952 passed, 1 failed, 134 skipped
6416: /work/rs/moq-relay/../../test/audio-quality/congestion.toml: unknown field `down`, expected one of `limits`, `listen`, `server`, `connect`, `client`, `quic`, `log`, `runtime`, `cluster`, `auth`, `web`, `stats`, `cache`, `internal`, `drain_timeout`, `file`
6441:ci: test gh passed in 753s
6757:ci: check workers passed in 0s
6758:ci: check biome passed in 0s
6761:ci: check quest passed in 0s
6762:ci: check js passed in 0s
6768:ci: check shell passed in 0s
6769:ci: test js passed in 0s
6771:ci: check large_files passed in 0s
6776:ci: check text passed in 0s
6777:ci: check toml passed in 0s
6782:ci: check nix passed in 0s
6784:ci: check justfile passed in 0s
6786:ci: check markdown passed in 0s
6788:ci: check gh passed in 0s
6790:ci: test gh passed in 0s
6885:ci: test rs failed
codex
The gate finished. All checks and tooling tests passed; the Rust test run stopped only at the same failure confirmed on `base`. I’ll record #8442 for requeue unchanged with `cause: base`, including the validation limits.
exec
/bin/bash -lc "git switch base; python3 - <<'PY'
import json
from pathlib import Path
result = [{
'number': 8442,
'action': 'requeue',
'cause': 'base',
'reason': 'Unchanged PR; base reproduces the failure; ran nix develop --command just rs test -p moq-relay --lib -E test(=config::tests::repository_configs_load) on base and nix develop --command just ci base on a scratch merge; both fail only at the same base test.',
'failure': 'moq-relay config::tests::repository_configs_load',
'comment': 'The failure reproduces on base alone: "'`test/audio-quality/congestion.toml: unknown field `down``. The repository configuration test scans tracked TOML under test/ and treats this shaper profile as relay configuration. Neither that test nor the profile is changed by this PR. The scratch merge was clean. `nix develop --command just ci base` passed all check lanes and the repository tooling tests; the Rust run passed 4,952 tests, including `moq-quic tests::resumed_client_times_out`, then stopped at the same configuration failure (3,467 tests not run). The pinned PR head remains unchanged. The base configuration test needs to distinguish relay fixtures from shaper profiles.'"'
}]
Path('quest-land-result.json').write_text(json.dumps(result, indent=2) + '\\n')
PY
git status --short; git rev-parse pr-8442" in /work
succeeded in 2281ms:
Switched to branch 'base'
?? quest-land-result.json
519277c2f55d4466b0b88f5f5852906111c2f30c
exec
/bin/bash -lc "python3 - <<'PY'
import json
from pathlib import Path
path = Path('quest-land-result.json')
result = json.loads(path.read_text())
result[0]['comment'] = result[0]['comment'].replace('"'`test/audio-quality/congestion.toml: unknown field `down``'"', '"'`test/audio-quality/congestion.toml` reports unknown field "down"'"')
path.write_text(json.dumps(result, indent=2) + '\\n')
assert len(result) == 1 and result[0]['number'] == 8442
assert len(result[0]['failure']) <= 200
PY" in /work
succeeded in 200ms:
codex
Wrote [quest-land-result.json](/work/quest-land-result.json): requeue #8442 unchanged with `cause: base`.
The failure reproduces on base alone. Scratch-merge checks passed; tests stopped at the same base failure. The PR’s new regression test passed.
tokens used
81,932
Wrote [quest-land-result.json](/work/quest-land-result.json): requeue #8442 unchanged with `cause: base`.
The failure reproduces on base alone. Scratch-merge checks passed; tests stopped at the same base failure. The PR’s new regression test passed.
fixer result written
(written by codex, gpt-6.1-sol, medium effort) |
…idle # Conflicts: # quest/m1/README.md
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The only new commit is 125a71e, a merge of origin/main. Its only code change is the quest/m1/README.md conflict resolution, which matches main's version (the stale bullet list is gone); the Rust fix and its regression test are byte-identical to the approved 519277c.
Verification: cargo test --release -p moq-quic --lib resumed_client_times_out passes on the merged tree (1 passed), and rustfmt --edition 2024 --check is clean on both changed Rust files.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Summary
A
moq-quicconnection now times out on the first datagram it reads past its idle deadline, instead of letting that datagram restart the idle timer.A client stopped past its idle timeout (SIGSTOP, a VM pause) wakes with the relay's ACKs and keep-alives queued on its socket. The tokio driver handles received datagrams before overdue timers, so those stale packets restarted
Timer::Idle. Their ACKs also folded the stop into the RTT sample, andreset_idle_timeout'smax(idle, 3 * PTO)then stretched the new deadline to about a minute. The client only learned the connection was gone from a stateless reset, which the endpoint-wide reset limit throttles when many clients resume together.Connection::handle_eventnow checksTimer::Idleagainst the datagram's read time first, and fires the timeout instead of handling the datagram once it has expired. The check lives in moq-quic, not in the driver's poll order, so every driver (tokio, uring) gets it and it holds whichever of the endpoint and connection tasks runs first after the resume. It touches only the idle timer, so ordinary timers (loss detection, keep-alive) keep their current ordering relative to queued packets.Reproduced first with
resumed_client_times_out(rs/moq-quic/src/tests/mod.rs), on mock time: the server times out while the client is stopped, then the client reads its buffered datagrams at 4x the idle timeout, as the driver does. Before the fix the connection stayed alive (assertion failed: conn.is_drained()); after, it ends withTimedOut.Finishes
quest/m1/quic-resume-idle.md.Choices made without asking
moq-quic'shandle_eventrather than reorderingConnectionDriver::pollto run timers beforeprocess_conn_events. A driver reorder would also fire every other overdue timer ahead of packets that were read before it expired (for example, a spurious PTO under a busy runtime), and moq-uring would need the same change.Public API and wire
None.
Caused by
No single PR: the receive-before-timer ordering and the idle restart on receipt are as old as the code, inherited from quinn.
Follow-ups
Models
Size: M
Implementer: Claude Code, Opus 5.5, medium effort
Closes #8382
🤖 Generated with Claude Code
(Written by Opus 5.5 with medium)
Review
Rewritten by
quest reviewafter each round.01a1c61e-84f1-7942-a8ec-10d41d9f80d2, round 1The change prevents buffered datagrams from reviving an expired connection and adds a simulated-time regression test, with no actionable defects found; builds and tests were intentionally not run.