From 3ea74310d7ac8902823fec453c61555dc77c528e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 12 Feb 2020 07:06:20 -0300 Subject: [PATCH] Fix crash when faulthandler starts initialized (#6598) Use suggestion in review and use a subplugin so hooks will only be active if we enable faulthandler ourselves. Fix #6575 Co-authored-by: Daniel Hahler --- changelog/6575.bugfix.rst | 3 + src/_pytest/faulthandler.py | 122 +++++++++++++++++++++-------------- testing/test_faulthandler.py | 35 +++++++++- 3 files changed, 107 insertions(+), 53 deletions(-) create mode 100644 changelog/6575.bugfix.rst diff --git a/changelog/6575.bugfix.rst b/changelog/6575.bugfix.rst new file mode 100644 index 000000000..0fdfb64b3 --- /dev/null +++ b/changelog/6575.bugfix.rst @@ -0,0 +1,3 @@ +Fix internal crash when ``faulthandler`` starts initialized +(for example with ``PYTHONFAULTHANDLER=1`` environment variable set) and ``faulthandler_timeout`` defined +in the configuration file. diff --git a/src/_pytest/faulthandler.py b/src/_pytest/faulthandler.py index 068bec528..ed2dfd025 100644 --- a/src/_pytest/faulthandler.py +++ b/src/_pytest/faulthandler.py @@ -17,70 +17,92 @@ def pytest_addoption(parser): def pytest_configure(config): import faulthandler - # avoid trying to dup sys.stderr if faulthandler is already enabled - if faulthandler.is_enabled(): - return + if not faulthandler.is_enabled(): + # faulthhandler is not enabled, so install plugin that does the actual work + # of enabling faulthandler before each test executes. + config.pluginmanager.register(FaultHandlerHooks(), "faulthandler-hooks") + else: + from _pytest.warnings import _issue_warning_captured - stderr_fd_copy = os.dup(_get_stderr_fileno()) - config.fault_handler_stderr = os.fdopen(stderr_fd_copy, "w") - faulthandler.enable(file=config.fault_handler_stderr) + # Do not handle dumping to stderr if faulthandler is already enabled, so warn + # users that the option is being ignored. + timeout = FaultHandlerHooks.get_timeout_config_value(config) + if timeout > 0: + _issue_warning_captured( + pytest.PytestConfigWarning( + "faulthandler module enabled before pytest configuration step, " + "'faulthandler_timeout' option ignored" + ), + config.hook, + stacklevel=2, + ) -def _get_stderr_fileno(): - try: - return sys.stderr.fileno() - except (AttributeError, io.UnsupportedOperation): - # python-xdist monkeypatches sys.stderr with an object that is not an actual file. - # https://docs.python.org/3/library/faulthandler.html#issue-with-file-descriptors - # This is potentially dangerous, but the best we can do. - return sys.__stderr__.fileno() +class FaultHandlerHooks: + """Implements hooks that will actually install fault handler before tests execute, + as well as correctly handle pdb and internal errors.""" + def pytest_configure(self, config): + import faulthandler -def pytest_unconfigure(config): - import faulthandler + stderr_fd_copy = os.dup(self._get_stderr_fileno()) + config.fault_handler_stderr = os.fdopen(stderr_fd_copy, "w") + faulthandler.enable(file=config.fault_handler_stderr) - faulthandler.disable() - # close our dup file installed during pytest_configure - f = getattr(config, "fault_handler_stderr", None) - if f is not None: + def pytest_unconfigure(self, config): + import faulthandler + + faulthandler.disable() + # close our dup file installed during pytest_configure # re-enable the faulthandler, attaching it to the default sys.stderr # so we can see crashes after pytest has finished, usually during # garbage collection during interpreter shutdown config.fault_handler_stderr.close() del config.fault_handler_stderr - faulthandler.enable(file=_get_stderr_fileno()) + faulthandler.enable(file=self._get_stderr_fileno()) + @staticmethod + def _get_stderr_fileno(): + try: + return sys.stderr.fileno() + except (AttributeError, io.UnsupportedOperation): + # pytest-xdist monkeypatches sys.stderr with an object that is not an actual file. + # https://docs.python.org/3/library/faulthandler.html#issue-with-file-descriptors + # This is potentially dangerous, but the best we can do. + return sys.__stderr__.fileno() -@pytest.hookimpl(hookwrapper=True) -def pytest_runtest_protocol(item): - timeout = float(item.config.getini("faulthandler_timeout") or 0.0) - if timeout > 0: + @staticmethod + def get_timeout_config_value(config): + return float(config.getini("faulthandler_timeout") or 0.0) + + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_protocol(self, item): + timeout = self.get_timeout_config_value(item.config) + stderr = item.config.fault_handler_stderr + if timeout > 0 and stderr is not None: + import faulthandler + + faulthandler.dump_traceback_later(timeout, file=stderr) + try: + yield + finally: + faulthandler.cancel_dump_traceback_later() + else: + yield + + @pytest.hookimpl(tryfirst=True) + def pytest_enter_pdb(self): + """Cancel any traceback dumping due to timeout before entering pdb. + """ import faulthandler - stderr = item.config.fault_handler_stderr - faulthandler.dump_traceback_later(timeout, file=stderr) - try: - yield - finally: - faulthandler.cancel_dump_traceback_later() - else: - yield + faulthandler.cancel_dump_traceback_later() + @pytest.hookimpl(tryfirst=True) + def pytest_exception_interact(self): + """Cancel any traceback dumping due to an interactive exception being + raised. + """ + import faulthandler -@pytest.hookimpl(tryfirst=True) -def pytest_enter_pdb(): - """Cancel any traceback dumping due to timeout before entering pdb. - """ - import faulthandler - - faulthandler.cancel_dump_traceback_later() - - -@pytest.hookimpl(tryfirst=True) -def pytest_exception_interact(): - """Cancel any traceback dumping due to an interactive exception being - raised. - """ - import faulthandler - - faulthandler.cancel_dump_traceback_later() + faulthandler.cancel_dump_traceback_later() diff --git a/testing/test_faulthandler.py b/testing/test_faulthandler.py index 73bb66cf8..49ade6e5d 100644 --- a/testing/test_faulthandler.py +++ b/testing/test_faulthandler.py @@ -88,7 +88,7 @@ def test_cancel_timeout_on_hook(monkeypatch, hook_name): exception (pytest-dev/pytest-faulthandler#14). """ import faulthandler - from _pytest import faulthandler as plugin_module + from _pytest.faulthandler import FaultHandlerHooks called = [] @@ -98,6 +98,35 @@ def test_cancel_timeout_on_hook(monkeypatch, hook_name): # call our hook explicitly, we can trust that pytest will call the hook # for us at the appropriate moment - hook_func = getattr(plugin_module, hook_name) - hook_func() + hook_func = getattr(FaultHandlerHooks, hook_name) + hook_func(self=None) assert called == [1] + + +@pytest.mark.parametrize("faulthandler_timeout", [0, 2]) +def test_already_initialized(faulthandler_timeout, testdir): + """Test for faulthandler being initialized earlier than pytest (#6575)""" + testdir.makepyfile( + """ + def test(): + import faulthandler + assert faulthandler.is_enabled() + """ + ) + result = testdir.run( + sys.executable, + "-X", + "faulthandler", + "-mpytest", + testdir.tmpdir, + "-o", + "faulthandler_timeout={}".format(faulthandler_timeout), + ) + # ensure warning is emitted if faulthandler_timeout is configured + warning_line = "*faulthandler.py*faulthandler module enabled before*" + if faulthandler_timeout > 0: + result.stdout.fnmatch_lines(warning_line) + else: + result.stdout.no_fnmatch_line(warning_line) + result.stdout.fnmatch_lines("*1 passed*") + assert result.ret == 0