From a8d248ec6c6685c3576f7b35f58dd5c949b7b3e7 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 13 Oct 2026 21:31:29 -0400 Subject: [PATCH] fix(utilities): fall back to the default threading excepthook before Python 3.10 (#1691) `threading.__excepthook__` is 3.10+, so `_threading_excepthook`'s re-entry fallback and its no-previous-hook delegate raised `AttributeError` on 3.8 and 3.9. - `pcapkit.utilities.compat` re-exports the default as `default_threading_excepthook`, chosen by `sys.version_info`: `threading.__excepthook__` from 3.10, `_thread._excepthook` on 3.8 and 3.9 (the same object), `None` below 3.8. `exceptions.py` imports it and keeps no feature probe of its own. - Tests capture the default before `import pcapkit` instead of reading `threading.__excepthook__`, and cover both fallbacks in-process and a hook installed before import. `test_compat` covers each branch of the new name, faking 3.5 and 3.9 through a generalised `load_compat_as`. The touched modules pass on 3.8.20, 3.9.25, 3.10.21 and 3.14.8. --- docs/source/pcapkit/utilities/index.rst | 2 + pcapkit/utilities/compat.py | 27 +++- pcapkit/utilities/exceptions.py | 12 +- tests/utilities/test_compat.py | 106 +++++++++++--- tests/utilities/test_exceptions_excepthook.py | 134 +++++++++++++++++- 5 files changed, 248 insertions(+), 33 deletions(-) diff --git a/docs/source/pcapkit/utilities/index.rst b/docs/source/pcapkit/utilities/index.rst index dcf2d6660b..5e57f9eab8 100644 --- a/docs/source/pcapkit/utilities/index.rst +++ b/docs/source/pcapkit/utilities/index.rst @@ -58,3 +58,5 @@ following objects and functions: - Python 3.11 * - :data:`typing.TypeAlias` - Python 3.10 + * - :data:`threading.__excepthook__`, as ``default_threading_excepthook`` + - Python 3.10 diff --git a/pcapkit/utilities/compat.py b/pcapkit/utilities/compat.py index dc73fa5959..6686dbd55a 100644 --- a/pcapkit/utilities/compat.py +++ b/pcapkit/utilities/compat.py @@ -8,7 +8,7 @@ __all__ = [ # functions - 'final', 'localcontext', 'show_flag_values', + 'final', 'localcontext', 'show_flag_values', 'default_threading_excepthook', # exceptions 'ModuleNotFoundError', @@ -226,3 +226,28 @@ def show_flag_values(value: 'IntFlag') -> 'list[int]': from typing_extensions import TypeAlias else: from typing import TypeAlias # novermin + +# The interpreter's own default :data:`threading.excepthook`, as it stood before +# anything replaced it, which :data:`threading.__excepthook__` names only from +# 3.10 (issue #1691). The object itself is older: CPython 3.8 and 3.9 start +# :data:`threading.excepthook` as ``_thread._excepthook``, the same function 3.10 +# and later bind as ``__excepthook__`` (checked with ``is`` on 3.8.20, 3.9.25, +# 3.10.21 and 3.14.8). Importing it from :mod:`_thread`, rather than reading +# whatever :data:`threading.excepthook` holds when this module loads, keeps it the +# real default on 3.8 and 3.9 even when a host application, or an earlier loaded +# copy of :mod:`pcapkit.utilities.exceptions`, has already replaced the hook -- +# as it is on 3.10 and later. 3.6 and 3.7 have no thread exception hook at all, +# so the name is :data:`None` there rather than a failed import; nothing on those +# interpreters calls it. +# +# The stdlib name is not reused as it stands, unlike every other name here: a +# bare ``__excepthook__`` imported into a module reads as +# :data:`sys.__excepthook__`, which takes three arguments rather than one, and +# :mod:`pcapkit.utilities.exceptions` uses both. ``default_`` is the word that +# module's docstrings already use for this object. +if sys.version_info < (3, 8): + default_threading_excepthook = None +elif sys.version_info < (3, 10): + from _thread import _excepthook as default_threading_excepthook +else: + from threading import __excepthook__ as default_threading_excepthook # novermin diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index 0e920dc36a..3f627f0626 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING from pcapkit.utilities.compat import ModuleNotFoundError # pylint: disable=redefined-builtin +from pcapkit.utilities.compat import default_threading_excepthook from pcapkit.utilities.logging import DEVMODE, VERBOSE, get_logger if TYPE_CHECKING: @@ -341,9 +342,10 @@ def _threading_excepthook(args: 'ExceptHookArgs') -> 'None': traceback -- then its own message tersely, with ``limit=0`` against the real traceback so a chained exception still comes out right. Anything else is handed to ``_previous_threading_excepthook`` -- or - :data:`threading.__excepthook__`, the interpreter's own default, if nothing - was previously installed -- unchanged, header included, since that path - does not touch the default hook's own printing at all. + :data:`~pcapkit.utilities.compat.default_threading_excepthook`, the + interpreter's own default, if nothing was previously installed -- + unchanged, header included, since that path does not touch the default + hook's own printing at all. This hook is needed because the interpreter's *default* :data:`threading.excepthook` consults :data:`sys.tracebacklimit`, which @@ -352,7 +354,7 @@ def _threading_excepthook(args: 'ExceptHookArgs') -> 'None': """ if getattr(_threading_hook_state, 'running', False): - threading.__excepthook__(args) + default_threading_excepthook(args) return _threading_hook_state.running = True @@ -368,7 +370,7 @@ def _threading_excepthook(args: 'ExceptHookArgs') -> 'None': print(f'Exception in thread {name}:', file=sys.stderr) traceback.print_exception(args.exc_type, args.exc_value, args.exc_traceback, 0) else: - delegate = _previous_threading_excepthook or threading.__excepthook__ + delegate = _previous_threading_excepthook or default_threading_excepthook delegate(args) finally: _threading_hook_state.running = False diff --git a/tests/utilities/test_compat.py b/tests/utilities/test_compat.py index 7ef9b0f486..c126410cbd 100644 --- a/tests/utilities/test_compat.py +++ b/tests/utilities/test_compat.py @@ -5,8 +5,9 @@ interpreter provides, and a fallback for the oldest interpreter the package supports. Only one of the two can run on any given interpreter, so the only way to test the other is to lie to the module about which interpreter it is on -- -:meth:`CompatTests.load_compat_as_python35` executes the file a second time with -:data:`sys.version_info` faked to ``(3, 5)``. +:meth:`CompatTests.load_compat_as` executes the file a second time with +:data:`sys.version_info` faked to ``(3, 5)``. ``default_threading_excepthook`` +has a third, for 3.8 and 3.9 only, which a fake ``(3, 9)`` reaches instead. That lie is not free, and containing its cost is most of what this module is about. A fake version is process-global for as long as it is installed, so *any* @@ -28,7 +29,7 @@ #. :data:`WARM_BEFORE_FAKING` is imported *before* the fake goes in, so the memoised value is the true one, and - :meth:`CompatTests.load_compat_as_python35` **asserts** that nothing at all + :meth:`CompatTests.load_compat_as` **asserts** that nothing at all was first-imported under the fake -- the ordering is checked rather than assumed. #. :meth:`CompatTests.assert_aenum_is_not_poisoned` runs as a cleanup on every @@ -53,11 +54,14 @@ import decimal import enum import importlib +import subprocess import sys +import textwrap import unittest from unittest import mock from tests._support import load_module, purge_modules, reimport_once_per_class +from tests._tiers import ROOT # NOTE: :mod:`aenum` is deliberately *not* imported at module scope, however # convenient that would be for the assertions below. A module-level import would @@ -96,7 +100,7 @@ #: #: The rest are here because the faked branches import them too. Whether each one #: memoises anything version-dependent is deliberately *not* the question: the -#: invariant :meth:`CompatTests.load_compat_as_python35` asserts is that nothing +#: invariant :meth:`CompatTests.load_compat_as` asserts is that nothing #: whatever is first-imported under the fake, which needs no judgement about #: which third-party caches are dangerous. A future interpreter or :mod:`aenum` #: release that pulls in one more module fails that assertion here, at the line @@ -106,8 +110,10 @@ #: running the test until it stopped complaining, and that distinction is the #: point of having it. Every import statement on a branch reachable at ``(3, 5)``: #: :mod:`collections.abc` (line 34), :mod:`pathlib` (63), :mod:`threading` (67), -#: :mod:`typing` (4, 68, 126), :mod:`aenum` (143), :mod:`typing_extensions` (148, -#: 202), :mod:`contextlib` (153), :mod:`decimal` (154), :mod:`enum` (179). +#: :mod:`typing` (4, 68, 126), :mod:`aenum` (154), :mod:`typing_extensions` (172, +#: 226), :mod:`contextlib` (177), :mod:`decimal` (178), :mod:`enum` (203). A fake +#: ``(3, 9)`` reaches three more: :mod:`builtins` (31), :mod:`functools` (123) and +#: :mod:`_thread` (251). #: #: Four of those never actually surface, and *why* is the point. Three -- #: :mod:`collections.abc`, :mod:`contextlib` and :mod:`pathlib` -- are imported at @@ -118,9 +124,11 @@ #: why the four are on the list despite costing a :data:`sys.modules` lookup each #: and nothing else. Measured: with only the five that do surface warmed and #: :mod:`pathlib` forced cold, the window leaks ``['pathlib', 'pathlib._os']``; -#: with the full list it is empty even with five of the nine forced cold. -WARM_BEFORE_FAKING = ('aenum', 'collections.abc', 'contextlib', 'decimal', 'enum', - 'pathlib', 'threading', 'typing', 'typing_extensions') +#: with the full list, then nine names, it is empty even with five of them forced +#: cold. +WARM_BEFORE_FAKING = ('_thread', 'aenum', 'builtins', 'collections.abc', 'contextlib', + 'decimal', 'enum', 'functools', 'pathlib', 'threading', 'typing', + 'typing_extensions') #: Prefix of the names the loaders themselves write, which are excluded from the #: "nothing was imported under the fake" check. :func:`tests._support.load_module` @@ -179,7 +187,7 @@ def setUp(self) -> None: # ended -- including when it ended by raising. It is a sweep of a handful # of module attributes, so paying it on the tests that fake nothing costs # nothing and covers the case where a future test in this file learns to - # fake a version without going through ``load_compat_as_python35``. + # fake a version without going through ``load_compat_as``. self.addCleanup(self.assert_aenum_is_not_poisoned) self.compat = load_module('pcapkit.utilities.compat', 'pcapkit/utilities/compat.py') @@ -212,15 +220,15 @@ def assert_aenum_is_not_poisoned(self) -> None: f'and raises AttributeError. Nothing undoes this: restoring sys.version_info ' f'does not un-memoise a value. See issue #687 and WARM_BEFORE_FAKING.') - def load_compat_as_python35(self, module_name: str) -> 'object': - """Execute :file:`pcapkit/utilities/compat.py` as if on Python 3.5. + def load_compat_as(self, module_name: str, version: 'tuple[int, int]') -> 'object': + """Execute :file:`pcapkit/utilities/compat.py` as if on an older Python. Two things have to hold for that to be safe, and the second is the whole of issue #687. - The interpreter has to *look* like 3.5 while the module body runs, which - is what the :func:`~unittest.mock.patch.object` here does. And nothing may - be imported for the first time while it looks that way, because an + The interpreter has to *look* like ``version`` while the module body runs, + which is what the :func:`~unittest.mock.patch.object` here does. And nothing + may be imported for the first time while it looks that way, because an import-time cache of the faked value is not something restoring :data:`sys.version_info` can undo -- the value is already memoised inside a module this suite does not own. @@ -236,19 +244,21 @@ def load_compat_as_python35(self, module_name: str) -> 'object': Args: module_name: Dotted name to execute the file under. Must not be the - name the real module holds, or the 3.5-flavoured module would be - bound over it. + name the real module holds, or the faked module would be bound + over it. + version: The :data:`sys.version_info` to fake. ``(3, 5)`` takes + every fallback at once. Returns: - The executed module, with every ``< 3.6`` fallback in place of the - implementation this interpreter would otherwise have supplied. + The executed module, with each fallback ``version`` reaches in place + of the implementation this interpreter would otherwise have supplied. """ for name in WARM_BEFORE_FAKING: importlib.import_module(name) before = frozenset(sys.modules) - with mock.patch.object(sys, 'version_info', (3, 5)): + with mock.patch.object(sys, 'version_info', version): compat = load_module(module_name, 'pcapkit/utilities/compat.py') leaked = sorted( @@ -258,7 +268,7 @@ def load_compat_as_python35(self, module_name: str) -> 'object': self.assertEqual( leaked, [], f'{leaked} were imported for the first time while sys.version_info was ' - f'faked to (3, 5), so whatever any of them memoised about the interpreter ' + f'faked to {version!r}, so whatever any of them memoised about the interpreter ' f'at import time is now wrong for the rest of the process -- and restoring ' f'sys.version_info does not undo it. Add them to WARM_BEFORE_FAKING so ' f'they are imported under the real version first. See issue #687.') @@ -294,6 +304,40 @@ class DemoFlag(enum.IntFlag): self.assertEqual(self.compat.show_flag_values(DemoFlag.READ | DemoFlag.EXEC), [1, 4]) + def test_default_threading_excepthook_is_the_hook_the_interpreter_started_with(self) -> None: + """GitHub issue #1691: the real default, even once a host hook replaced it. + + :data:`threading.excepthook` is only the default until something replaces + it, so a fresh interpreter reads it first, then installs a hook of its own, + and only then runs :file:`pcapkit/utilities/compat.py` -- by path, so no + other part of :mod:`pcapkit` is imported. On 3.8 and 3.9 this is what pins + ``_thread._excepthook`` over whatever the hook holds at import. + + """ + code = textwrap.dedent(''' + import importlib.util + import sys + import threading + + STARTUP = threading.excepthook if sys.version_info >= (3, 8) else None + + def host_hook(args): + pass + threading.excepthook = host_hook + + spec = importlib.util.spec_from_file_location('compat_probe', sys.argv[1]) + compat = importlib.util.module_from_spec(spec) + spec.loader.exec_module(compat) + print('IS_STARTUP:', compat.default_threading_excepthook is STARTUP) + print('IS_HOST:', compat.default_threading_excepthook is host_hook) + ''') + proc = subprocess.run( + [sys.executable, '-c', code, str(ROOT / 'pcapkit' / 'utilities' / 'compat.py')], + capture_output=True, text=True, timeout=60) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn('IS_STARTUP: True', proc.stdout) + self.assertIn('IS_HOST: False', proc.stdout) + def test_faking_the_version_leaves_aenum_reading_the_real_one(self) -> None: """The regression test for issue #687. @@ -321,7 +365,7 @@ def test_faking_the_version_leaves_aenum_reading_the_real_one(self) -> None: """ purge_modules(['pcapkit.utilities.compat_py35_probe']) - self.load_compat_as_python35('pcapkit.utilities.compat_py35_probe') + self.load_compat_as('pcapkit.utilities.compat_py35_probe', (3, 5)) # Not vacuous: the branch just executed did ``from aenum import StrEnum``, # so aenum is in sys.modules by now whether or not the warm-up put it @@ -343,9 +387,10 @@ class TransportProtocolLike(aenum.IntFlag): def test_python35_fallback_implementations(self) -> None: purge_modules(['pcapkit.utilities.compat_py35']) - compat = self.load_compat_as_python35('pcapkit.utilities.compat_py35') + compat = self.load_compat_as('pcapkit.utilities.compat_py35', (3, 5)) self.assertTrue(issubclass(compat.ModuleNotFoundError, ImportError)) + self.assertIsNone(compat.default_threading_excepthook) class CollectionLike: def __len__(self): return 0 @@ -418,6 +463,21 @@ class OldFlag(enum.IntFlag): with self.assertRaises(ValueError): compat.show_flag_values(-1) + def test_python39_default_threading_excepthook_is_the_310_one(self) -> None: + """The 3.8 and 3.9 branch lands on the object 3.10 and later name. + + ``_thread._excepthook`` still exists from 3.10 on, so faking ``(3, 9)`` + runs that branch on those interpreters too, where it must agree with + :data:`threading.__excepthook__` -- the identity the branch rests on. On + 3.8 and 3.9 both loads take the same branch. + + """ + purge_modules(['pcapkit.utilities.compat_py39']) + compat = self.load_compat_as('pcapkit.utilities.compat_py39', (3, 9)) + + self.assertIs(compat.default_threading_excepthook, + self.compat.default_threading_excepthook) + if __name__ == '__main__': unittest.main() diff --git a/tests/utilities/test_exceptions_excepthook.py b/tests/utilities/test_exceptions_excepthook.py index fcf3e91741..0c1ee6c292 100644 --- a/tests/utilities/test_exceptions_excepthook.py +++ b/tests/utilities/test_exceptions_excepthook.py @@ -35,6 +35,8 @@ """ from __future__ import annotations +import contextlib +import io import os import subprocess import sys @@ -62,8 +64,18 @@ def run_in_subprocess(code: 'str', devmode: 'bool' = False) -> 'subprocess.Compl actually proves something about the tree under review rather than about whatever happens to be on ``sys.path`` by default. + It also binds ``DEFAULT_THREADING_HOOK`` to the interpreter's own default + :data:`threading.excepthook`. :data:`threading.__excepthook__` names it only + from Python 3.10 (GitHub issue #1691); before that it is the hook a fresh + interpreter starts with, so it is read before ``import pcapkit``, whose + ``tbtrim`` rule replaces it. + """ preamble = textwrap.dedent(f''' + import sys as _sys + import threading as _threading + DEFAULT_THREADING_HOOK = (_threading.__excepthook__ if _sys.version_info >= (3, 10) + else _threading.excepthook) import pcapkit assert pcapkit.__file__.startswith({REPO_ROOT!r}), ( 'wrong tree', pcapkit.__file__, {REPO_ROOT!r}) @@ -543,7 +555,7 @@ def boom(): raise error captured = io.StringIO() saved_hook = threading.excepthook - threading.excepthook = threading.__excepthook__ + threading.excepthook = DEFAULT_THREADING_HOOK sys.tracebacklimit = 0 try: t = threading.Thread(target=boom, name=name) @@ -603,7 +615,7 @@ def boom(): raise error captured = io.StringIO() saved_hook = threading.excepthook - threading.excepthook = threading.__excepthook__ + threading.excepthook = DEFAULT_THREADING_HOOK sys.tracebacklimit = 0 try: t = threading.Thread(target=boom, name=name) @@ -648,7 +660,7 @@ def test_a_non_base_error_on_a_thread_is_delegated_to_the_previous_hook(self) -> def previous_hook(args): print('PREVIOUS_THREADING_HOOK_RAN', file=sys.stderr) - threading.__excepthook__(args) + DEFAULT_THREADING_HOOK(args) threading.excepthook = previous_hook try: @@ -684,7 +696,7 @@ def test_installing_twice_does_not_wrap_the_threading_hook_either(self) -> None: def previous_hook(args): print('PREVIOUS_THREADING_HOOK_RAN', file=sys.stderr) - threading.__excepthook__(args) + DEFAULT_THREADING_HOOK(args) threading.excepthook = previous_hook exc._install_excepthook() @@ -729,6 +741,74 @@ def exploding_delegate(args): self.assertEqual(proc.returncode, 0, proc.stderr) self.assertEqual(int(proc.stdout.strip()), 1) + def test_a_non_base_error_with_no_previous_hook_goes_to_the_default_hook(self) -> None: + """Nothing captured yet: the default hook's own output, on every Python. + + GitHub issue #1691: this delegate was ``threading.__excepthook__``, + which Python 3.8 and 3.9 lack, so the call raised instead of printing. + + """ + code = ''' + import io + import sys + import threading + import contextlib + import pcapkit.utilities.exceptions as exc + exc.logger.disabled = True + assert exc._previous_threading_excepthook is None, 'precondition' + + try: + raise ValueError('no previous hook') + except ValueError: + etype, value, tb = sys.exc_info() + args = threading.ExceptHookArgs([etype, value, tb, threading.current_thread()]) + + def output(hook): + captured = io.StringIO() + with contextlib.redirect_stderr(captured): + hook(args) + return captured.getvalue() + + expected = output(DEFAULT_THREADING_HOOK) + actual = output(exc._threading_excepthook) + print('MATCH' if actual == expected else 'MISMATCH') + print(repr(expected)) + print(repr(actual)) + ''' + proc = run_in_subprocess(code) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(proc.stdout.splitlines()[0], 'MATCH', proc.stdout) + self.assertIn('ValueError: no previous hook', proc.stdout) + + def test_the_fallback_is_the_interpreter_default_despite_an_earlier_hook(self) -> None: + """A hook installed before pcapkit is imported is not mistaken for the default. + + On 3.10 and later that is :data:`threading.__excepthook__` for free; + on 3.8 and 3.9 it must not come from whatever :data:`threading.excepthook` + held at import, or the two would disagree. + + """ + code = ''' + import sys + import threading + + def host_hook(args): + pass + threading.excepthook = host_hook + + for name in [name for name in sys.modules + if name == 'pcapkit' or name.startswith('pcapkit.')]: + del sys.modules[name] + import pcapkit.utilities.exceptions as exc + + print('IS_DEFAULT:', exc.default_threading_excepthook is DEFAULT_THREADING_HOOK) + print('IS_HOST:', exc.default_threading_excepthook is host_hook) + ''' + proc = run_in_subprocess(code) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn('IS_DEFAULT: True', proc.stdout) + self.assertIn('IS_HOST: False', proc.stdout) + def test_two_threads_raising_concurrently_do_not_confuse_each_others_guard(self) -> None: """The per-thread (not shared) re-entrancy flag, under real concurrency. @@ -789,5 +869,51 @@ def worker(name): self.assertIn('T1 completed without raising', proc.stdout) +class ThreadingDefaultHookInProcessTests(unittest.TestCase): + """Both fallbacks to the default thread hook, run where coverage can see them. + + GitHub issue #1691. The subprocess tests above compare the printed output + exactly; these drive the same two branches of + :func:`~pcapkit.utilities.exceptions._threading_excepthook` in this process, + on a freshly loaded module that has never captured a previous hook. + + """ + + def setUp(self) -> None: + self._saved_devmode = os.environ.get('PCAPKIT_DEVMODE') + self.exc = bootstrap(devmode=False)['exceptions'] + try: + raise ValueError('in-process default hook') + except ValueError: + etype, value, tb = sys.exc_info() + self.args = threading.ExceptHookArgs([etype, value, tb, threading.current_thread()]) + + def tearDown(self) -> None: + if self._saved_devmode is None: + os.environ.pop('PCAPKIT_DEVMODE', None) + else: + os.environ['PCAPKIT_DEVMODE'] = self._saved_devmode + + def assert_hook_prints_the_default_output(self) -> None: + captured = io.StringIO() + with contextlib.redirect_stderr(captured): + self.exc._threading_excepthook(self.args) + output = captured.getvalue() + self.assertIn(f'Exception in thread {threading.current_thread().name}:', output) + self.assertIn('ValueError: in-process default hook', output) + + def test_with_no_previous_hook_a_non_base_error_goes_to_the_default(self) -> None: + self.assertIsNone(self.exc._previous_threading_excepthook, + 'precondition: a fresh module has captured nothing') + self.assert_hook_prints_the_default_output() + + def test_on_reentry_the_hook_falls_back_to_the_default(self) -> None: + self.exc._threading_hook_state.running = True + try: + self.assert_hook_prints_the_default_output() + finally: + self.exc._threading_hook_state.running = False + + if __name__ == '__main__': unittest.main()