From eea285ddbff9547a700411003529c1b544e9ef5d Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Wed, 8 Jan 2020 14:33:13 +0100 Subject: [PATCH] Allow to config the behavior on CTRL+C 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) --- CONTRIBUTORS | 1 + docs/changelog/1493.feature.rst | 1 + docs/config.rst | 15 ++++++++++++ src/tox/action.py | 25 +++++++++++++------ src/tox/config/__init__.py | 42 +++++++++++++++++++++++++++++++- src/tox/session/__init__.py | 4 ++- src/tox/venv.py | 2 ++ tests/unit/config/test_config.py | 19 +++++++++++++++ 8 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 docs/changelog/1493.feature.rst diff --git a/CONTRIBUTORS b/CONTRIBUTORS index f944ddb2d..5701b5b13 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -57,6 +57,7 @@ Mark Hirota Matt Good Matt Jeffery Mattieu Agopian +Mehdi Abaakouk Michael Manganiello Mickaël Schoentgen Mikhail Kyshtymov diff --git a/docs/changelog/1493.feature.rst b/docs/changelog/1493.feature.rst new file mode 100644 index 000000000..20f8fb304 --- /dev/null +++ b/docs/changelog/1493.feature.rst @@ -0,0 +1 @@ +Add ``interrupt_timeout`` and ``terminate_timeout`` that configure delay between SIGINT, SIGTERM and SIGKILL when tox is interrupted. - by :user:`sileht` diff --git a/docs/config.rst b/docs/config.rst index 8d9dbb74a..4eba670c1 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -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 :conf:``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 :conf:``interrupt_timeout`` seconds, sends it a SIGTERM, wait + :conf:``terminate_timeout`` seconds, and sends it a SIGKILL if it haven't exited. Jenkins override ++++++++++++++++ diff --git a/src/tox/action.py b/src/tox/action.py index 10707b48f..6dae0d07b 100644 --- a/src/tox/action.py +++ b/src/tox/action.py @@ -18,14 +18,23 @@ 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 @@ -36,6 +45,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))) @@ -180,10 +191,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() @@ -193,7 +204,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: diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index d80da5fff..3b6126765 100644 --- a/src/tox/config/__init__.py +++ b/src/tox/config/__init__.py @@ -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 @@ -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", @@ -1231,7 +1248,16 @@ 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": @@ -1448,6 +1474,20 @@ 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: diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py index b2e901e16..a2e0ca782 100644 --- a/src/tox/session/__init__.py +++ b/src/tox/session/__init__.py @@ -19,7 +19,7 @@ import tox from tox import reporter from tox.action import Action -from tox.config import parseconfig +from tox.config import INTERRUPT_TIMEOUT, TERMINATE_TIMEOUT, parseconfig 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 @@ -170,6 +170,8 @@ def newaction(self, name, msg, *args): self.resultlog.command_log, self.popen, sys.executable, + INTERRUPT_TIMEOUT, + TERMINATE_TIMEOUT, ) def runcommand(self): diff --git a/src/tox/venv.py b/src/tox/venv.py index 4edf741b7..0cd78c24f 100644 --- a/src/tox/venv.py +++ b/src/tox/venv.py @@ -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): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 9fd5b23ca..a717b8c37 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -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):