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

Handle sigterm by killing the commands subprocess #1860

Merged
merged 2 commits into from Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/changelog/1772.bugfix.rst
@@ -0,0 +1,2 @@
Fix a killed tox (via SIGTERM) leaving the commands subprocesses running
by handling it as if it were a KeyboardInterrupt - by :user:`dajose`
6 changes: 3 additions & 3 deletions docs/config.rst
Expand Up @@ -607,9 +607,9 @@ Complete list of settings that you can put into ``testenv*`` sections:

.. versionadded:: 3.15.2

When an interrupt is sent via Ctrl+C, the SIGINT is sent to all foreground
processes. The :conf:``suicide_timeout`` gives the running process time to
cleanup and exit before receiving (in some cases, a duplicate) SIGINT from
When an interrupt is sent via Ctrl+C or the tox process is killed with a SIGTERM,
a SIGINT is sent to all foreground processes. The :conf:``suicide_timeout`` gives
the running process time to cleanup and exit before receiving (in some cases, a duplicate) SIGINT from
tox.

.. conf:: interrupt_timeout ^ float ^ 0.3
Expand Down
13 changes: 13 additions & 0 deletions src/tox/action.py
Expand Up @@ -49,6 +49,10 @@ def __init__(
self.suicide_timeout = suicide_timeout
self.interrupt_timeout = interrupt_timeout
self.terminate_timeout = terminate_timeout
if is_main_thread():
# python allows only main thread to install signal handlers
# see https://docs.python.org/3/library/signal.html#signals-and-threads
self._install_sigterm_handler()

def __enter__(self):
msg = "{} {}".format(self.msg, " ".join(map(str, self.args)))
Expand Down Expand Up @@ -278,3 +282,12 @@ def _rewrite_args(self, cwd, args):
new_args.append(str(arg))

return new_args

def _install_sigterm_handler(self):
"""Handle sigterm as if it were a keyboardinterrupt"""

def sigterm_handler(signum, frame):
reporter.error("Got SIGTERM, handling it as a KeyboardInterrupt")
raise KeyboardInterrupt()

signal.signal(signal.SIGTERM, sigterm_handler)
5 changes: 3 additions & 2 deletions tests/integration/test_provision_int.py
Expand Up @@ -73,7 +73,8 @@ def test_provision_from_pyvenv(initproj, cmd, monkeypatch):
"sys.platform == 'win32'",
reason="triggering SIGINT reliably on Windows is hard",
)
def test_provision_interrupt_child(initproj, monkeypatch, capfd):
@pytest.mark.parametrize("signal_type", [signal.SIGINT, signal.SIGTERM])
def test_provision_interrupt_child(initproj, monkeypatch, capfd, signal_type):
monkeypatch.delenv(str("PYTHONPATH"), raising=False)
monkeypatch.setenv(str("TOX_REPORTER_TIMESTAMP"), str("1"))
initproj(
Expand Down Expand Up @@ -123,7 +124,7 @@ def test_provision_interrupt_child(initproj, monkeypatch, capfd):
# 1 process for the host tox, 1 for the provisioned
assert len(all_process) >= 2, all_process

process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT)
process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal_type)
process.communicate()
out, err = capfd.readouterr()
assert ".tox KeyboardInterrupt: from" in out, out
Expand Down