Skip to content

Commit 9be7a4d

Browse files
committed
Block SIGTTOU only while lending the terminal to a pipeline
SIGTTOU was blocked on the main thread for the whole life of a terminal pipeline. A signal mask survives fork and exec, so every child started meanwhile, whether a shell producer or a subprocess run by command code, kept SIGTTOU blocked for life and could change terminal modes from the background. The consumer now owns the terminal only during a lend, so block SIGTTOU for the lend alone, and spawn a shell producer, which starts inside one, with it unblocked.
1 parent a614316 commit 9be7a4d

4 files changed

Lines changed: 131 additions & 32 deletions

File tree

‎cmd2/cmd2.py‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3621,13 +3621,6 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState:
36213621
# exit must unblock a producer writing to a full pipe immediately.
36223622
subproc_stdin.close()
36233623
if terminal_fd is not None:
3624-
import signal
3625-
3626-
# The producer may still write diagnostics while the consumer owns
3627-
# the terminal. Block SIGTTOU after Popen so the child retains normal
3628-
# job-control behavior, and restore our mask with the handoff.
3629-
previous_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGTTOU})
3630-
terminal_stack.callback(signal.pthread_sigmask, signal.SIG_SETMASK, previous_mask)
36313624
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr, terminal_fd=terminal_fd)
36323625
terminal_stack.enter_context(cmd_pipe_proc_reader.manage_terminal())
36333626

@@ -5304,14 +5297,16 @@ def do_shell(self, args: argparse.Namespace) -> None:
53045297
terminal_stack.enter_context(pipeline.lend_terminal())
53055298
while True:
53065299
try:
5307-
# For any stream that is a StdSim, we will use a pipe so we can capture its output
5308-
proc = subprocess.Popen( # noqa: S602
5309-
expanded_command,
5310-
stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout, # type: ignore[unreachable]
5311-
stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,
5312-
shell=True,
5313-
**kwargs,
5314-
)
5300+
# For any stream that is a StdSim, we will use a pipe so we can capture its output.
5301+
# A command joining the pipeline is spawned inside the lend, which blocks SIGTTOU.
5302+
with utils.unblocked_sigttou() if "process_group" in kwargs else contextlib.nullcontext():
5303+
proc = subprocess.Popen( # noqa: S602
5304+
expanded_command,
5305+
stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout, # type: ignore[unreachable]
5306+
stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,
5307+
shell=True,
5308+
**kwargs,
5309+
)
53155310
break
53165311
except PermissionError:
53175312
# The pipeline exited before the command could join its group.

‎cmd2/utils.py‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,22 @@ def write(self, b: bytes) -> None:
536536
self.std_sim_instance.flush()
537537

538538

539+
@contextlib.contextmanager
540+
def unblocked_sigttou() -> Iterator[None]:
541+
"""Let a child started inside :meth:`ProcReader.lend_terminal` keep normal job control.
542+
543+
The lend blocks SIGTTOU for its thread, and a child inherits that mask for life. Spawning
544+
touches no terminal, so unblocking it for the spawn alone cannot stop this thread.
545+
"""
546+
import signal
547+
548+
previous_mask = signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGTTOU})
549+
try:
550+
yield
551+
finally:
552+
signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask)
553+
554+
539555
class ProcReader:
540556
"""Used to capture stdout and stderr from a Popen process if any of those were set to subprocess.PIPE.
541557
@@ -685,25 +701,35 @@ def lend_terminal(self) -> Iterator[None]:
685701
reads through input(), getpass(), or third-party libraries. Lending during
686702
writes lets an interactive consumer drain a full pipe without deadlocking.
687703
"""
704+
import signal
705+
688706
terminal_fd = self._terminal_fd
689707
if terminal_fd is None or self._proc.returncode is not None:
690708
yield
691709
return
692-
with self._terminal_lock:
693-
try:
694-
self._set_foreground_group(terminal_fd, self._proc.pid)
695-
except OSError as error:
696-
# The group can disappear before the watcher has reaped its leader.
697-
if error.errno not in (errno.ESRCH, errno.EINVAL):
698-
raise
699-
self._terminal_available.set()
710+
# While the consumer owns the terminal, a signal handler run on this thread may still
711+
# write diagnostics to it. Block SIGTTOU for the lend only: a signal mask survives fork
712+
# and exec, so blocking it for the whole pipeline would leak into every child the
713+
# command starts. A child started during a lend must unblock it; see unblocked_sigttou().
714+
previous_mask = signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGTTOU})
700715
try:
701-
yield
702-
finally:
703716
with self._terminal_lock:
704-
self._terminal_available.clear()
705-
if os.tcgetpgrp(terminal_fd) == self._proc.pid:
706-
self._set_foreground_group(terminal_fd, self._original_group)
717+
try:
718+
self._set_foreground_group(terminal_fd, self._proc.pid)
719+
except OSError as error:
720+
# The group can disappear before the watcher has reaped its leader.
721+
if error.errno not in (errno.ESRCH, errno.EINVAL):
722+
raise
723+
self._terminal_available.set()
724+
try:
725+
yield
726+
finally:
727+
with self._terminal_lock:
728+
self._terminal_available.clear()
729+
if os.tcgetpgrp(terminal_fd) == self._proc.pid:
730+
self._set_foreground_group(terminal_fd, self._original_group)
731+
finally:
732+
signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask)
707733

708734
def _wait_for_job(self, terminal_fd: int) -> None:
709735
"""Reap a foreground pipeline and relay its stops to the outer shell's job.

‎tests/test_cmd2.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -963,10 +963,9 @@ def start_pipe(*args, **kwargs):
963963
reader.wait_for_exit.assert_called_once_with(0.2)
964964
reader.wait.assert_called_once_with()
965965
process.wait.assert_not_called()
966-
assert sigmask.call_args_list == [
967-
mock.call(signal.SIG_BLOCK, {signal.SIGTTOU}),
968-
mock.call(signal.SIG_SETMASK, set()),
969-
]
966+
# SIGTTOU is blocked only inside ProcReader's lends. Blocking it for the whole
967+
# pipeline would leak the mask into every child the command starts.
968+
sigmask.assert_not_called()
970969
else:
971970
process.wait.assert_called_once()
972971
assert popen.call_args.kwargs["stdin"].closed

‎tests/test_pipeline_job_control.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,85 @@ def wait_until(predicate):
642642
process.wait(timeout=5)
643643

644644

645+
@pytest.mark.parametrize("producer", ["command", "shell"])
646+
def test_pipeline_children_inherit_an_ordinary_signal_mask(tmp_path, producer) -> None:
647+
"""Processes started during a terminal pipeline must not inherit a blocked SIGTTOU.
648+
649+
cmd2 blocks SIGTTOU for itself while it lends the terminal. A signal mask survives fork
650+
and exec, so a child spawned with it blocked -- a shell producer, or a subprocess run by
651+
command code -- would keep it for life, and change terminal modes from the background
652+
where it should be stopped.
653+
"""
654+
import pty
655+
656+
shell = shutil.which("bash")
657+
if shell is None:
658+
pytest.skip("requires an interactive bash shell")
659+
probe = tmp_path / "probe.py"
660+
outcome = tmp_path / "outcome"
661+
probe.write_text(
662+
"import pathlib, signal\n"
663+
"blocked = signal.SIGTTOU in signal.pthread_sigmask(signal.SIG_BLOCK, [])\n"
664+
f"pathlib.Path({str(outcome)!r}).write_text(repr(blocked))\n",
665+
encoding="utf-8",
666+
)
667+
application = tmp_path / "application.py"
668+
application.write_text(
669+
"import subprocess, sys\n"
670+
"from cmd2 import Cmd\n"
671+
"class App(Cmd):\n"
672+
" def do_probe(self, _):\n"
673+
" self.poutput('probing')\n"
674+
f" subprocess.run([sys.executable, {str(probe)!r}], check=True)\n"
675+
"app = App()\n"
676+
"app.prompt = 'TEST> '\n"
677+
"app.cmdloop()\n",
678+
encoding="utf-8",
679+
)
680+
master, slave = pty.openpty()
681+
bootstrap = (
682+
"import os, fcntl, termios; os.setsid(); "
683+
"fcntl.ioctl(0, termios.TIOCSCTTY, 0); "
684+
"os.execv(os.environ['TEST_SHELL'], ['bash', '--noprofile', '--norc', '-i'])"
685+
)
686+
env = dict(os.environ, TERM="xterm-256color", PS1="OUTER> ", TEST_SHELL=shell, SHELL=shell)
687+
env["PYTHONPATH"] = str(Path(__file__).resolve().parents[1])
688+
process = subprocess.Popen([sys.executable, "-c", bootstrap], stdin=slave, stdout=slave, stderr=slave, env=env)
689+
os.close(slave)
690+
decoder = codecs.getincrementaldecoder("utf-8")("replace")
691+
transcript = ""
692+
693+
def wait_until(predicate):
694+
nonlocal transcript
695+
deadline = time.monotonic() + 10
696+
while time.monotonic() < deadline:
697+
if select.select([master], [], [], 0.05)[0]:
698+
data = decoder.decode(os.read(master, 65536))
699+
transcript += data
700+
if "\x1b[6n" in data:
701+
# Answer prompt-toolkit's cursor-position request as a terminal would.
702+
os.write(master, b"\x1b[1;1R")
703+
if predicate():
704+
return
705+
pytest.fail(f"terminal condition timed out:\n{transcript}\n{describe_processes(process.pid, master)}")
706+
707+
command = "probe" if producer == "command" else f"shell {shlex.quote(sys.executable)} {shlex.quote(str(probe))}"
708+
try:
709+
wait_until(lambda: "OUTER> " in transcript)
710+
os.write(master, f"{shlex.quote(sys.executable)} {shlex.quote(str(application))}\n".encode())
711+
wait_until(lambda: "TEST>" in transcript)
712+
start = len(transcript)
713+
os.write(master, f"{command} | cat\n".encode())
714+
wait_until(lambda: outcome.exists() and outcome.read_text() != "" and "TEST>" in transcript[start:])
715+
assert outcome.read_text() == "False"
716+
os.write(master, b"quit\n")
717+
wait_until(lambda: os.tcgetpgrp(master) == process.pid)
718+
finally:
719+
os.close(master)
720+
process.kill()
721+
process.wait(timeout=5)
722+
723+
645724
@pytest.mark.parametrize("suspend", [False, True])
646725
def test_shell_producer_keeps_the_terminal_after_its_consumer_exits(tmp_path, suspend) -> None:
647726
"""A shell producer that outlives its consumer still reads the terminal.

0 commit comments

Comments
 (0)