From b71dfa0a81dc156c9489df6c0390249a9d4914c1 Mon Sep 17 00:00:00 2001 From: Joshua Hesketh Date: Thu, 21 May 2020 15:52:39 +1000 Subject: [PATCH] Add option to delay propegating SIGINT to child process Fixes #1497 Ctrl+C sends a SIGTERM to both Tox and the running process, causing the running process to receive the signal twice once tox passes it along. This can often cancel or interfer with the process's cleanup. Instead, add an option to allow a process to suicide before sending the SIGTERM. Co-Authored-By: Stefan H. Holek --- docs/config.rst | 23 +++++++++++++++++------ src/tox/action.py | 4 +++- src/tox/config/__init__.py | 8 ++++++++ src/tox/venv.py | 1 + tests/unit/config/test_config.py | 5 ++++- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 76c3a5828c..99a7984499 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -581,21 +581,32 @@ Complete list of settings that you can put into ``testenv*`` sections: via the ``-e`` tox will only run those three (even if ``coverage`` may specify as ``depends`` other targets too - such as ``py27, py35, py36, py37``). +.. conf:: suicide_timeout ^ float ^ 0.1 + + .. 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 + tox. + .. conf:: interrupt_timeout ^ float ^ 0.3 .. versionadded:: 3.15.0 - When tox is interrupted, it propagates the signal to the child process, - waits :conf:``interrupt_timeout`` seconds, and sends it a SIGTERM if it hasn't - exited. + When tox is interrupted, it propagates the signal to the child process + after :conf:``suicide_timeout`` seconds. If the process still hasn't exited + after :conf:``interrupt_timeout`` seconds, its sends a SIGTERM. .. conf:: terminate_timeout ^ float ^ 0.2 .. versionadded:: 3.15.0 - When tox is interrupted, it propagates the signal to the child process, - waits :conf:``interrupt_timeout`` seconds, sends it a SIGTERM, waits - :conf:``terminate_timeout`` seconds, and sends it a SIGKILL if it hasn't exited. + When tox is interrupted, after waiting :conf:``interrupt_timeout`` seconds, + it propagates the signal to the child process, waits + :conf:``interrupt_timeout`` seconds, sends it a SIGTERM, waits + :conf:``terminate_timeout`` seconds, and sends it a SIGKILL if it hasn't + exited. Substitutions ------------- diff --git a/src/tox/action.py b/src/tox/action.py index a7f022e2f6..74eec55aa9 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -32,6 +32,7 @@ def __init__( command_log, popen, python, + suicide_timeout, interrupt_timeout, terminate_timeout, ): @@ -45,6 +46,7 @@ def __init__( self.command_log = command_log self._timed_report = None self.python = python + self.suicide_timeout = suicide_timeout self.interrupt_timeout = interrupt_timeout self.terminate_timeout = terminate_timeout @@ -188,7 +190,7 @@ def evaluate_cmd(self, input_file_handler, process, redirect): def handle_interrupt(self, process): """A three level stop mechanism for children - INT -> TERM -> KILL""" msg = "from {} {{}} pid {}".format(os.getpid(), process.pid) - if process.poll() is None: + if self._wait(process, self.suicide_timeout) is None: self.info("KeyboardInterrupt", msg.format("SIGINT")) process.send_signal(signal.CTRL_C_EVENT if sys.platform == "win32" else signal.SIGINT) if self._wait(process, self.interrupt_timeout) is None: diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index fd3cb07f04..262cdce935 100644 --- a/src/tox/config/__init__.py +++ b/src/tox/config/__init__.py @@ -53,6 +53,7 @@ WITHIN_PROVISION = os.environ.get(str("TOX_PROVISION")) == "1" +SUICIDE_TIMEOUT = 0.1 INTERRUPT_TIMEOUT = 0.3 TERMINATE_TIMEOUT = 0.2 @@ -827,6 +828,13 @@ def develop(testenv_config, value): parser.add_testenv_attribute_obj(DepOption()) + parser.add_testenv_attribute( + name="suicide_timeout", + type="float", + default=SUICIDE_TIMEOUT, + help="timeout to allow process to exit before sending SIGINT", + ) + parser.add_testenv_attribute( name="interrupt_timeout", type="float", diff --git a/src/tox/venv.py b/src/tox/venv.py index bf91e19023..6524296be0 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -130,6 +130,7 @@ def new_action(self, msg, *args): command_log, self.popen, self.envconfig.envpython, + self.envconfig.suicide_timeout, self.envconfig.interrupt_timeout, self.envconfig.terminate_timeout, ) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 911be6e36b..c1551ddba7 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -175,11 +175,12 @@ def test_is_same_dep(self): assert DepOption._is_same_dep("pkg_hello-world3==1.0", "pkg_hello-world3<=2.0") assert not DepOption._is_same_dep("pkg_hello-world3==1.0", "otherpkg>=2.0") - def test_interrupt_terminate_timeout_set_manually(self, newconfig): + def test_suicide_interrupt_terminate_timeout_set_manually(self, newconfig): config = newconfig( [], """ [testenv:dev] + suicide_timeout = 30.0 interrupt_timeout = 5.0 terminate_timeout = 10.0 @@ -187,10 +188,12 @@ def test_interrupt_terminate_timeout_set_manually(self, newconfig): """, ) envconfig = config.envconfigs["other"] + assert 0.1 == envconfig.suicide_timeout assert 0.3 == envconfig.interrupt_timeout assert 0.2 == envconfig.terminate_timeout envconfig = config.envconfigs["dev"] + assert 30.0 == envconfig.suicide_timeout assert 5.0 == envconfig.interrupt_timeout assert 10.0 == envconfig.terminate_timeout