Skip to content

Commit

Permalink
fix(signals): fix processing exit signals and exit exception (#5399)
Browse files Browse the repository at this point in the history
### Before

Case 1: Handler catches the exit signal and do not pass it forward. As
result xonsh could be suspended by OS or crash. Also exit code is wrong.
See repeatable examples with SIGHUP in
#5381 (comment).

Case 2: From bash/zsh as login shell run xonsh. Then send quit signal to
xonsh. The terminal state will be broken: disabled SIGINT, mouse pointer
produces mouse state codes.

### After

Case 1: Xonsh exit normally with right exit code. Fixed #5381 #5304
#5371.

Case 2: After exiting with right exit code the state of the terminal is
good.

## For community
⬇️ **Please click the 👍 reaction instead of leaving a `+1` or 👍
comment**

---------

Co-authored-by: a <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed May 13, 2024
1 parent bb394a8 commit fd5304f
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 12 deletions.
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 @@ -1225,3 +1232,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

0 comments on commit fd5304f

Please sign in to comment.