Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed processing exit signals and exit exception #5399

Merged
merged 24 commits into from
May 13, 2024
23 changes: 23 additions & 0 deletions news/sig_exit_fix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Fixed processing exit signals and exceptions (e.g. SIGHUP in #5381) to provide careful exiting with right exit code and TTY cleaning.

**Security:**

* <news item>
1 change: 1 addition & 0 deletions tests/procs/test_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def test_capture_always(


@skip_if_on_windows
@pytest.mark.flaky(reruns=3, reruns_delay=1)
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)"]]
Expand Down
29 changes: 28 additions & 1 deletion tests/test_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@
def run_xonsh(
cmd,
stdin=sp.PIPE,
stdin_cmd=None,
stdout=sp.PIPE,
stderr=sp.STDOUT,
single_command=False,
interactive=False,
path=None,
add_args: list = None,
timeout=20,
):
env = dict(os.environ)
if path is None:
Expand All @@ -72,6 +74,7 @@ def run_xonsh(
input = cmd
if add_args:
args += add_args

proc = sp.Popen(
args,
env=env,
Expand All @@ -81,8 +84,12 @@ def run_xonsh(
universal_newlines=True,
)

if stdin_cmd:
proc.stdin.write(stdin_cmd)
proc.stdin.flush()

try:
out, err = proc.communicate(input=input, timeout=20)
out, err = proc.communicate(input=input, timeout=timeout)
except sp.TimeoutExpired:
proc.kill()
raise
Expand Down Expand Up @@ -1054,3 +1061,23 @@ def test_main_d():
single_command=True,
)
assert out == "dummy\n"


def test_catching_system_exit():
stdin_cmd = "__import__('sys').exit(2)\n"
out, err, ret = run_xonsh(
cmd=None, stdin_cmd=stdin_cmd, interactive=True, single_command=False, timeout=3
)
if ON_WINDOWS:
assert ret == 1
else:
assert ret == 2


@skip_if_on_windows
def test_catching_exit_signal():
stdin_cmd = "kill -SIGHUP @(__import__('os').getpid())\n"
out, err, ret = run_xonsh(
cmd=None, stdin_cmd=stdin_cmd, interactive=True, single_command=False, timeout=3
)
assert ret > 0
13 changes: 9 additions & 4 deletions xonsh/built_ins.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,20 @@ def resetting_signal_handle(sig, f):
"""Sets a new signal handle that will automatically restore the old value
once the new handle is finished.
"""
oldh = signal.getsignal(sig)
prev_signal_handler = signal.getsignal(sig)

def newh(s=None, frame=None):
def new_signal_handler(s=None, frame=None):
f(s, frame)
signal.signal(sig, oldh)
signal.signal(sig, prev_signal_handler)
if sig != 0:
"""
There is no immediate exiting here.
The ``sys.exit()`` function raises a ``SystemExit`` exception.
This exception must be caught and processed in the upstream code.
"""
sys.exit(sig)

signal.signal(sig, newh)
signal.signal(sig, new_signal_handler)


def helper(x, name=""):
Expand Down
11 changes: 8 additions & 3 deletions xonsh/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,13 +577,18 @@ def func_sig_ttin_ttou(n, f):
else:
# pass error to finally clause
exc_info = sys.exc_info()
except SystemExit:
exc_info = sys.exc_info()
finally:
if exc_info != (None, None, None):
err_type, err, _ = exc_info
if err_type is SystemExit:
raise err
print_exception(None, exc_info)
exit_code = 1
XSH.exit = True
code = getattr(exc_info[1], "code", 0)
exit_code = int(code) if code is not None else 0
else:
exit_code = 1
print_exception(None, exc_info)
events.on_exit.fire()
postmain(args)
return exit_code
Expand Down
12 changes: 9 additions & 3 deletions xonsh/ptk_shell/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from types import MethodType

from prompt_toolkit import ANSI
from prompt_toolkit.application.current import get_app
from prompt_toolkit.auto_suggest import AutoSuggestFromHistory
from prompt_toolkit.clipboard import InMemoryClipboard
from prompt_toolkit.enums import EditingMode
Expand Down Expand Up @@ -410,8 +411,12 @@ def cmdloop(self, intro=None):
raw_line = line
line = self.precmd(line)
self.default(line, raw_line)
except (KeyboardInterrupt, SystemExit):
except (KeyboardInterrupt, SystemExit) as e:
self.reset_buffer()
if isinstance(e, SystemExit):
get_app().reset() # Reset TTY mouse and keys handlers.
self.restore_tty_sanity() # Reset TTY SIGINT handlers.
raise
except EOFError:
if XSH.env.get("IGNOREEOF"):
print('Use "exit" to leave the shell.', file=sys.stderr)
Expand Down Expand Up @@ -583,8 +588,9 @@ def color_style(self):

def restore_tty_sanity(self):
"""An interface for resetting the TTY stdin mode. This is highly
dependent on the shell backend. Also it is mostly optional since
it only affects ^Z backgrounding behaviour.
dependent on the shell backend.
For prompt-toolkit it allows to fix case when terminal lost
SIGINT catching and Ctrl+C is not working after abnormal exiting.
"""
# PTK does not seem to need any specialization here. However,
# if it does for some reason in the future...
Expand Down
4 changes: 3 additions & 1 deletion xonsh/readline_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,11 +618,13 @@ def cmdloop(self, intro=None):
while not XSH.exit:
try:
self._cmdloop(intro=intro)
except (KeyboardInterrupt, SystemExit):
except (KeyboardInterrupt, SystemExit) as e:
print(file=self.stdout) # Gives a newline
fix_readline_state_after_ctrl_c()
self.reset_buffer()
intro = None
if isinstance(e, SystemExit):
raise

@property
def prompt(self):
Expand Down