From 3e433b8a88c9ede3ba1aba4d788d52b2b96c7e31 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 19 Oct 2026 20:25:45 -0400 Subject: [PATCH 1/2] fix(corekit): let _py_abc cache its ABC lookups on sentinel types (#1960) - SentinelMeta.__setattr__ now lets a frozen sentinel type rebind _abc_negative_cache and _abc_negative_cache_version, and only where the type's own __dict__ already holds them, which only _py_abc puts there. Without the C _abc module, isinstance against a sentinel type raised TypeError. Every other write and every delete is still refused. - New test runs the checks in a child interpreter that really falls back to _py_abc, and pins that the names stay refused under the C module. - The SentinelMeta docstring and the sentinel convention page name the exception. Closes #1960 --- .../conventions/sentinel-convention.rst | 5 +- pcapkit/corekit/sentinels.py | 22 +- .../corekit/test_sentinel_py_abc_1960_unit.py | 198 ++++++++++++++++++ 3 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 tests/corekit/test_sentinel_py_abc_1960_unit.py diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index d721315414..e088b73c04 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -162,7 +162,10 @@ Where the sentinels differ, the differences are **needs, not inconsistencies**: name, as ``None.x = 1`` does. ``__slots__`` is empty, so there is no ``__dict__`` to reach round them either. The metaclass does the same for the type, raising :exc:`TypeError` as ``type(None).x = 1`` does, so neither ``_name`` nor the cached - instance can be swapped out. It also refuses to let a concrete sentinel type be + instance can be swapped out. It lets through only ``_abc_negative_cache`` and + ``_abc_negative_cache_version``, which ``_py_abc``, the fallback :mod:`abc` uses + without the C ``_abc`` module, rebinds during an :func:`isinstance` check + (:issue:`1960`). It also refuses to let a concrete sentinel type be subclassed, by a ``class`` statement or by ``register``, as ``type(None)`` cannot be; :class:`~pcapkit.corekit.sentinels.Sentinel` itself stays subclassable, being abstract. The owner asked for this in review of the change for :issue:`1842`: a sentinel's repr diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 1abb27e187..4b0aec0a12 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -57,6 +57,17 @@ #: every attribute write. _FROZEN = '_frozen' +#: frozenset[str]: The cache names ``_py_abc``, the pure-Python fallback +#: :mod:`abc` uses where the C ``_abc`` module is missing, rebinds on a class +#: while an :func:`isinstance` or :func:`issubclass` check runs against it +#: (:issue:`1960`). A frozen sentinel type still lets these through, and only +#: where its own :attr:`~object.__dict__` already holds the name, which only +#: that fallback's :meth:`~abc.ABCMeta.__new__` puts there. Each can only make +#: a check answer "no" or look again, never "yes". ``_abc_registry`` and +#: ``_abc_cache`` are left out, being set only while the type is built, and +#: replacing either could make a check answer "yes". +_PY_ABC_CACHE = frozenset({'_abc_negative_cache', '_abc_negative_cache_version'}) + def _not_a_base(cls: 'type') -> 'TypeError': """The error deriving from ``cls`` raises, worded as for ``type(None)``.""" @@ -84,7 +95,10 @@ class SentinelMeta(abc.ABCMeta): * Every sentinel type is immutable once its ``class`` statement has run. Setting or deleting any attribute on it raises :exc:`TypeError`, as ``type(None).x = 1`` does, so ``_name``, ``_instance`` and the methods - every sentinel inherits stay as they were defined. + every sentinel inherits stay as they were defined. The one exception is + ``_abc_negative_cache`` and ``_abc_negative_cache_version``, which + ``_py_abc``, the fallback :mod:`abc` uses without the C ``_abc`` + module, rebinds during an :func:`isinstance` check (:issue:`1960`). A concrete sentinel type also gets ``__final__`` set to :obj:`True` here. :func:`~pcapkit.utilities.compat.final` would record the same, but a class @@ -139,10 +153,12 @@ def __setattr__(cls, name: 'str', value: 'Any') -> 'None': value: Attribute value. Raises: - TypeError: Once the ``class`` statement of ``cls`` has run. + TypeError: Once the ``class`` statement of ``cls`` has run, unless + ``name`` is one of the ``_py_abc`` caches in ``_PY_ABC_CACHE`` + that ``cls`` already holds. """ - if cls.__dict__.get(_FROZEN): + if cls.__dict__.get(_FROZEN) and not (name in _PY_ABC_CACHE and name in cls.__dict__): raise TypeError(f'cannot set {name!r} attribute of immutable type {cls.__name__!r}') super().__setattr__(name, value) diff --git a/tests/corekit/test_sentinel_py_abc_1960_unit.py b/tests/corekit/test_sentinel_py_abc_1960_unit.py new file mode 100644 index 0000000000..2732e2847f --- /dev/null +++ b/tests/corekit/test_sentinel_py_abc_1960_unit.py @@ -0,0 +1,198 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1960: a sentinel type answers :func:`isinstance` under ``_py_abc``. + +Where the C ``_abc`` module is missing, as on GraalPy, :mod:`abc` falls back to +``_py_abc``, whose ``__subclasscheck__`` rebinds ``_abc_negative_cache`` and +``_abc_negative_cache_version`` on the class once any ABC has registered a +virtual subclass since the class was built. :class:`~pcapkit.corekit.sentinels.SentinelMeta` +refused every write to a built sentinel type, so ``isinstance(1, NullType)`` +raised :exc:`TypeError` there. It now lets those two names through, and no +other. + +:class:`PyABCFallbackTests` runs the checks in a child interpreter that really +takes the fallback. It starts with ``-S``, so that no module but :mod:`io` has +imported :mod:`abc` yet, drops both from :data:`sys.modules`, blocks ``_abc`` +and imports them again. The child reports whether :class:`abc.ABCMeta` is +``_py_abc``'s and whether ``_collections_abc`` was already loaded, rather than +trusting the block. :class:`CModuleTests` pins that, under the C module, the +names let through for ``_py_abc`` are still refused. + +""" + +from __future__ import annotations + +import _py_abc +import abc +import importlib +import json +import os +import subprocess # nosec B404 +import sys +import textwrap +import unittest + +from tests._support import reimport_once_per_class + +#: Repository root, put first on the child interpreter's path so that it +#: imports this tree rather than whatever :mod:`pcapkit` is installed. +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +#: Every sentinel type, the abstract base first, and the module-level name of +#: each concrete one's instance. +TYPES = ( + ('Sentinel', None), + ('NullType', 'NULL'), + ('NoValueType', 'NO_VALUE'), + ('NoDefaultType', 'NO_DEFAULT'), + ('AbsentType', 'ABSENT'), +) + +#: Run in the child. It prints one JSON object: whether the fallback was taken, +#: and for each sentinel type the outcome of every probe, as the ``repr`` of +#: what it returned or the name of the exception it raised. +CHILD = textwrap.dedent(''' + import sys + + preloaded = sorted(name for name in ('abc', 'io', '_collections_abc') if name in sys.modules) + for name in ('abc', 'io'): + sys.modules.pop(name, None) + sys.modules['_abc'] = None + + import abc + import io # noqa: F401 # rebuilt on the fallback before anything uses it + import json + import _py_abc + + import pcapkit + from pcapkit.corekit import sentinels + + class Probe(abc.ABC): + """An ABC whose registrations invalidate every ``_py_abc`` negative cache.""" + + def bump(): + """Register a fresh virtual subclass, so the next check rebinds the cache.""" + Probe.register(type('Virtual', (), {})) + + def outcome(probe): + try: + return repr(probe()) + except Exception as error: + return type(error).__name__ + + report = { + 'preloaded': preloaded, + 'fallback': abc.ABCMeta is _py_abc.ABCMeta, + 'metaclass_base': sentinels.SentinelMeta.__mro__[1] is _py_abc.ABCMeta, + 'file': pcapkit.__file__, + 'types': {}, + } + for name, instance_name in json.loads(sys.argv[1]): + type_ = getattr(sentinels, name) + instance = None if instance_name is None else getattr(sentinels, instance_name) + probes = report['types'][name] = {} + bump() + probes['isinstance(1)'] = outcome(lambda: isinstance(1, type_)) + bump() + probes['issubclass(int)'] = outcome(lambda: issubclass(int, type_)) + bump() + probes['isinstance(instance)'] = outcome(lambda: isinstance(instance, type_)) + probes['isinstance(instance, Sentinel)'] = outcome( + lambda: isinstance(instance, sentinels.Sentinel)) + probes['register(int)'] = outcome(lambda: type_.register(int)) + for attr in ('x', '_name', '_instance', '_abc_cache', '_abc_registry'): + probes['set ' + attr] = outcome(lambda: setattr(type_, attr, None)) + for attr in ('x', '_abc_negative_cache', '_abc_negative_cache_version'): + probes['delete ' + attr] = outcome(lambda: delattr(type_, attr)) + if instance is not None: + probes['_name'] = outcome(lambda: type_.__dict__['_name']) + print(json.dumps(report)) +''') + + +def _expected(instance_name: 'str | None') -> 'dict[str, str]': + """The outcome of every probe :data:`CHILD` runs against a sentinel type. + + Args: + instance_name: Module-level name of the type's one instance, or + :obj:`None` for the abstract base. + + """ + concrete = instance_name is not None + probes = { + 'isinstance(1)': 'False', + 'issubclass(int)': 'False', + 'isinstance(instance)': repr(concrete), + 'isinstance(instance, Sentinel)': repr(concrete), + # Only a concrete type refuses; the abstract base registers ``int``, + # after every check against it has run. + 'register(int)': 'TypeError' if concrete else repr(int), + } + if concrete: + probes['_name'] = repr(instance_name) + for attr in ('x', '_name', '_instance', '_abc_cache', '_abc_registry'): + probes['set ' + attr] = 'TypeError' + for attr in ('x', '_abc_negative_cache', '_abc_negative_cache_version'): + probes['delete ' + attr] = 'TypeError' + return probes + + +class PyABCFallbackTests(unittest.TestCase): + """Every sentinel type behaves under ``_py_abc`` as it does under ``_abc``.""" + + report = {} # type: dict + + @classmethod + def setUpClass(cls) -> 'None': + path = [REPO_ROOT] + [entry for entry in sys.path if entry] + env = dict(os.environ, PCAPKIT_DEVMODE='0', PYTHONSAFEPATH='1', + PYTHONPATH=os.pathsep.join(path)) + proc = subprocess.run([sys.executable, '-S', '-c', CHILD, json.dumps(TYPES)], # nosec B603 + cwd=REPO_ROOT, env=env, capture_output=True, text=True, + timeout=120, check=False) + if proc.returncode != 0: + raise AssertionError(f'child interpreter failed:\n{proc.stderr}') + cls.report = json.loads(proc.stdout.strip().splitlines()[-1]) + + def test_the_child_really_takes_the_fallback(self) -> 'None': + """:mod:`abc` is ``_py_abc``'s, and nothing else had imported it at startup.""" + self.assertTrue(self.report['fallback'], 'abc.ABCMeta is not _py_abc.ABCMeta') + self.assertTrue(self.report['metaclass_base'], 'SentinelMeta is not built on _py_abc') + self.assertNotIn('_collections_abc', self.report['preloaded']) + self.assertTrue(self.report['file'].startswith(REPO_ROOT), self.report['file']) + + def test_every_probe_behaves_as_under_the_c_module(self) -> 'None': + """Each check answers as it should, and every other write is still refused.""" + for name, instance_name in TYPES: + expected = _expected(instance_name) + actual = self.report['types'][name] + with self.subTest(type=name): + self.assertEqual(sorted(actual), sorted(expected)) + for probe, outcome in expected.items(): + with self.subTest(type=name, probe=probe): + self.assertEqual(actual[probe], outcome) + + +class CModuleTests(unittest.TestCase): + """Under the C ``_abc`` module, the names let through for ``_py_abc`` stay refused.""" + + def setUp(self) -> 'None': + reimport_once_per_class(self) + + def test_the_py_abc_cache_names_are_refused_where_the_type_lacks_them(self) -> 'None': + """Neither ``_abc_negative_cache`` nor its version can be set on a type that lacks it.""" + sentinels = importlib.import_module('pcapkit.corekit.sentinels') + if abc.ABCMeta is _py_abc.ABCMeta: + self.skipTest('abc is the _py_abc fallback here; PyABCFallbackTests covers it') + for name, _ in TYPES: + type_ = getattr(sentinels, name) + for attr in ('_abc_negative_cache', '_abc_negative_cache_version'): + with self.subTest(type=name, attr=attr): + self.assertNotIn(attr, type_.__dict__) + message = f'{attr!r} attribute of immutable type {name!r}' + with self.assertRaisesRegex(TypeError, message): + setattr(type_, attr, None) + self.assertNotIn(attr, type_.__dict__) + + +if __name__ == '__main__': + unittest.main() From a5cc7084242d51fb5c7892b744efee247755f0ce Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 19 Oct 2026 20:42:30 -0400 Subject: [PATCH 2/2] docs(corekit): write the ABCMeta.__new__ reference as a literal (#1960) - The _PY_ABC_CACHE comment, the SentinelMeta docstring and the sentinel convention page name abc, _py_abc and ABCMeta.__new__ as literals rather than roles, since ABCMeta.__new__ is not in the Python inventory. --- .../source/contributing/conventions/sentinel-convention.rst | 2 +- pcapkit/corekit/sentinels.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index e088b73c04..a1cf4f1291 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -163,7 +163,7 @@ Where the sentinels differ, the differences are **needs, not inconsistencies**: reach round them either. The metaclass does the same for the type, raising :exc:`TypeError` as ``type(None).x = 1`` does, so neither ``_name`` nor the cached instance can be swapped out. It lets through only ``_abc_negative_cache`` and - ``_abc_negative_cache_version``, which ``_py_abc``, the fallback :mod:`abc` uses + ``_abc_negative_cache_version``, which ``_py_abc``, the fallback ``abc`` uses without the C ``_abc`` module, rebinds during an :func:`isinstance` check (:issue:`1960`). It also refuses to let a concrete sentinel type be subclassed, by a ``class`` statement or by ``register``, as ``type(None)`` cannot be; diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 4b0aec0a12..eea62b1fdd 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -58,11 +58,11 @@ _FROZEN = '_frozen' #: frozenset[str]: The cache names ``_py_abc``, the pure-Python fallback -#: :mod:`abc` uses where the C ``_abc`` module is missing, rebinds on a class +#: ``abc`` uses where the C ``_abc`` module is missing, rebinds on a class #: while an :func:`isinstance` or :func:`issubclass` check runs against it #: (:issue:`1960`). A frozen sentinel type still lets these through, and only #: where its own :attr:`~object.__dict__` already holds the name, which only -#: that fallback's :meth:`~abc.ABCMeta.__new__` puts there. Each can only make +#: that fallback's ``ABCMeta.__new__`` puts there. Each can only make #: a check answer "no" or look again, never "yes". ``_abc_registry`` and #: ``_abc_cache`` are left out, being set only while the type is built, and #: replacing either could make a check answer "yes". @@ -97,7 +97,7 @@ class SentinelMeta(abc.ABCMeta): ``type(None).x = 1`` does, so ``_name``, ``_instance`` and the methods every sentinel inherits stay as they were defined. The one exception is ``_abc_negative_cache`` and ``_abc_negative_cache_version``, which - ``_py_abc``, the fallback :mod:`abc` uses without the C ``_abc`` + ``_py_abc``, the fallback ``abc`` uses without the C ``_abc`` module, rebinds during an :func:`isinstance` check (:issue:`1960`). A concrete sentinel type also gets ``__final__`` set to :obj:`True` here.