From 803a5494ef23187e920eeb4b42e922b87cda5966 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 May 2022 07:18:51 -0400 Subject: [PATCH] fix: the SIGTERM handler is now opt-in. #1310 --- CHANGES.rst | 7 +++++++ coverage/config.py | 2 ++ coverage/control.py | 13 +++++++------ doc/config.rst | 18 ++++++++++++++++++ tests/test_concurrency.py | 13 ++++++++++--- 5 files changed, 44 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index f7258a307..c27722380 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,9 +20,16 @@ development at the same time, such as 4.5.x and 5.0. Unreleased ---------- +- A new setting, :ref:`config_run_sigterm`, controls whether a SIGTERM signal + handler is used. In 6.3, the signal handler was always installed, to capture + data at unusual process ends. Unfortunately, this introduced other problems + (see `issue 1310`_). Now the signal handler is only used if you opt-in by + setting ``[run] sigterm = true``. + - On Python 3.11, the ``[toml]`` extra no longer installs tomli, instead using tomllib from the standard library. Thanks `Shantanu `_. +.. _issue 1310: https://github.com/nedbat/coveragepy/issues/1310 .. _pull 1359: https://github.com/nedbat/coveragepy/pull/1359 diff --git a/coverage/config.py b/coverage/config.py index 1571c0176..1ad46597c 100644 --- a/coverage/config.py +++ b/coverage/config.py @@ -190,6 +190,7 @@ def __init__(self): self.relative_files = False self.run_include = None self.run_omit = None + self.sigterm = False self.source = None self.source_pkgs = [] self.timid = False @@ -364,6 +365,7 @@ def copy(self): ('relative_files', 'run:relative_files', 'boolean'), ('run_include', 'run:include', 'list'), ('run_omit', 'run:omit', 'list'), + ('sigterm', 'run:sigterm', 'boolean'), ('source', 'run:source', 'list'), ('source_pkgs', 'run:source_pkgs', 'list'), ('timid', 'run:timid', 'boolean'), diff --git a/coverage/control.py b/coverage/control.py index 6387d0dd3..a0571c976 100644 --- a/coverage/control.py +++ b/coverage/control.py @@ -536,12 +536,13 @@ def _init_for_start(self): # Register our clean-up handlers. atexit.register(self._atexit) - is_main = (threading.current_thread() == threading.main_thread()) - if is_main and not env.WINDOWS: - # The Python docs seem to imply that SIGTERM works uniformly even - # on Windows, but that's not my experience, and this agrees: - # https://stackoverflow.com/questions/35772001/x/35792192#35792192 - self._old_sigterm = signal.signal(signal.SIGTERM, self._on_sigterm) + if self.config.sigterm: + is_main = (threading.current_thread() == threading.main_thread()) + if is_main and not env.WINDOWS: + # The Python docs seem to imply that SIGTERM works uniformly even + # on Windows, but that's not my experience, and this agrees: + # https://stackoverflow.com/questions/35772001/x/35792192#35792192 + self._old_sigterm = signal.signal(signal.SIGTERM, self._on_sigterm) def _init_data(self, suffix): """Create a data file if we don't have one yet.""" diff --git a/doc/config.rst b/doc/config.rst index be23dbdfa..70f56c0e5 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -268,6 +268,24 @@ need to know the source origin. .. versionadded:: 5.0 +.. _config_run_sigterm: + +[run] sigterm +............. + +(boolean, default False) if true, register a SIGTERM signal handler to capture +data when the process ends due to a SIGTERM signal. This includes +:meth:`Process.terminate `, and other +ways to terminate a process. This can help when collecting data in usual +situations, but can also introduce problems (see `issue 1310`_). + +Only on Linux and Mac. + +.. _issue 1310: https://github.com/nedbat/coveragepy/issues/1310 + +.. versionadded:: 6.4 (in 6.3 this was always enabled) + + .. _config_run_source: [run] source diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index 9d534981a..0a51d4d96 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -696,7 +696,8 @@ def random_load(): # pragma: nested class SigtermTest(CoverageTest): """Tests of our handling of SIGTERM.""" - def test_sigterm_saves_data(self): + @pytest.mark.parametrize("sigterm", [False, True]) + def test_sigterm_saves_data(self, sigterm): # A terminated process should save its coverage data. self.make_file("clobbered.py", """\ import multiprocessing @@ -724,7 +725,8 @@ def subproc(x): [run] parallel = True concurrency = multiprocessing - """) + """ + ("sigterm = true" if sigterm else "") + ) out = self.run_command("coverage run clobbered.py") # Under the Python tracer on Linux, we get the "Trace function changed" # message. Does that matter? @@ -735,7 +737,11 @@ def subproc(x): assert out == "START\nNOT THREE\nEND\n" self.run_command("coverage combine") out = self.run_command("coverage report -m") - assert self.squeezed_lines(out)[2] == "clobbered.py 17 1 94% 6" + if sigterm: + expected = "clobbered.py 17 1 94% 6" + else: + expected = "clobbered.py 17 5 71% 5-10" + assert self.squeezed_lines(out)[2] == expected def test_sigterm_still_runs(self): # A terminated process still runs its own SIGTERM handler. @@ -766,6 +772,7 @@ def on_sigterm(signum, frame): [run] parallel = True concurrency = multiprocessing + sigterm = True """) out = self.run_command("coverage run handler.py") assert out == "START\nSIGTERM\nEND\n"