← 输入框|github.com|刷新
Skip to content

Fix two races behind the flaky pipeline job-control test - #1794

Open
tleonhardt wants to merge 2 commits into
mainfrom
fix/flaky-pipeline-job-control-test
Open

tleonhardt wants to merge 2 commits into
mainfrom
fix/flaky-pipeline-job-control-test

Conversation

@tleonhardt

@tleonhardt tleonhardt commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Fixes #1791.

The two failures in the issue had different causes. Both were races in the test, and both reproduce
locally only under CI-like load. Each commit fixes one.

1. [wrapper-stop-and-interrupt-busy-shell-producer]: Ctrl-Z sent to a stopped pipeline

The test sent Ctrl-Z as soon as the pipeline's group owned the terminal. On macOS the pipeline can
still be stopped at that point. A trace of a failing run showed this sequence:

  1. The pager prints PAGER_READY during the start-gate lend and blocks reading the terminal.
  2. The gate lend ends after 0.2s and cmd2 takes the terminal back. macOS wakes the blocked reader,
    finds it in the background, and sends SIGTTIN to the whole pipeline group, so the bash wrapper
    stops along with the pager.
  3. do_shell() lends the terminal again. The group is now the foreground but still stopped, and the
    watcher thread only continues it once it next runs. Under load that was over 200ms after the lend.
  4. In that gap busy printed BUSY_READY and the test sent Ctrl-Z. The bash wrapper and pager were
    already stopped and discarded the SIGTSTP. Only busy stopped.
  5. The watcher's SIGCONT then continued the whole group, including busy. The main thread's next poll
    of busy saw only continued, so nothing relayed a suspension. That SIGCONT is the
    PAGER_RESUMED in the CI transcripts.

Fix: before each Ctrl-Z, wait until the consumer and pager are running (no T state in ps), not
just until they own the terminal. A new running() helper shares a states() helper with
stopped().

2. [orphaned-job-control]: Ctrl-C handled only on the next key

The test timed out waiting for the first PAGER_INTERRUPT. The terminal had ISIG on, the foreground
was the pager's group, and the pager had nothing pending or blocked. The simulated pager handles
SIGINT in Python while blocking in os.read(). A signal that arrives after Python last checked for
one, but before the read() system call starts, is recorded without interrupting that read, so the
handler only runs once the next key arrives. The test sends Ctrl-C as soon as it sees PAGER_ALIVE,
which the pager writes just before it reads again.

To confirm this, I made each timeout send the stuck pager a plain x (no signal). In all 7 failures
that made its SIGINT handler run, so the Ctrl-C had been delivered and was waiting on that read.

Fix: the pager waits for keys in select() on the terminal and a signal.set_wakeup_fd() pipe.
Every handled signal makes the pipe readable, so select() returns even when the signal lands just
before it. The pager still reads the terminal itself, so it still stops with SIGTTIN when it reads
from the background.

Testing

Each race was stress-tested with coverage on (which also traces the cmd2 subprocess) and 32 to 64
copies of the test running at once. Each copy had its own COVERAGE_FILE and --basetemp. All
temporary instrumentation used during the investigation has been removed; cmd2/ is unchanged.

Variant Python Before After
wrapper-stop-and-interrupt-busy-shell-producer 3.15t 13/240 failed 1920/1920 passed
orphaned-job-control 3.15 2–7 per 1920 failed 5760/5760 passed
All 11 variants 3.15 and 3.15t 64/64 runs each passed

The "after" count for the first variant combines 960 runs before and 960 after the second fix. For
the first race, temporary sleeps that delayed the watcher's SIGCONT made the old test fail 16/16 with
the CI signature; the fixed test passed 16/16.

make check, make test (2838 passed, 3 skipped), and make docs-test pass locally. Coverage is
unchanged.

Notes

  • A narrow cmd2 window remains. A Ctrl-Z typed between a lend and the watcher continuing a
    pipeline stopped for terminal access is lost. For someone typing, that gap is normally well under a
    millisecond; it only grows to hundreds of milliseconds on a heavily loaded machine. Closing it in
    the library would mean continuing the pipeline as part of the lend, the way a shell's fg does,
    which needs a restructure of the watcher's loop. That's left for a possible follow-up.
  • Cleanup can mask a failure. When this test times out, cleanup can raise PermissionError from
    os.killpg(pipeline_group, SIGKILL), because the pager was just killed and is an unreaped zombie
    (macOS answers EPERM). The timeout is still shown first, but the remaining cleanup is skipped. I
    left this alone because it only affects failure reports.

test_pipeline_stops_with_cmd2_and_returns_terminal sent Ctrl-Z as soon as the pipeline owned the
terminal. On macOS the pipeline can still be stopped then: when cmd2 takes the terminal back between
lends, a pager blocked reading it gets SIGTTIN for its whole group, and the pipeline's watcher continues
the group only once it next runs after the following lend. A Ctrl-Z in that gap is discarded by the
processes already stopped, and the watcher's SIGCONT then undoes it for a shell producer, so cmd2 never
suspends and the test times out waiting for the outer shell to get the terminal back.

Wait until the consumer and pager are running before each Ctrl-Z.

Refs #1791
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.75%. Comparing base (fe7c2ad) to head (3a793c2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1794   +/-   ##
=======================================
  Coverage   99.75%   99.75%           
=======================================
  Files          23       23           
  Lines        6574     6574           
=======================================
  Hits         6558     6558           
  Misses         16       16           
Flag Coverage Δ
unittests 99.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@tleonhardt tleonhardt self-assigned this Oct 30, 2026
@tleonhardt tleonhardt added the bug label Oct 30, 2026
The orphaned-job-control variant intermittently timed out waiting for the first PAGER_INTERRUPT. The
simulated pager handles SIGINT in Python while blocking in os.read(). A signal that arrives after
Python last checked for one, but before the read() system call starts, is recorded without
interrupting that read, so the handler only runs once the next key arrives. The test sends Ctrl-C as
soon as it sees PAGER_ALIVE, which the pager writes just before it reads again.

Wait for keys in select() on the terminal and a signal.set_wakeup_fd() pipe instead. Every handled
signal makes the pipe readable, so select() returns even when the signal lands just before it.

Fixes #1791
@tleonhardt tleonhardt changed the title Wait for a running pipeline before Ctrl-Z in the job-control test Fix two races behind the flaky pipeline job-control test Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: pipeline job control on macOS free-threaded Python (wrapper-stop-and-interrupt-busy-shell-producer)

1 participant