Allow to config the behavior on CTRL+C

[sileht]

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)

Contribution checklist:

(also see CONTRIBUTING.rst for details)

• wrote descriptive pull request text
• added/updated test(s)
• updated/extended the documentation
• added relevant issue keyword
  in message body
• added news fragment in changelog folder
  • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
  • <type> is must be one of bugfix, feature, deprecation,breaking, doc, misc
  • if PR has no issue: consider creating one first or change it to the PR number after creating the PR
  • "sign" fragment with "by :user:<your username>"
  • please use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files - by :user:superuser."
  • also see examples
• added yourself to CONTRIBUTORS (preserving alphabetical order)

[gaborbernat]

On the surface the idea looks ok; will need tests and documentation for it though 👍

[gaborbernat]

you can use the :conf:`interrupt_timeout` to make them references please do so

[sileht]

done

[gaborbernat]

Thanks!

[stefanholek]

Hi People,

I have been dealing with a similar issue the other day, and my solution is different. 😎

I have a test runner which install unittests's signal handlers. When hitting Ctrl+C in tox, the test runner sometimes receives a SIGINT while in the process of shutting down. This seemed wrong, and I traced it down to tox unconditionally forwarding any received SIGINT. The situation however is that all foreground processes receive the SIGINT, not just tox, and when tox forwards it the test runner receives the SIGINT twice. My solutions is to make tox wait for processes to go away on their own before starting to send signals.

I see you are talking about SIGKILL, but maybe that's just a side-effect of tox SIGINT'ing immediately? Also, my issue is not fixed by the changes in #1493.

Anyway, here is my patch (which no longer applies now, but you get the idea):

diff --git a/src/tox/action.py b/src/tox/action.py
index 10707b4..9f826a0 100644
--- a/src/tox/action.py
+++ b/src/tox/action.py
@@ -18,6 +18,7 @@ from tox.reporter import Verbosity
 from tox.util.lock import get_unique_file
 from tox.util.stdlib import is_main_thread
 
+WAIT_SUICIDE = 0.5
 WAIT_INTERRUPT = 0.3
 WAIT_TERMINATE = 0.2
 
@@ -177,7 +178,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, WAIT_SUICIDE) 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:
@@ -193,7 +194,7 @@ class Action(object):
         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:

[sileht]

#1493 does not change anything about signal handling by default. But just allow to configure different timeouts.

I got your double SIGINT issue. But my issue was a bit different.
My tooling don't care about receiving SIGINT/SIGTERM manytimes, any additional signals are just ignored during the processes shutdown procedure. My patch was to allow to increase the delay between the first SIGINT and the final deadly SIGKILL, so my test runner can cleanly shutdown everything, other signals sent between are ignored by my tooling.

I think your issue/use-case is valid too, feel free to open an issue to track it.

[stefanholek]

Ok, thanks.