Skip to content

Commit

Permalink
Add option to delay propegating SIGINT to child process
Browse files Browse the repository at this point in the history
Fixes tox-dev#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 <stefan@epy.co.at>
  • Loading branch information
jhesketh and stefanholek committed May 21, 2020
1 parent 58fec20 commit 6c56a2b
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 8 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Johannes Christ
Jon Dufresne
Josh Smeaton
Josh Snyder
Joshua Hesketh
Julian Krause
Jurko Gospodnetić
Krisztian Fekete
Expand Down
1 change: 1 addition & 0 deletions docs/changelog/1497.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an option to allow a process to suicide before sending the SIGTERM. - by :user:`jhesketh`
23 changes: 17 additions & 6 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------
Expand Down
4 changes: 3 additions & 1 deletion src/tox/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def __init__(
command_log,
popen,
python,
suicide_timeout,
interrupt_timeout,
terminate_timeout,
):
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions src/tox/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/tox/venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,25 @@ 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
[testenv:other]
""",
)
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

Expand Down

0 comments on commit 6c56a2b

Please sign in to comment.