diff --git a/news/waitpid_returncode.rst b/news/waitpid_returncode.rst new file mode 100644 index 000000000..98566e839 --- /dev/null +++ b/news/waitpid_returncode.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Fixed populating the return code for interrupted process. + +**Security:** + +* diff --git a/tests/procs/test_specs.py b/tests/procs/test_specs.py index f2591e74b..402707e8b 100644 --- a/tests/procs/test_specs.py +++ b/tests/procs/test_specs.py @@ -1,6 +1,7 @@ """Tests the xonsh.procs.specs""" import itertools +import signal import sys from subprocess import Popen @@ -8,7 +9,12 @@ import pytest from xonsh.procs.posix import PopenThread from xonsh.procs.proxies import STDOUT_DISPATCHER, ProcProxy, ProcProxyThread -from xonsh.procs.specs import SubprocSpec, cmds_to_specs, run_subproc +from xonsh.procs.specs import ( + SubprocSpec, + _run_command_pipeline, + cmds_to_specs, + run_subproc, +) from xonsh.pytest.tools import skip_if_on_windows from xonsh.tools import XonshError @@ -134,6 +140,15 @@ def test_capture_always( assert exp in capfd.readouterr().out +@skip_if_on_windows +def test_interrupted_process_returncode(xonsh_session): + xonsh_session.env["RAISE_SUBPROC_ERROR"] = False + cmd = [["python", "-c", "import os, signal; os.kill(os.getpid(), signal.SIGINT)"]] + specs = cmds_to_specs(cmd, captured="stdout") + (p := _run_command_pipeline(specs, cmd)).end() + assert p.proc.returncode == -signal.SIGINT + + @skip_if_on_windows @pytest.mark.parametrize( "cmds, exp_stream_lines, exp_list_lines", diff --git a/xonsh/jobs.py b/xonsh/jobs.py index 0e841e479..3a239d595 100644 --- a/xonsh/jobs.py +++ b/xonsh/jobs.py @@ -39,6 +39,20 @@ _jobs_thread_local = threading.local() _tasks_main: collections.deque[int] = collections.deque() +def waitpid(pid, opt): + """ + Transparent wrapper on `os.waitpid` to make notes about undocumented subprocess behavior. + Basically ``p = subprocess.Popen()`` populates ``p.returncode`` after ``p.wait()``, ``p.poll()`` + or ``p.communicate()`` (https://docs.python.org/3/library/os.html#os.waitpid). + But if you're using `os.waitpid()` BEFORE these functions you're capturing return code + from a signal subsystem and ``p.returncode`` will be ``0``. + After ``os.waitid`` call you need to set return code manually + ``p.returncode = -os.WTERMSIG(status)`` like in Popen. + See also ``xonsh.tools.describe_waitpid_status()``. + """ + return os.waitpid(pid, opt) + + @contextlib.contextmanager def use_main_jobs(): """Context manager that replaces a thread's task queue and job dictionary @@ -270,7 +284,7 @@ else: if thread.pid is None: # When the process stopped before os.waitpid it has no pid. raise ChildProcessError("The process PID not found.") - _, wcode = os.waitpid(thread.pid, os.WUNTRACED) + _, wcode = waitpid(thread.pid, os.WUNTRACED) except ChildProcessError as e: # No child processes if return_error: return e @@ -284,7 +298,7 @@ else: elif os.WIFSIGNALED(wcode): print() # get a newline because ^C will have been printed thread.signal = (os.WTERMSIG(wcode), os.WCOREDUMP(wcode)) - thread.returncode = None + thread.returncode = -os.WTERMSIG(wcode) # Default Popen else: thread.returncode = os.WEXITSTATUS(wcode) thread.signal = None diff --git a/xonsh/procs/specs.py b/xonsh/procs/specs.py index 72fae475a..4751c14e9 100644 --- a/xonsh/procs/specs.py +++ b/xonsh/procs/specs.py @@ -934,25 +934,32 @@ def run_subproc(cmds, captured=False, envs=None): return _run_specs(specs, cmds) -def _run_specs(specs, cmds): +def _run_command_pipeline(specs, cmds): captured = specs[-1].captured if captured == "hiddenobject": - command = HiddenCommandPipeline(specs) + cp = HiddenCommandPipeline(specs) else: - command = CommandPipeline(specs) - proc = command.proc - background = command.spec.background + cp = CommandPipeline(specs) + proc = cp.proc + background = cp.spec.background if not all(x.is_proxy for x in specs): xj.add_job( { "cmds": cmds, - "pids": [i.pid for i in command.procs], + "pids": [i.pid for i in cp.procs], "obj": proc, "bg": background, - "pipeline": command, - "pgrp": command.term_pgid, + "pipeline": cp, + "pgrp": cp.term_pgid, } ) + return cp + + +def _run_specs(specs, cmds): + cp = _run_command_pipeline(specs, cmds) + proc, captured, background = cp.proc, specs[-1].captured, cp.spec.background + # For some reason, some programs are in a stopped state when the flow # reaches this point, hence a SIGCONT should be sent to `proc` to make # sure that the shell doesn't hang. @@ -961,16 +968,16 @@ def _run_specs(specs, cmds): # now figure out what we should return if captured == "object": - return command # object can be returned even if backgrounding + return cp # object can be returned even if backgrounding elif captured == "hiddenobject": if not background: - command.end() - return command + cp.end() + return cp elif background: return elif captured == "stdout": - command.end() - return command.output + cp.end() + return cp.output else: - command.end() + cp.end() return diff --git a/xonsh/tools.py b/xonsh/tools.py index 550f5dffe..7053a9e02 100644 --- a/xonsh/tools.py +++ b/xonsh/tools.py @@ -2841,3 +2841,17 @@ def to_repr_pretty_(inst, p, cycle): elif len(inst): p.break_() p.pretty(dict(inst)) + + +def describe_waitpid_status(status): + """Describes ``pid, status = os.waitpid(pid, opt)`` status.""" + funcs = [ + os.WIFEXITED, + os.WEXITSTATUS, + os.WIFSIGNALED, + os.WTERMSIG, + os.WIFSTOPPED, + os.WSTOPSIG, + ] + for f in funcs: + print(f.__name__, "-", f(status), "-", f.__doc__)