Skip to content

Commit

Permalink
Allow to config the behavior on CTRL+C
Browse files Browse the repository at this point in the history
By default, we run SIGKILL after 0.5 seconds. Most of the time is
enough. But if the interrupted command have a complex processes tree,
it might not be enough to propagate the signal.

In such case processes are left behind and never killed.
If theses processes use static network port or keep file open. Next
call of tox will fail until the all processes left behind are manually
killed.

This change adds some configuration to be able to config the timeout
before signals are sent.

If the approach work for you, I will polish the PR (doc+test)
  • Loading branch information
sileht committed Jan 9, 2020
1 parent 6472eac commit 34ee673
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 8 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Mark Hirota
Matt Good
Matt Jeffery
Mattieu Agopian
Mehdi Abaakouk
Michael Manganiello
Mickaël Schoentgen
Mikhail Kyshtymov
Expand Down
1 change: 1 addition & 0 deletions docs/changelog/1493.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ``interrupt_timeout`` and ``terminate_timeout`` that configure delay between SIGINT, SIGTERM and SIGKILL when tox is interrupted. - by :user:`sileht`
15 changes: 15 additions & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,21 @@ Global settings are defined under the ``tox`` section as:
Name of the virtual environment used to create a source distribution from the
source tree.

.. conf:: interrupt_timeout ^ float ^ 0.3

.. versionadded:: 3.15.0

When tox is interrupted, it propagates the signal to the child process,
wait `interrupt_timeout` seconds, and sends it a SIGTERM if it haven't
exited.

.. conf:: terminate_timeout ^ float ^ 0.2

.. versionadded:: 3.15.0

When tox is interrupted, it propagates the signal to the child process,
wait `interrupt_timeout` seconds, sends it a SIGTERM, wait
`terminate_timeout` seconds, and sends it a SIGKILL if it haven't exited.

Jenkins override
++++++++++++++++
Expand Down
14 changes: 7 additions & 7 deletions src/tox/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@
from tox.util.lock import get_unique_file
from tox.util.stdlib import is_main_thread

WAIT_INTERRUPT = 0.3
WAIT_TERMINATE = 0.2


class Action(object):
"""Action is an effort to group operations with the same goal (within reporting)"""

def __init__(self, name, msg, args, log_dir, generate_tox_log, command_log, popen, python):
def __init__(self, name, msg, args, log_dir, generate_tox_log, command_log, popen,
python, interrupt_timeout, terminate_timeout):
self.name = name
self.args = args
self.msg = msg
Expand All @@ -36,6 +34,8 @@ def __init__(self, name, msg, args, log_dir, generate_tox_log, command_log, pope
self.command_log = command_log
self._timed_report = None
self.python = python
self.interrupt_timeout = interrupt_timeout
self.terminate_timeout = terminate_timeout

def __enter__(self):
msg = "{} {}".format(self.msg, " ".join(map(str, self.args)))
Expand Down Expand Up @@ -180,10 +180,10 @@ def handle_interrupt(self, process):
if process.poll() 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, WAIT_INTERRUPT) is None:
if self._wait(process, self.interrupt_timeout) is None:
self.info("KeyboardInterrupt", msg.format("SIGTERM"))
process.terminate()
if self._wait(process, WAIT_TERMINATE) is None:
if self._wait(process, self.terminate_timeout) is None:
self.info("KeyboardInterrupt", msg.format("SIGKILL"))
process.kill()
process.communicate()
Expand All @@ -193,7 +193,7 @@ def _wait(process, timeout):
if sys.version_info >= (3, 3):
# python 3 has timeout feature built-in
try:
process.communicate(timeout=WAIT_INTERRUPT)
process.communicate(timeout=timeout)
except subprocess.TimeoutExpired:
pass
else:
Expand Down
36 changes: 35 additions & 1 deletion src/tox/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@

WITHIN_PROVISION = os.environ.get(str("TOX_PROVISION")) == "1"

INTERRUPT_TIMEOUT = 0.3
TERMINATE_TIMEOUT = 0.2


def get_plugin_manager(plugins=()):
# initialize plugin manager
Expand Down Expand Up @@ -798,6 +801,20 @@ def develop(testenv_config, value):

parser.add_testenv_attribute_obj(DepOption())

parser.add_testenv_attribute(
name="interrupt_timeout",
type="float",
default=INTERRUPT_TIMEOUT,
help="timeout before sending SIGTERM after SIGINT",
)

parser.add_testenv_attribute(
name="terminate_timeout",
type="float",
default=TERMINATE_TIMEOUT,
help="timeout before sending SIGKILL after SIGTERM",
)

parser.add_testenv_attribute(
name="commands",
type="argvlist",
Expand Down Expand Up @@ -1231,7 +1248,8 @@ def make_envconfig(self, name, section, subs, config, replace=True):
for env_attr in config._testenv_attr:
atype = env_attr.type
try:
if atype in ("bool", "path", "string", "dict", "dict_setenv", "argv", "argvlist"):
if atype in ("bool", "float", "path", "string",
"dict", "dict_setenv", "argv", "argvlist"):
meth = getattr(reader, "get{}".format(atype))
res = meth(env_attr.name, env_attr.default, replace=replace)
elif atype == "basepython":
Expand Down Expand Up @@ -1448,6 +1466,22 @@ def _getdict(self, value, default, sep, replace=True):

return d

def getfloat(self, name, default=None, replace=True):
s = self.getstring(name, default, replace=replace)
if not s or not replace:
s = default
if s is None:
raise KeyError("no config value [{}] {} found".format(self.section_name, name))

if not isinstance(s, float):
try:
s = float(s)
except ValueError:
raise tox.exception.ConfigError(
"{}: invalid float {!r}".format(name, s)
)
return s

def getbool(self, name, default=None, replace=True):
s = self.getstring(name, default, replace=replace)
if not s or not replace:
Expand Down
4 changes: 4 additions & 0 deletions src/tox/session/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from tox import reporter
from tox.action import Action
from tox.config import parseconfig
from tox.config import INTERRUPT_TIMEOUT
from tox.config import TERMINATE_TIMEOUT
from tox.config.parallel import ENV_VAR_KEY_PRIVATE as PARALLEL_ENV_VAR_KEY_PRIVATE
from tox.config.parallel import OFF_VALUE as PARALLEL_OFF
from tox.logs.result import ResultLog
Expand Down Expand Up @@ -170,6 +172,8 @@ def newaction(self, name, msg, *args):
self.resultlog.command_log,
self.popen,
sys.executable,
INTERRUPT_TIMEOUT,
TERMINATE_TIMEOUT,
)

def runcommand(self):
Expand Down
2 changes: 2 additions & 0 deletions src/tox/venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ def new_action(self, msg, *args):
command_log,
self.popen,
self.envconfig.envpython,
self.envconfig.interrupt_timeout,
self.envconfig.terminate_timeout,
)

def get_result_json_path(self):
Expand Down
19 changes: 19 additions & 0 deletions tests/unit/config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +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):
config = newconfig(
[],
"""
[testenv:dev]
interrupt_timeout = 5.0
terminate_timeout = 10.0
[testenv:other]
""",
)
envconfig = config.envconfigs["other"]
assert 0.3 == envconfig.interrupt_timeout
assert 0.2 == envconfig.terminate_timeout

envconfig = config.envconfigs["dev"]
assert 5.0 == envconfig.interrupt_timeout
assert 10.0 == envconfig.terminate_timeout


class TestConfigPlatform:
def test_config_parse_platform(self, newconfig):
Expand Down

0 comments on commit 34ee673

Please sign in to comment.