Skip to content

Fix Windows page/pipe encoding bug and bug in how signals are handled when running a piped shell application - #1765

Open
tleonhardt wants to merge 21 commits into
mainfrom
pipeline-job-control
Open

tleonhardt wants to merge 21 commits into
mainfrom
pipeline-job-control

Conversation

@tleonhardt

@tleonhardt tleonhardt commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Summary

On POSIX, a command piped to an interactive program, such as help -v | less, now runs that program as the terminal's foreground job, as a shell pipeline does.

Prior to this on main, every pipe process starts in its own session and never receives the terminal. Ctrl-Z and fg therefore can't suspend and resume it together with cmd2, and it can't take the keyboard the way a shell's foreground job would.

This bug was discovered and a fix for it was created on the reserved_row_toolbar branch (#1761). It is extracted here, without the toolbar, so it can be reviewed and released on its own since it is a bug and is unrelated to the work on that branch.

This PR also fixes a Windows piping bug that was introduced when we switched to all utf-8 and discovered during manual regression testing.

What changes

  • Terminal pipelines. When a pipe's stdout is cmd2's controlling terminal and the pipe starts on the main thread, the pipe process gets its own process group. cmd2 lends it the terminal:

    • as it starts, so a pager can set its terminal modes. The pipeline is held in a small /bin/sh gate (read -r _ || exit 1; exec "$SHELL" -c <command>) until cmd2 has made its group the foreground one and sent a newline down its stdin pipe. So the pager never runs before it owns the terminal, however late cmd2 gets to hand it over.
    • while a cmd2 write to the pipe would block. What the pipe takes at once goes straight through, so a pager gets the terminal once it stops reading to wait for a key, and ordinary writes cost no terminal handoff.
    • while a subprocess that writes to the pipe itself is blocked on it (see below)
    • while cmd2 waits for it to exit

    Otherwise the terminal stays with cmd2, so command code can still read the keyboard with read_input(), select(), input(), getpass or raw reads.

  • Job control. ProcReader watches the pipeline's group on a thread and relays its Ctrl-Z stops to cmd2's own job, so the shell suspends and resumes both together.

    • Ctrl-Z that reaches cmd2 directly, while cmd2 holds the terminal, stops and resumes the pipeline too.
    • Whichever process of the job reports a stop first (the consumer, a shell producer, or cmd2 itself) suspends the whole job, once. A consumer that ignores Ctrl-Z therefore cannot leave a stopped producer behind.
    • When cmd2 is a session leader, with no shell to resume it, no part of its pipeline's job stops on Ctrl-Z. That includes a shell producer that joins it.
    • The watcher always records the pipeline's exit, even if the application ignores SIGCHLD or the terminal hangs up. Signals to a group that cmd2 may no longer signal (a zombie on macOS, or a setuid program such as sudo) are tolerated.
  • Ctrl-C. Ctrl-C is forwarded to the pipeline without signalling cmd2's group a second time. Once the pipeline's leader has exited and been reaped, it reaches a shell producer that joined the group, and otherwise nothing, since the leader's ID may have been reused. A pipe write that finds the consumer ended by Ctrl-C raises KeyboardInterrupt, except in code under sigint_protection, which gets BrokenPipeError.

  • Subprocess producers. A subprocess given self.stdout, such as subprocess.run(..., stdout=self.stdout) in a custom command or a ! command in run_script s.txt | less, writes to the pipe's descriptor itself, bypassing cmd2's writes. The pipe writer's fileno() hands such producers the write end of a relay pipe. A thread passes their output on and lends the consumer the terminal while a producer is blocked on the relay's full pipe. Otherwise, command code keeps the terminal. cmd2's own writes wait for pending relay output, which keeps output in order, and a consumer's exit still reaches producers as EPIPE or SIGPIPE.

  • shell producers. A shell command piped to such a program (shell git log | less) joins the pipeline's job for as long as it runs. If the pipeline exits first, the command runs in cmd2's group as before.

  • Unchanged cases. These still run in their own session:

    • pipes started off the main thread
    • pipes whose stdout is not the terminal, such as a pipe nested in a command whose own output is piped (run_script s.txt | less, where the script runs big | cat), or one whose output cmd2 captures in a pyscript

    Windows is unchanged.

Testing

  • New tests/test_pipeline_job_control.py runs cmd2 under an interactive bash on a pseudo-terminal and reads the screen with pyte. It covers:
    • Ctrl-C, Ctrl-Z and fg
    • wrapper shells and orphaned sessions
    • nested prompts
    • pagers setting modes at startup, including when cmd2 is delayed after starting the pipeline
    • shell producers
    • producers that write to the pipe's descriptor directly: a subprocess, a script's shell command, and a nested pipe
    • Ctrl-Z between pipe writes, an application that ignores SIGCHLD, and Ctrl-C ending the pager during protected code
  • On main's code, the Ctrl-Z/fg and interrupt cases fail. In the Ctrl-Z case, ps shows the pager in its own session while the terminal's foreground group stays cmd2's.
  • Unit tests for ProcReader, the pipe writer and the descriptor relay were added to tests/test_utils.py, and redirection edge cases to tests/test_cmd2.py.
  • pyte is added to the dev and test dependency groups; it was already in uv.lock. Coverage now follows the cmd2 applications these tests start (patch = ["subprocess"]). --maxschedchunk=1 spreads the slow terminal tests across workers.
  • Piping 20,000 lines to cat takes about as long as on main (0.57s against 0.54s).
  • make check, make test and make docs-test pass locally on macOS.
  • The job-control internals (the pipe writer, ProcReader's terminal methods) are private, so they are not published in the API docs.

Notes

  • The changes are all on POSIX paths, so Linux and macOS CI are the checks that matter here.
  • The changelog now targets 4.3.0 rather than 4.2.5, as the changes have grown beyond a patch release.
  • Known limitation: a pager that sets its terminal modes only after cmd2's 0.2s startup check, such as help | { sleep 0.5; less; }, can be left on a cooked terminal. main misbehaves the same way here, so this is not a regression.
  • The PR also includes a cherry-pick of f3e8009 from the toolbar branch. It gives the corrupt-history tests their own temp files instead of a fixed /tmp/doesntmatter. With --maxschedchunk=1 spreading tests across workers, those tests raced on Linux and failed on Windows, where that directory does not exist.

A pipe process always ran in its own session, so it never received the
terminal. Piped to an interactive program such as less, Ctrl-Z and fg
did not suspend and resume it together with cmd2, and the program could
not take the keyboard as a shell's foreground job would.

When a pipe's output goes to cmd2's controlling terminal and the pipe is
started on the main thread, run the pipe process in a new process group
and lend it the terminal as it starts, while cmd2 writes to it, and while
cmd2 waits for it. Between writes the terminal returns to cmd2, so
command code can still read the keyboard. ProcReader watches the group on
a thread, relays its job-control stops to cmd2's job, and forwards Ctrl-C
without signalling cmd2's own group again. A shell command piped to such
a program joins the pipeline's job for as long as it runs. Pipes started
off the main thread, or whose output cmd2 captures, still run in their
own session.

The terminal tests drive cmd2 under an interactive bash on a
pseudo-terminal, using pyte to read the screen, and coverage now follows
the applications those tests start.

Extracted from the reserved-row toolbar work (PR #1761) without the
toolbar.
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.75%. Comparing base (9f882d7) to head (0509109).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1765      +/-   ##
==========================================
+ Coverage   99.66%   99.75%   +0.08%     
==========================================
  Files          23       23              
  Lines        5974     6431     +457     
==========================================
+ Hits         5954     6415     +461     
+ Misses         20       16       -4     
Flag Coverage Δ
unittests 99.75% <100.00%> (+0.08%) ⬆️

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.

test_history_file_bad_compression and test_history_file_bad_json wrote to
a fixed /tmp/doesntmatter, which on the Windows runner is D:\tmp and does
not exist on a fresh machine. They passed only when the preceding
permission-error test had already run: mocking builtins.open there does not
stop the history setup from creating the file's parent directory, so that
test created D:\tmp as a side effect. Under xdist the order is not fixed,
and adding tests elsewhere shifted the schedule so both writers reached a
worker first and failed on every Windows job.

All three tests now use tmp_path, so none depends on a directory existing
or on another test having run. Each passes alone on a single worker.

Validation: 2587 passed, 6 skipped with coverage; make check, make test,
make docs-test and git diff --check passed.
@tleonhardt tleonhardt self-assigned this Sep 26, 2026
On Ubuntu 3.11 CI the outer shell once printed the old size after the
test resized the pseudo-terminal while the job was stopped. Record each
resize with the size read back straight after it, and include the size
at the timeout, to tell a resize that never took effect from one undone
later.
@tleonhardt tleonhardt added the bug label Sep 27, 2026
@tleonhardt tleonhardt changed the title Run POSIX pipes to interactive programs as the terminal's foreground job Fix bug in how signals are handled when running a piped shell application Sep 27, 2026
@tleonhardt

tleonhardt commented Sep 27, 2026 •

Copy link
Copy Markdown
Member Author

Manual testing on both main and this branch on MacOS validated that the ability to Ctrl-Z and then fg from a piped shell application such as less was previously broken and is now fixed.

I'll make sure to also do manual testing on both Linux and Windows before merging.

I'm also going to spend more time looking at various potential edge cases.

@tleonhardt
tleonhardt marked this pull request as draft September 27, 2026 13:55
Three bugs in the terminal-pipeline job control, found in review and
reproduced on a pseudo-terminal with real less:

1. Output written straight to the pipe's descriptor hung the pager.

   cmd2 lent the pager the terminal only during its own writes through
   PipelineWriter. A subprocess given self.stdout (for example
   subprocess.run(..., stdout=self.stdout) in a custom command, or a `!`
   command run by `run_script s.txt | less`) writes to the descriptor
   itself, so no lend happened. Once the pipe filled, less stopped with
   SIGTTIN reading the keyboard, the watcher waited for a lend that never
   came, and the producer blocked on the full pipe forever. On main these
   cases worked, because the pager ran in its own session.

   PipelineWriter.fileno() now hands out the write end of a relay pipe.
   A thread passes that output on to the consumer and lends it the
   terminal while a producer is blocked on the relay's full pipe, which
   is the only time the consumer needs the terminal to make progress.
   Once no producer is waiting, command code gets the terminal back, as
   between cmd2's own writes, so a later input() still works. cmd2's own
   writes first wait for pending relay output, which keeps output in
   order. When the consumer exits, the relay closes its pipe, so
   producers still get EPIPE or SIGPIPE.

2. A pipe nested in a piped command took the terminal from the outer
   pager.

   A pipeline counted as the terminal's job when either its stdout or its
   stderr was the foreground terminal. In `run_script s.txt | less` with
   `big | cat` in the script, the inner pipeline's stdout is the outer
   pipe, but its stderr is the terminal. It became a terminal job and its
   lends went to cat's group, so less was never lent the terminal, and
   the command hung as in (1).

   Only a pipeline whose stdout is the foreground terminal is now the
   terminal's job. Others, including nested pipelines and pipelines whose
   stdout cmd2 captures, run in their own session, as on main.

3. The pager could start before it owned the terminal.

   The consumer could run between Popen() and cmd2's first terminal
   lend. A pager such as less sets its terminal modes as it starts, and a
   background tcsetattr() stops it with SIGTTOU. On macOS the call then
   fails with EINTR once the process continues, and less carries on with
   a cooked terminal, so q needs Enter and keys are echoed. This was
   reproduced by delaying cmd2 after it starts the pipeline.

   A terminal pipeline now starts as
   `/bin/sh -c 'read -r _ || exit 1; exec "$SHELL" -c <command>'`. cmd2
   sends a single newline down the consumer's stdin pipe only after
   making the pipeline's group the foreground one. From a pipe, the read
   builtin takes no more than that line, so no extra descriptor is needed.
   That matters because dash, /bin/sh on Debian and Ubuntu, cannot
   redirect descriptors above 9. Checked with sh, bash, dash and zsh.
   If cmd2 fails before sending the newline, it closes the pipe, and the
   held pipeline reads EOF and exits rather than waiting forever.

Tests:
- A PTY test covering the three hang cases in (1) and (2): a subprocess
  writer, a script's shell command, and a nested pipe. It fails without
  this change.
- The pager-startup PTY test gains a variant that delays cmd2 after it
  starts the pipeline, which fails without this change.
- Unit tests for the relay: lending only while a producer waits, giving
  the terminal back whether or not output is pending, ordering with
  cmd2's own writes, and passing the consumer's exit on to producers.
Fixes for the remaining review findings on terminal pipelines. Each bug
was reproduced on a pseudo-terminal before being fixed, except where
noted.

- The pipeline's watcher thread could die, leaving the pipeline without
  a return code, so cmd2 treated it as still running.
  - With SIGCHLD ignored, the system reaps the pipeline itself and
    waitpid() fails with ECHILD. The watcher died with a traceback.
  - On macOS, signaling a group whose only member is a zombie fails
    with EPERM, not ESRCH. That happens when a suspended pager is killed
    before fg (2 of 6 runs failed). A setuid consumer such as sudo gives
    the same error.
  Every signal to the pipeline's group now goes through one helper that
  tolerates both errors. The watcher records a return code in all cases:
  0 when the system reaped the pipeline, as Popen.wait() reports, and a
  plain reap if job control fails, for instance after a hangup.
  Handing the terminal to a group that has just vanished also tolerates
  Linux's EPERM.

- Ctrl-Z while cmd2 owned the terminal, between writes to its pipe,
  stopped only cmd2. The consumer ran on in the background, writing
  into the shell session. cmd2 now stops the pipeline along with itself
  and continues it on fg, as a shell stops its whole job. The watcher
  counts these stops, so it does not relay them back to cmd2 as a second
  suspension.

- A pipe write inside sigint_protection raised KeyboardInterrupt once
  Ctrl-C had ended the consumer. The writer now asks whether cmd2's
  SIGINT handler would interrupt at that point. Protected code, including
  redirection cleanup, gets BrokenPipeError. This replaces the separate
  finish_producer() flag.

- Every pipe write handed the terminal to the consumer and back. Piping
  20,000 lines to cat took about 1.0s, against 0.54s on main. Writes the
  pipe accepts at once now go straight through. The terminal is lent only
  once a write would block, since only then does the consumer need it.
  Output a producer left in the descriptor relay is still passed on
  first, under a lend. The same test now takes 0.57s.

- Once the pipeline's leader had been reaped, Ctrl-C fell back to
  signaling its process ID as a group. After the group is gone, the
  system may reuse that ID for an unrelated process. The pipeline now
  tracks the shell producers that joined its group, and finds the group
  through one that cmd2 has not reaped yet. Otherwise it signals nothing.
  (Confirmed from the code, not reproduced: pid reuse cannot be forced.)

Cleanups:
- The watcher and the producer-stop relay shared a copy of the suspend
  sequence; it is now one helper, _suspend_with_cmd2().
- New internals are private, so the API docs do not publish them:
  _PipelineWriter, _unblocked_sigttou, and ProcReader's _terminal_group,
  _manage_terminal, _lend_terminal and _wait_for_exit.

Tests:
- PTY tests: Ctrl-Z between pipe writes stops and resumes the whole
  pipeline exactly once; an application that ignores SIGCHLD gets no
  watcher traceback; Ctrl-C ending the pager gives protected code
  BrokenPipeError. Each fails without its fix.
- Unit tests: the watcher records an exit after ECHILD or a hangup,
  with ESRCH or EPERM from killpg; it skips the stops cmd2 sent; a
  direct Ctrl-Z signals the pipeline and a relayed one does not;
  send_sigint signals nothing once the pipeline and its producers are
  gone, and tolerates EPERM; the writer does not interrupt protected
  code.
The parameters of test_proc_reader_watcher_always_records_an_exit used
signal.SIGTSTP, which Windows lacks. Parameters are evaluated when
pytest collects the module, before the test's skipif applies, so the
whole of tests/test_utils.py failed to collect on Windows. The
parameters now name the stop, and the test builds its status.
Adding test_proc_reader_sigint_to_a_group_cmd2_may_not_signal above it
took its skipif, so the test patched os.getpgid on Windows, which has
no such function. Both tests now carry the skip.
Two intermittent failures of test_pipeline_stops_with_cmd2_and_returns_terminal
under parallel load, neither caused by cmd2:

- Interactive bash intermittently stops a job it has just started on a
  TOSTOP terminal, before the job runs any code of its own. The test set
  TOSTOP before starting bash in its exit_sigint cases, so the launch of
  the application, or of printf or stty during a stop, was sometimes
  reported Stopped. Reproduced without cmd2: across 14 parallel bash
  sessions launching Python, 10 of 2,240 launches stopped with TOSTOP set
  and none of 2,240 without it. The test now sets TOSTOP only once the
  pager owns the terminal, and again after fg: while the job is stopped,
  bash restores its own terminal modes, so its commands run without it.
  TOSTOP is still in effect when Ctrl-C is sent, which is what the test
  needs it for.

- The pseudo-terminal occasionally reported its old size after the test
  resized it while the job was stopped, though every process of the job
  was stopped and nothing else sets a size. The test now confirms the new
  size through the shell and resizes again if the shell saw the old one,
  reporting every attempt if it never takes.
- A Ctrl-Z that reaches cmd2 directly while the pipeline's group is
  already gone: cmd2 neither counts the stop nor continues the group
  later, and still suspends itself.
- A descriptor relay whose producers have all closed its pipe finishes
  and closes the consumer's pipe, and then has nothing pending: flush()
  returns at once and idle() is true.
Fixes from further review of the terminal pipeline job control. Each was
reproduced before being fixed, except where noted.

- A `shell` producer could hang on Ctrl-Z when cmd2 is a session leader
  (started with exec, so there is no outer shell). The pipeline's
  consumer inherited an ignored SIGTSTP, but the producer that joins its
  job did not. Ctrl-Z during `shell sleep 3 | cat` stopped the producer
  for good while cat ran on, and cmd2 waited on the producer forever.
  Both spawns now share one helper, _session_leader_job_stops(), that
  applies the session leader's Ctrl-Z behavior.

- A producer's stop was dropped when the consumer ignores Ctrl-Z. The
  producer's wait relayed its stop only once the consumer's watcher had
  exited, assuming the consumer would stop too. With a consumer that
  ignores SIGTSTP, the producer stayed stopped and cmd2 hung.

- A direct Ctrl-Z to cmd2 counted the SIGSTOP it sent the pipeline, so
  that the watcher could skip the resulting stop report. A consumer that
  was already stopped, for instance waiting for the terminal, produces no
  report, and the count never came back down. A later genuine stop of the
  consumer would then have been ignored.

  The last two are fixed together. Suspensions of the job now take one
  lock, and count as they finish. Whichever stop is reported first, the
  consumer's, a producer's, or cmd2's own, suspends the whole job. A
  suspension ends by continuing the whole pipeline, so a stop reported
  before one finished needs nothing more and is skipped. This replaces
  the count of cmd2's own stops.

- send_sigint() and terminate() used the pipeline's process ID even after
  its watcher had reaped it, when the system may already have given that
  ID to an unrelated process. Both now check the return code first.
  (Confirmed from the code; pid reuse cannot be forced in a test.)

- The descriptor relay could report output as passed on while it was
  still on its way: bytes read from the relay's pipe were neither in the
  pipe nor counted until the relay thread took its lock again. cmd2's
  next write could then overtake the end of a subprocess's output. The
  relay now waits for output outside the lock, then reads and counts it
  under the lock.

The next release is 4.3.0 rather than 4.2.5: the changelog heading is
updated, as these changes have grown beyond a patch release.

Tests:
- PTY test: Ctrl-Z during `shell <producer> | <consumer>`, run from a
  shell or as a session leader, with a consumer that stops or one that
  ignores Ctrl-Z. Three of the four cases hung before this change.
- Unit tests: a producer's stop suspends the job while the watcher runs,
  unless a finished suspension already resolved it; the watcher skips such
  stops and follows newer ones while it waits for the terminal; a
  suspension waits for one in progress; a direct Ctrl-Z while another
  thread relays a stop leaves the pipeline to that thread; a reaped
  pipeline is never signaled; the relay counts output it is reading.
mypy checks the platform it runs on. On Windows, it rejected the POSIX-only
terminal job control in cmd2.utils with 69 attr-defined errors (os.tcsetpgrp,
signal.SIGTSTP, select.poll and the like), and narrowed terminal_fd in
_redirect_output() to None, reporting the pipeline code as unreachable. CI
runs mypy on Ubuntu only, so it never saw either.

Guarding each POSIX-only function does not help: mypy reports the code after
an `if sys.platform == "win32": raise` guard as unreachable, and ruff forbids
the assert it does accept. Pin mypy's platform to Linux instead, so a Windows
run matches CI. Windows-only branches go unchecked by mypy, as they already
did in CI; ty still checks them.

Also annotate terminal_fd, whose type should not depend on narrowing.
`help -v | more` showed every box-drawing character as mojibake, such as
ΓöÇ for ─. Since 4.2.4, pipes are written as UTF-8, but Windows console
programs such as more, sort and findstr decode piped input with the
console's output code page, which is 437 or 850 on most systems.

On Windows, pipes now use the console's output code page. It cannot
represent everything, an emoji for instance, and 4.2.4 moved to UTF-8
because encoding failures aborted the command. So replace what the code
page lacks instead of failing. Code page 65001 (chcp 65001) is reported
as utf-8. Without a console, and on other platforms, pipes still use
UTF-8. The POSIX terminal pipeline's writer uses the same encoding and
error handling.
test_descriptor_relay_counts_output_it_is_reading paused the relay after
it had taken output out of its pipe, asked idle() from another thread,
and released the relay after 0.3s whether or not that thread had asked
yet. On the slower macOS runners (Python 3.11 to 3.13), the thread
sometimes asked only after the release. By then the relay had passed the
output on, so idle() rightly answered True, and the test failed.

The asking thread now reports that it is about to ask, and the test
checks only the answer given while the read is paused: none yet, because
idle() waits for the relay, or False. It still fails every time against
the relay that counted output only after reading it.
main is the branch for every release, whether patch, minor or major.
Feature branches merge into it, and releases are tagged and published
from it. CLAUDE.md said main held only the next patch release.
…hells

Fixes from another review of the pipeline changes:

- A cmd2 write could hang when a subprocess wrote through the descriptor
  relay at the same time. The write fast path checked the consumer's pipe
  for room, then wrote without a terminal lend. The relay thread writes
  to the same pipe, and could fill it in between. The write then blocked
  with no lend, while the pager, waiting for a key, stayed stopped. Once
  a relay exists, every write is now made under a lend. Without one,
  writes the pipe has room for still need none. DescriptorRelay.idle()
  had no other caller, and is gone.

- The start gate ran /bin/sh, which Android does not have, so every
  terminal pipe there failed to start. It now uses the POSIX shell that
  Popen(shell=True) itself uses: /system/bin/sh on Android.

- On Windows before Python 3.14, a console code page Python has no
  cpNNNNN codec for fell back to UTF-8, although Python knows many of
  them by other names. Pipes now try those names too: the ISO-8859 family,
  KOI8-R and KOI8-U, US-ASCII, GB18030, EUC-JP and EUC-KR. Python 3.14
  has a codec for every code page Windows supports.

- A shell command run from a worker thread joined the main thread's
  terminal pipeline. Joining relays job-control stops to the main thread,
  and may change signal handlers, which only the main thread may do: as a
  session leader, it raised ValueError. Only the main thread's shell
  commands join now.

Tests cover each, and each fails without its fix: every write is lent
once a relay exists; the gate uses Android's shell; code pages map to
their codecs on any platform; a worker thread's shell command stays out
of the pipeline. The relay tests that used idle() now use flush().
From Python 3.14, Windows has a codec for every code page Windows accepts.
That includes pseudo code pages such as 1 (CP_OEMCP), which the test used
as a code page without a codec, so it failed there. The test already
skipped code page 50220 on those versions for the same reason.
@tleonhardt
tleonhardt marked this pull request as ready for review September 27, 2026 18:41
@tleonhardt
tleonhardt requested a review from bambu September 27, 2026 18:42
Fixes from another review of the pipeline changes:

- A pipe write that found the consumer ended by Ctrl-C raised
  KeyboardInterrupt on whatever thread was writing. Ctrl-C interrupts
  only the main thread, so a worker thread printing to a piped command
  died with a traceback, where it had quietly got BrokenPipeError before.
  Only the main thread is cancelled now.

- A Ctrl-Z that reached cmd2 directly sent SIGSTOP and SIGCONT to the
  consumer's process ID even after its watcher had reaped it, when the
  system may already have given that ID to an unrelated process group.
  The pipeline's group is now signaled through the consumer only until it
  is reaped, and then through a shell producer that joined the group and
  is still running, or not at all. send_sigint() shares that lookup,
  _joined_group().

- Taking the terminal back at the end of a lend raised OSError once the
  terminal had hung up, replacing the SystemExit that SIGHUP raises. cmd2
  then carried on over a dead terminal instead of exiting. The handoff
  now ignores the failure, as it does not matter once the terminal is gone.

- The Ctrl-Z handler's terminal handoffs could raise from a signal
  handler into whatever the main thread was doing, once the terminal or
  the pipeline's group was gone. They now ignore that failure too.

- ppaged() still encoded its output as UTF-8, which Windows' default
  pager, more, shows as mojibake, just as it did for pipes. It now uses
  the console's code page and replaces what the code page lacks.

Each has a test that fails without its fix.
@tleonhardt tleonhardt changed the title Fix bug in how signals are handled when running a piped shell application Fix Windows page/pipe encoding bug and bug in how signals are handled when running a piped shell application Sep 27, 2026
- A shell command that joins a terminal pipeline now writes to the
  consumer's pipe itself instead of through the descriptor relay. It
  holds the terminal lent for as long as it runs, so it never needed the
  relay's copying thread. Creating the relay also sent every later cmd2
  write down the slower lend path. Output already written to the relay,
  and cmd2's own buffered output, reach the consumer first.

  A command now joins only when self.stdout is that pipeline's writer. One
  whose output a command redirected elsewhere, such as to a file, has no
  reason to share the consumer's terminal and job.

- A pipeline's watcher can outlive job control, when waiting for the
  pipeline fails. Job control then restored the previous SIGTSTP handler,
  and a stop the watcher relayed later would stop cmd2 with nothing to
  resume it or the pipeline. Ending job control now marks the pipeline
  detached under the suspension lock, after any suspension in progress,
  and a detached watcher relays nothing.

- Cleanup: one helper, _sigttou_mask(), now blocks or unblocks SIGTTOU
  for the calling thread wherever that was done by hand. ppaged() takes
  the terminal back with the same thread-local block rather than
  ignoring SIGTTOU for the whole process, which was not thread-safe.
  _redirect_output() no longer closes the pipe's read end twice, and
  do_shell() tracks whether its command joined in one variable.
@tleonhardt

tleonhardt commented Sep 27, 2026 •

Copy link
Copy Markdown
Member Author

I manually tested on Mac, Linux, and Windows first on main to verify the bugs were present and then on this branch to verify they are now fixed. Typically I tested like so:

  1. uv run examples/getting_started.py
  2. help -v | less (POSIX) or help -v | more (Windows)
  3. Use / to search for a command name and verify it gets highlighted
  4. Use Ctrl+Z to suspend
  5. Run a shell command
  6. Resume using fg
  7. Verify application resumed in same state
  8. Hit q to quit and verify back in the cmd2 app
  9. Run another command to make sure everything is good

NOTE: Steps 3 through 7 were only run on POSIX oses.

This is ready for review whenever anyone has time.

@kmvanbrunt @bambu

The descriptor relay lent the terminal to the consumer whenever its own
pipe was full, taken as a sign that a producer was blocked on the
consumer. A producer that has just finished can leave that pipe full,
though. A command that ran subprocess.run(..., stdout=self.stdout) into
a slow consumer then resumed while the relay still held the lend,
passing the rest of the output on. The command's next terminal read came
from the background: it stopped cmd2 with SIGTTIN, or failed in an
orphaned session. Reproduced with a 256 KiB producer and a consumer
reading 16 KiB every 150 ms, whose input() failed on every run.

The relay now lends only to a consumer that is stopped, waiting for the
terminal to read the keyboard or set its modes, which the pipeline's
watcher now records. A consumer that never touches the terminal, however
slowly it reads, is never lent it. The relay also hands the terminal
back as soon as no producer is waiting, checking on every pass rather
than only when a write stalls. That check replaces the two narrower
release paths it had.

Tests: the reviewer's scenario through a real terminal, launched from a
shell and as a session leader; a relay that does not lend to a slow
consumer that never reads the keyboard; and the watcher marking the
consumer waiting only while it is stopped for the terminal. Each fails
without this change.
The pipeline's watcher read the count of finished suspensions before its
blocking waitpid, to recognize a stop that a suspension had already dealt
with. A whole suspension could finish during that wait, such as a direct
Ctrl-Z to cmd2 followed by fg. Its SIGCONT discards a stop the wait has
not collected yet, so the wait went on and kept the old count. The next
genuine Ctrl-Z then looked already dealt with and was skipped, leaving
the consumer stopped and cmd2 waiting for it indefinitely. Reproduced
with a real child by delaying the watcher's first wait until after a
stop and continue, then stopping the child again.

The watcher's wait now also reports continues (WCONTINUED). A suspension
always ends by continuing the pipeline, and the continue is reported even
when it discarded a stop, so the watcher reads the count afresh after
every change in the process's state. The wait for a shell producer that
joined the pipeline does the same, and takes a continue for neither a
stop nor an exit.

Tests: the reproduction above, and the producer's wait handling a
continue. Both fail without this change.
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.

1 participant