From 4943cbb2dd18060aba6ee043f4f2fd6317f5b056 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 09:58:17 -0800 Subject: [PATCH 1/4] chore(quest): claim quest/m1/dispatch-cancel-race.md Quest-Claim: 88ef78d8908669c2 From 9bf391fe481f8ba5c802dc02286c94a181bad0d0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 10:19:51 -0800 Subject: [PATCH 2/4] fix(ci): a cancelled dispatch lane stops its phase promptly A trapped Bash shell defers its trap while a foreground command runs, and a TERM that lands after its last pending-trap check but before it forks never reaches the child. A lane's phase subshell could then run its whole phase before the cancel took effect. The lane and its phase now fork every command as a job and wait for it: `wait` runs the trap at once, and Bash runs a trap left pending across a fork as soon as the fork returns. The trap resends TERM to the lane's process group, so a job forked after the first TERM still gets it, then waits for every job to exit. A new test forces the window with an xtrace PS4 hook that terminates the run right before the phase forks rs's lint. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 1 - quest/m1/dispatch-cancel-race.md | 25 ----- sh/dispatch-query.test.py | 45 +++++++++ sh/dispatch.sh | 164 ++++++++++++++++++++----------- 4 files changed, 149 insertions(+), 86 deletions(-) delete mode 100644 quest/m1/dispatch-cancel-race.md diff --git a/quest/m1/README.md b/quest/m1/README.md index d3032313f7a..8f5c3b9df23 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -54,7 +54,6 @@ moves what remains of them once their claims merge. - [Tests run on a mocked clock](/quest/m1/mock-clock-tests/README.md) - about 300 tests stop proving nothing happened by waiting, sleeping to settle, or asserting on elapsed time - [moq-mux reads H.265 SPS without scuffle](/quest/m1/h265-sps-reader.md) - moq-mux's own SPS reader replaces scuffle-h265 and scuffle-expgolomb, and the fuzz regressions still pass - [QMux reads the app-limited flag on big-endian Linux](/quest/m1/qmux-app-limited-endian.md) - the send-rate filter's app-limited mask follows the target's byte order -- [A cancelled dispatch lane stops its phase promptly](/quest/m1/dispatch-cancel-race.md) - a TERM landing just before a lane forks its phase's command still stops that command, instead of waiting out the phase ## Related diff --git a/quest/m1/dispatch-cancel-race.md b/quest/m1/dispatch-cancel-race.md deleted file mode 100644 index 30cf20ff1bb..00000000000 --- a/quest/m1/dispatch-cancel-race.md +++ /dev/null @@ -1,25 +0,0 @@ -# [S] A cancelled dispatch lane stops its phase promptly - -## Goal - -Terminating a `sh/dispatch.sh` run stops each lane's running phase promptly, -even when the group TERM lands after a lane's phase subshell checks for a -pending trap but before it forks the phase's command. A test forces that -window and proves the phase's command gets the signal. - -## Plan - -Planned 2026-11-07 (maintainer, /quest-complete) from a follow-up of -[moq-dev/moq#8138](https://github.com/moq-dev/moq/pull/8138). It was found by -reading Bash's source, not reproduced. In the lane shell (`sh/dispatch.sh`, -the `trap 'exit 143' TERM` around the phase), a TERM arriving between the -pending-trap check and the fork of the phase's command never reaches that -command. The deferred trap then runs only once the whole phase exits, so a -cancel can take as long as the phase. 8138's terminate test does not hit it, -because its phase is already running when TERM arrives. - -Reproduce it first, by forcing the window as 8138 forced its own (for example -a hook that sends the group TERM right before the fork) on the pinned Bash. -Then close the window at its cause, for example by having the lane resend its -pending TERM to the phase's process group once the fork returns. No retry -loop or timeout. Extend `sh/dispatch-query.test.py` with the forced case. diff --git a/sh/dispatch-query.test.py b/sh/dispatch-query.test.py index 340f337ed28..76474c49d0c 100644 --- a/sh/dispatch-query.test.py +++ b/sh/dispatch-query.test.py @@ -1,9 +1,11 @@ #!/usr/bin/env python3 """Exercise pre-setup selection with real diffs and no development toolchain.""" +import contextlib import os import select import shutil +import signal import statistics import subprocess import sys @@ -614,6 +616,49 @@ def test_terminating_the_run_stops_the_running_phase(self): with self.assertRaises(ProcessLookupError): os.kill(int(pid.read_text()), 0) + def test_terminating_just_before_a_phase_forks_its_command_stops_it(self): + # PS4 expands in the phase's shell after its last pending-trap check and + # before it forks a foreground command. The hook terminates the run, + # then blocks until the lane's group TERM kills it, so that TERM is + # pending when the shell would fork rs's lint. + script = (self.repo / "sh/dispatch.sh").read_text().splitlines() + line = next(n for n, text in enumerate(script, 1) if text.lstrip().startswith("check:rs)")) + hook = self.root / "hook.sh" + hook.write_text(f"""exec 9>/dev/null +BASH_XTRACEFD=9 +declare -A hook=([dispatch.sh:{line}]=1) +PS4='${{hook[${{BASH_SOURCE##*/}}:$LINENO]:+$(kill -TERM $$; exec sleep 600)}}' +set -x +""") + pid = self.root / "pid" + self.stub_ci(f'''case "$*" in + "rs lint-changed "*) echo $$ >"{pid}"; exec sleep 600 ;; +esac +''') + self.change("rs/kio/src/clock/mod.rs") + for jobs in ("1", "3"): + with self.subTest(jobs=jobs): + pid.unlink(missing_ok=True) + run = subprocess.Popen( + ["bash", "sh/dispatch.sh", "ci", "origin/main"], + cwd=self.repo, + env=dict(self.env, CHECK_JOBS=jobs, BASH_ENV=str(hook)), + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + ) + try: + stderr = run.communicate(timeout=60)[1].decode() + finally: + run.kill() + if pid.exists(): + with contextlib.suppress(ProcessLookupError): + os.kill(int(pid.read_text()), signal.SIGKILL) + self.assertEqual(run.returncode, 143, stderr) + self.assertRegex(stderr, r"\d+ check rs \(cancelled\)") + if pid.exists(): + with self.assertRaises(ProcessLookupError): + os.kill(int(pid.read_text()), 0) + def test_ci_runs_one_lint_preamble_and_one_test_pass(self): self.change("rs/kio/src/clock/mod.rs") calls = self.run_ci() diff --git a/sh/dispatch.sh b/sh/dispatch.sh index 097be27e37d..3060f0fea58 100755 --- a/sh/dispatch.sh +++ b/sh/dispatch.sh @@ -314,41 +314,69 @@ if [[ -z "$all" ]]; then js_list=$js_changed fi -# Tracked and new files, skipping whatever .gitignore does (node_modules, target) -# and tracked files deleted from the working tree, which no formatter can read. +# A trapped shell defers its trap while a foreground command runs, and a TERM +# that lands just before the fork never reaches the command, so a cancel would +# wait for the whole phase. A lane and its phase therefore fork every command +# as a job: `wait` runs the trap at once, and Bash runs a trap left pending +# across the fork as soon as the fork returns. Async lists ignore INT and QUIT +# without job control; the job restores them for the command. +run() { + { + trap - INT QUIT + exec "$@" + } & + wait "$!" +} + +# A TERM trap's body: resend TERM to the lane's process group, which reaches a +# job forked after the first TERM, then return once every job has exited. The +# resend can land while Bash still records `wait` as its builtin, which makes +# the next `wait` return at once (see the EXIT trap below), so it waits again. +stop() { + trap '' TERM + kill -TERM 0 + until wait; do :; done +} + +# Writes the tracked and new files matching the patterns to $listed, +# NUL-separated, skipping whatever .gitignore does (node_modules, target) and +# tracked files deleted from the working tree, which no formatter can read. files() { - git ls-files -z --cached --others --exclude-standard -- "$@" | - while IFS= read -r -d '' file; do - [[ ! -e "$file" ]] || printf '%s\0' "$file" - done + local file + run git ls-files -z --cached --others --exclude-standard -- "$@" >"$listed.all" + while IFS= read -r -d '' file; do + [[ ! -e "$file" ]] || printf '%s\0' "$file" + done <"$listed.all" >"$listed" } -# One phase of one module. The bindings run their tests in their check; rs, -# js, py, and gh test right after it, in the same lane. +# One phase of one module, run through `run`. The bindings run their tests in +# their check; rs, js, py, and gh test right after it, in the same lane. step() { + local listed=$logs/$2.files case "$1:$2" in - check:js) just js check-changed "$js_list" ;; - check:workers) just js workers ;; - check:moq_sh) just infra moq-sh test ;; + check:js) run just js check-changed "$js_list" ;; + check:workers) run just js workers ;; + check:moq_sh) run just infra moq-sh test ;; # The check keeps docs and the isolated default-media lint; the test # phase enforces the other clippy checks in its existing build. - check:rs) just rs lint-changed "$rs_list" ;; + check:rs) run just rs lint-changed "$rs_list" ;; check:bench) - python3 bench/compare.test.py - bash sh/local.sh cargo check --locked --package moq-relay --package moq-bench --features moq-relay/io-uring + run python3 bench/compare.test.py + run bash sh/local.sh cargo check --locked --package moq-relay --package moq-bench --features moq-relay/io-uring ;; - check:quest) bash sh/quest.sh check ;; - check:drill) just test drill-sensitivity --apply-only ;; - check:obs_compile) just obs compile ;; + check:quest) run bash sh/quest.sh check ;; + check:drill) run just test drill-sensitivity --apply-only ;; + check:obs_compile) run just obs compile ;; check:flake) if [[ -z "${CI_STRICT:-}" && "$OSTYPE" == linux* && "$HOSTTYPE" == x86_64 ]]; then # Repart creates its namespace as a Nix builder, whose policy can # differ from this shell's. A fresh name prevents cached success. read -r nonce &1); then - printf '%s\n' "$result" >&2 - [[ "$result" == *MOQ_USERNS_UNAVAILABLE* ]] || return 1 + if ! run bash sh/local.sh nix build --impure --file nix/runner/userns-probe.nix \ + --argstr nonce "$nonce" --no-link --builders '' --max-jobs 1 --print-build-logs >"$logs/flake.probe" 2>&1; then + mapfile -t result <"$logs/flake.probe" + printf '%s\n' "${result[@]}" >&2 + [[ "${result[*]}" == *MOQ_USERNS_UNAVAILABLE* ]] || return 1 echo "$action: skipping flake; the local Nix builder cannot create user namespaces" >&2 return 77 fi @@ -356,54 +384,68 @@ step() { # Without KVM, QEMU falls back to emulation and the VM tests time # out long after. Refuse rather than skip, so a local gate never # reads as passing without them. - if [[ "$OSTYPE" == linux* && "$HOSTTYPE" == x86_64 ]] && ! bash sh/kvm.sh; then + if [[ "$OSTYPE" == linux* && "$HOSTTYPE" == x86_64 ]] && ! run bash sh/kvm.sh; then echo "$action: flake's NixOS VM tests need KVM, but /dev/kvm is missing; run them on a KVM host, as CI does" >&2 return 1 fi # 77 marks the probe's skip, so the check's own 77 is a failure. # Full build logs name a failing VM subtest; Nix otherwise keeps # only the last 25 lines, which are QEMU's cleanup. - bash sh/local.sh nix flake check --print-build-logs || { + run bash sh/local.sh nix flake check --print-build-logs || { local status=$? [[ -n "${CI_STRICT:-}" ]] || ((status != 77)) || status=1 return "$status" } ;; - check:ts_paths) python3 sh/gh/ts-paths.py ;; - check:biome) just biome ;; + check:ts_paths) run python3 sh/gh/ts-paths.py ;; + check:biome) run just biome ;; check:markdown) - uv run --no-project python sh/js/install.py frozen - sh/markdown.sh check + run uv run --no-project python sh/js/install.py frozen + run sh/markdown.sh check ;; - check:large_files) sh/large-files.sh "$large_base" ;; - check:text) sh/text.sh ;; + check:large_files) run sh/large-files.sh "$large_base" ;; + check:text) run sh/text.sh ;; check:python) - files '*.py' '*.pyi' | xargs -0 ruff check --force-exclude - files '*.py' '*.pyi' | xargs -0 ruff format --check --force-exclude + files '*.py' '*.pyi' + run xargs -0 ruff check --force-exclude <"$listed" + run xargs -0 ruff format --check --force-exclude <"$listed" ;; - check:toml) RUST_LOG=error taplo format --check ;; - check:nix) files '*.nix' | xargs -0 nixfmt --check ;; - check:justfile) files justfile '*/justfile' | xargs -0 -n1 just --fmt --check --justfile ;; - test:rs) just rs test-changed "$rs_list" ;; - test:js) just js test-changed "$js_list" ;; - test:py) just py test ;; - test:gh) just gh test ;; - fix:rs) just rs fix-changed "$rs_list" ;; - fix:js) just js fix-changed "$js_list" ;; - fix:biome) just biome-fix ;; + check:toml) RUST_LOG=error run taplo format --check ;; + check:nix) + files '*.nix' + run xargs -0 nixfmt --check <"$listed" + ;; + check:justfile) + files justfile '*/justfile' + run xargs -0 -n1 just --fmt --check --justfile <"$listed" + ;; + test:rs) run just rs test-changed "$rs_list" ;; + test:js) run just js test-changed "$js_list" ;; + test:py) run just py test ;; + test:gh) run just gh test ;; + fix:rs) run just rs fix-changed "$rs_list" ;; + fix:js) run just js fix-changed "$js_list" ;; + fix:biome) run just biome-fix ;; fix:markdown) - uv run --no-project python sh/js/install.py update - sh/markdown.sh fix + run uv run --no-project python sh/js/install.py update + run sh/markdown.sh fix ;; fix:python) - files '*.py' '*.pyi' | xargs -0 ruff check --fix --force-exclude - files '*.py' '*.pyi' | xargs -0 ruff format --force-exclude + files '*.py' '*.pyi' + run xargs -0 ruff check --fix --force-exclude <"$listed" + run xargs -0 ruff format --force-exclude <"$listed" + ;; + fix:toml) RUST_LOG=error run taplo format ;; + fix:nix) + files '*.nix' + run xargs -0 nixfmt <"$listed" ;; - fix:toml) RUST_LOG=error taplo format ;; - fix:nix) files '*.nix' | xargs -0 nixfmt ;; - fix:justfile) files justfile '*/justfile' | xargs -0 -n1 just --fmt --justfile ;; - *:shell) sh/shell.sh "$1" ;; - *) just "$2" "$1" ;; + fix:justfile) + files justfile '*/justfile' + run xargs -0 -n1 just --fmt --justfile <"$listed" + ;; + *:shell) run sh/shell.sh "$1" ;; + *) run just "$2" "$1" ;; esac } @@ -431,13 +473,13 @@ github() { # Run one lane's modules in order, appending each phase's seconds to the lane's # times file and announcing each phase's start and end on fd 3. A failed phase # skips the rest of that module, but independent modules still run. A TERM -# records the phase it cut short. +# stops the running phase, waits for it, and records the phase it cut short. run_lane() { times=$1 shift current= lane_status=0 - trap 'cancelled; exit 143' TERM + trap 'stop; cancelled; exit 143' TERM for module in "$@"; do phases=("$verb") # gh's tooling tests run on every diff: path selection would let a @@ -452,16 +494,18 @@ run_lane() { began=$SECONDS if github; then echo "::group::$current"; else echo "==> $current"; fi echo "$action: $current started" >&3 - # A plain statement, so errexit stays on inside the subshell. - set +e + # A job, as `run` explains; errexit stays on inside it. Untrapped, + # the phase would die on TERM at once, orphaning its commands. + # Trapped, it exits once they do, so a cancel returns only after + # the whole tree has exited. ( + trap - INT QUIT set -e - # A subshell dies on TERM at once, orphaning the phase's - # processes. Trapped, it exits once they do, so a cancel - # returns only after the whole tree has exited. - trap 'exit 143' TERM + trap 'stop; exit 143' TERM step "$phase" "$module" - ) + ) & + set +e + wait "$!" status=$? set -e ! github || echo "::endgroup::" @@ -543,7 +587,7 @@ trap 'exit 130' INT trap 'exit 143' TERM # Each lane runs in its own process group, so a cancel reaches its whole tree; -# a lane shell defers its own TERM trap until the running phase exits. +# a lane shell stops its running phase and waits for it before it exits. # Concurrent lanes buffer their output and print it whole when they finish; a # serial run streams instead. A buffered lane still announces each phase on the # run's stderr as it starts and ends, so a run cut off mid-phase, such as by a From 063ff29ec1969238ef2cef58cdfc754be37f24bb Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 10:32:18 -0800 Subject: [PATCH 3/4] fix(ci): a dispatch job keeps its caller's stdin Co-Authored-By: Claude Opus 5.5 --- sh/dispatch-query.test.py | 11 +++++++++++ sh/dispatch.sh | 7 ++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sh/dispatch-query.test.py b/sh/dispatch-query.test.py index 76474c49d0c..b4ea5bcba21 100644 --- a/sh/dispatch-query.test.py +++ b/sh/dispatch-query.test.py @@ -659,6 +659,17 @@ def test_terminating_just_before_a_phase_forks_its_command_stops_it(self): with self.assertRaises(ProcessLookupError): os.kill(int(pid.read_text()), 0) + def test_formatters_get_the_listed_files(self): + # A deleted tracked file is listed by git but skipped. + self.change("justfile") + self.change("demo/justfile") + self.git("add", "demo/justfile") + self.git("commit", "-qm", "demo") + (self.repo / "demo/justfile").unlink() + calls = self.run_ci() + self.assertIn("--fmt --check --justfile justfile", calls) + self.assertFalse(any("demo/justfile" in call for call in calls), calls) + def test_ci_runs_one_lint_preamble_and_one_test_pass(self): self.change("rs/kio/src/clock/mod.rs") calls = self.run_ci() diff --git a/sh/dispatch.sh b/sh/dispatch.sh index 3060f0fea58..62be3315398 100755 --- a/sh/dispatch.sh +++ b/sh/dispatch.sh @@ -318,13 +318,14 @@ fi # that lands just before the fork never reaches the command, so a cancel would # wait for the whole phase. A lane and its phase therefore fork every command # as a job: `wait` runs the trap at once, and Bash runs a trap left pending -# across the fork as soon as the fork returns. Async lists ignore INT and QUIT -# without job control; the job restores them for the command. +# across the fork as soon as the fork returns. Without job control, an async +# list ignores INT and QUIT, which the job restores for the command, and may +# read /dev/null instead of the caller's stdin, which it redirects explicitly. run() { { trap - INT QUIT exec "$@" - } & + } <&0 & wait "$!" } From c15dc50431916c6b65d1d1dc139b6874d84a6ca9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 16:00:03 -0800 Subject: [PATCH 4/4] test(ci): forced-window cancel test TERMs the lane's group, not the dispatcher Co-Authored-By: Claude Opus 5.5 --- sh/dispatch-query.test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sh/dispatch-query.test.py b/sh/dispatch-query.test.py index b4ea5bcba21..1edbd02e2bd 100644 --- a/sh/dispatch-query.test.py +++ b/sh/dispatch-query.test.py @@ -618,16 +618,18 @@ def test_terminating_the_run_stops_the_running_phase(self): def test_terminating_just_before_a_phase_forks_its_command_stops_it(self): # PS4 expands in the phase's shell after its last pending-trap check and - # before it forks a foreground command. The hook terminates the run, - # then blocks until the lane's group TERM kills it, so that TERM is - # pending when the shell would fork rs's lint. + # before it forks a foreground command. The hook sends the lane's + # process group the TERM a cancel delivers, then blocks until that TERM + # kills it, so the TERM is pending when the shell would fork rs's lint. + # Signalling the group rather than the dispatcher keeps the dispatcher's + # own lane-launch window out of this test. script = (self.repo / "sh/dispatch.sh").read_text().splitlines() line = next(n for n, text in enumerate(script, 1) if text.lstrip().startswith("check:rs)")) hook = self.root / "hook.sh" hook.write_text(f"""exec 9>/dev/null BASH_XTRACEFD=9 declare -A hook=([dispatch.sh:{line}]=1) -PS4='${{hook[${{BASH_SOURCE##*/}}:$LINENO]:+$(kill -TERM $$; exec sleep 600)}}' +PS4='${{hook[${{BASH_SOURCE##*/}}:$LINENO]:+$(kill -TERM 0; exec sleep 600)}}' set -x """) pid = self.root / "pid"