Add option to delay propegating SIGINT to child process (#1588)

Co-Authored-By: Stefan H. Holek <stefan@epy.co.at>
Co-authored-by: Stefan H. Holek <stefan@epy.co.at>

8 files changed, 47 insertions, 19 deletions

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index e9488628..80dda65c 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -47,6 +47,7 @@ Johannes Christ
 Jon Dufresne
 Josh Smeaton
 Josh Snyder
+Joshua Hesketh
 Julian Krause
 Jurko Gospodnetić
 Krisztian Fekete
diff --git a/docs/changelog/1497.bugfix.rst b/docs/changelog/1497.bugfix.rst
new file mode 100644
index 00000000..dce157a1
--- /dev/null
+++ b/docs/changelog/1497.bugfix.rst
@@ -0,0 +1 @@
+Add an option to allow a process to suicide before sending the SIGTERM. - by :user:`jhesketh`
diff --git a/docs/config.rst b/docs/config.rst
index ad09f191..78d814e7 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -160,22 +160,6 @@ 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,
-    waits :conf:``interrupt_timeout`` seconds, and sends it a SIGTERM if it hasn't
-    exited.
-
-.. 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.
-
 Jenkins override
 ++++++++++++++++
 
@@ -597,6 +581,33 @@ 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.0
+
+    .. 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
+    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, 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 a7f022e2..74eec55a 100644
--- a/src/tox/action.py
+++ b/src/tox/action.py
@@ -32,6 +32,7 @@ class Action(object):
         command_log,
         popen,
         python,
+        suicide_timeout,
         interrupt_timeout,
         terminate_timeout,
     ):
@@ -45,6 +46,7 @@ class Action(object):
         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 @@ class Action(object):
     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 fd3cb07f..f42a68d5 100644
--- a/src/tox/config/__init__.py
+++ b/src/tox/config/__init__.py
@@ -53,6 +53,7 @@ hookimpl = tox.hookimpl
 
 WITHIN_PROVISION = os.environ.get(str("TOX_PROVISION")) == "1"
 
+SUICIDE_TIMEOUT = 0.0
 INTERRUPT_TIMEOUT = 0.3
 TERMINATE_TIMEOUT = 0.2
 
@@ -828,6 +829,13 @@ def tox_addoption(parser):
     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",
         default=INTERRUPT_TIMEOUT,
diff --git a/src/tox/session/__init__.py b/src/tox/session/__init__.py
index 22c32bd3..27d6310e 100644
--- a/src/tox/session/__init__.py
+++ b/src/tox/session/__init__.py
@@ -19,7 +19,7 @@ import py
 import tox
 from tox import reporter
 from tox.action import Action
-from tox.config import INTERRUPT_TIMEOUT, TERMINATE_TIMEOUT, parseconfig
+from tox.config import INTERRUPT_TIMEOUT, SUICIDE_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,7 @@ class Session(object):
             self.resultlog.command_log,
             self.popen,
             sys.executable,
+            SUICIDE_TIMEOUT,
             INTERRUPT_TIMEOUT,
             TERMINATE_TIMEOUT,
         )
diff --git a/src/tox/venv.py b/src/tox/venv.py
index bf91e190..6524296b 100644
--- a/src/tox/venv.py
+++ b/src/tox/venv.py
@@ -130,6 +130,7 @@ class VirtualEnv(object):
             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 911be6e3..91f5c204 100644
--- a/tests/unit/config/test_config.py
+++ b/tests/unit/config/test_config.py
@@ -175,11 +175,12 @@ class TestVenvConfig:
         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 @@ class TestVenvConfig:
         """,
         )
         envconfig = config.envconfigs["other"]
+        assert 0.0 == 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