From 4998b959a774da534657f3d2da2f67120ea2dfcd Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 4 Nov 2026 13:00:12 -0500 Subject: [PATCH 1/2] fix(utilities): serialise first resolution of lazy exports and module descriptors so concurrent imports cannot deadlock (#2771) - Threads making first imports at once raised _DeadlockError: a family member's protocol module imports a module of its base's package, and the import system takes that module's lock before waiting on the base's package, while a thread importing the package takes the two the other way. - pcapkit.utilities.importlib.import_lock, a shared RLock, is held by every first resolution in lazy_exports and by ModuleDescriptor.klass when its module is not imported yet; resolved names and imported modules are read without it. - IGMP's base and version classes on the dispatch path resolve through descriptors rather than function-level imports, which bypassed the lock. - ProtocolBase._registry_lock goes (#2773): get_protocol resolves through the lock. - The ALL-tier race test covers lazy names, family-member dispatch and get_protocol, and fails on any first import made without the lock; it fails on the unfixed tree. --- docs/source/pcapkit/utilities/importlib.rst | 9 +- pcapkit/corekit/module.py | 14 +- pcapkit/protocols/internet/igmp/protocol.py | 19 +- pcapkit/protocols/internet/internet.py | 9 +- pcapkit/protocols/protocol.py | 10 +- pcapkit/utilities/importlib.py | 39 ++- .../test_lazy_exports_threads_2771_unit.py | 242 ++++++++++++++++++ 7 files changed, 316 insertions(+), 26 deletions(-) create mode 100644 tests/utilities/test_lazy_exports_threads_2771_unit.py diff --git a/docs/source/pcapkit/utilities/importlib.rst b/docs/source/pcapkit/utilities/importlib.rst index cfc1446245..64632aeb13 100644 --- a/docs/source/pcapkit/utilities/importlib.rst +++ b/docs/source/pcapkit/utilities/importlib.rst @@ -7,9 +7,14 @@ Lazy Imports :pep:`562` ``__getattr__`` and ``__dir__`` that import each name it exports the first time the name is reached, rather than with the package itself. Every package under :mod:`pcapkit.const` imports its enumerations that way (:issue:`2729`), so a caller -pays only for the enumerations it uses. This module imports nothing from -:mod:`pcapkit`; see :ref:`utilities-or-corekit`. +pays only for the enumerations it uses. Those first imports, and that of a +:class:`~pcapkit.corekit.module.ModuleDescriptor`, hold :data:`import_lock`, one +thread at a time (:issue:`2771`). This module imports nothing from :mod:`pcapkit`; +see :ref:`utilities-or-corekit`. .. versionadded:: 1.6.0 .. autofunction:: pcapkit.utilities.importlib.lazy_exports + +.. autodata:: pcapkit.utilities.importlib.import_lock + :no-value: diff --git a/pcapkit/corekit/module.py b/pcapkit/corekit/module.py index c0ca7d2644..d61a7a0162 100644 --- a/pcapkit/corekit/module.py +++ b/pcapkit/corekit/module.py @@ -16,6 +16,7 @@ from pcapkit.corekit.sentinels import NULL from pcapkit.utilities.exceptions import ProtocolError +from pcapkit.utilities.importlib import import_lock __all__ = ['ModuleDescriptor'] @@ -83,6 +84,13 @@ def klass(self) -> 'Type[_T]': pre-reload class forever, and an instance of it fails :func:`isinstance` against the live one. + A first import, of a module not in :data:`sys.modules` yet, holds + :data:`~pcapkit.utilities.importlib.import_lock`, as the first imports + of :func:`~pcapkit.utilities.importlib.lazy_exports` do: threads + dispatching at once to protocols not imported yet otherwise deadlock + the import system (:issue:`2771`). A module already imported, or + part way through another thread's import, is read without it. + """ if self.name is NULL: # ``module`` is a ``str`` and the caller never named a class -- @@ -110,7 +118,11 @@ def klass(self) -> 'Type[_T]': # from the ``getattr`` below instead, with the same message. pass try: - return getattr(importlib.import_module(self.module), name) + if self.module in sys.modules: + return getattr(importlib.import_module(self.module), name) + # NOTE: the lock lazy_exports holds, so racing first imports cannot deadlock (#2771). + with import_lock: + return getattr(importlib.import_module(self.module), name) except AttributeError as error: # GitHub issue #832: a ``register_*`` helper that builds a # descriptor from a bad class name fails here with a bare stdlib diff --git a/pcapkit/protocols/internet/igmp/protocol.py b/pcapkit/protocols/internet/igmp/protocol.py index 47e4ae2989..31c3e351bd 100644 --- a/pcapkit/protocols/internet/igmp/protocol.py +++ b/pcapkit/protocols/internet/igmp/protocol.py @@ -31,6 +31,7 @@ from pcapkit.const.igmp.type import Type as Enum_Type from pcapkit.const.reg.transtype import TransType as Enum_TransType +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.internet.igmp.data import IGMP as Data_IGMP from pcapkit.protocols.internet.igmp.schema import IGMP as Schema_IGMP from pcapkit.protocols.internet.internet import Internet @@ -50,6 +51,16 @@ __all__ = ['IGMP'] +#: tuple[tuple[int, ModuleDescriptor[IGMP]], ...]: Each version and the class +#: :meth:`IGMP._member_of_version` picks for it, resolved there rather than imported, +#: so a version's first import holds :data:`~pcapkit.utilities.importlib.import_lock` +#: (:issue:`2771`). +_MEMBERS = ( + (1, ModuleDescriptor('pcapkit.protocols.internet.igmpv1.protocol', 'IGMPv1')), + (2, ModuleDescriptor('pcapkit.protocols.internet.igmpv2.protocol', 'IGMPv2')), + (3, ModuleDescriptor('pcapkit.protocols.internet.igmpv3.protocol', 'IGMPv3')), +) # type: tuple[tuple[int, ModuleDescriptor[IGMP]], ...] + class IGMP(Internet[_PT, _ST], Generic[_PT, _ST], schema=Schema_IGMP, data=Data_IGMP): @@ -473,12 +484,8 @@ def _member_of_version(version: 'Any') -> 'Type[IGMP]': where it equals one. """ - from pcapkit.protocols.internet.igmpv1.protocol import IGMPv1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.internet.igmpv2.protocol import IGMPv2 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.internet.igmpv3.protocol import IGMPv3 # isort: skip # pylint: disable=import-outside-toplevel - if isinstance(version, int) and not isinstance(version, bool): - for number, member in ((1, IGMPv1), (2, IGMPv2), (3, IGMPv3)): + for number, member in _MEMBERS: if version == number: - return cast('Type[IGMP]', member) + return member.klass raise ProtocolError(f'IGMP: invalid version: {version!r}') diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index b83a930e85..c05b31c717 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -35,6 +35,7 @@ from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.data import Data + from pcapkit.protocols.internet.igmp.protocol import IGMP __all__ = ['Internet'] @@ -44,6 +45,12 @@ #: it while parsing, see :data:`~pcapkit.utilities.config.EXTENSION_HEADER_LIMIT`. _exthdr_rebuilt = contextvars.ContextVar('_exthdr_rebuilt', default=0) +#: The IGMP base :meth:`Internet._next_layer_class` hands a message to, to pick its +#: version class. Resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_IGMP: 'ModuleDescriptor[IGMP]' = \ + ModuleDescriptor('pcapkit.protocols.internet.igmp.protocol', 'IGMP') + class Internet(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstract-method """Abstract base class for internet layer protocol family. @@ -453,7 +460,7 @@ class registered for ``proto``. Paired with it, the keywords that warn(message, ProtocolWarning, stacklevel=stacklevel()) return Raw, {'error': message} - from pcapkit.protocols.internet.igmp.protocol import IGMP as IGMPBase # isort: skip # pylint: disable=import-outside-toplevel + IGMPBase = _IGMP.klass # pylint: disable=invalid-name protocol = self._lookup_next_layer(self.__proto__, cast('int', proto)) if protocol is IGMPBase: diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 9ce9e00cb5..0cda02a356 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -36,7 +36,6 @@ import string import struct import textwrap -import threading import types import urllib.parse from typing import TYPE_CHECKING, Any, Generic, Optional, Type, TypeVar, cast, overload @@ -645,12 +644,6 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta): _shipped_registry: 'types.MappingProxyType[str, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]' = \ types.MappingProxyType(dict(__registry__)) - #: Serialises :meth:`get_protocol`'s resolution of a descriptor. Two threads - #: importing protocol modules from different points can deadlock, one - #: holding a package and the other a module inside it, so a first import is - #: made by one thread at a time (:issue:`2763`). - _registry_lock: 'threading.RLock' = threading.RLock() - #: Construction keywords this protocol consumes out of ``**kwargs`` instead #: of declaring as a parameter, e.g. with ``kwargs.get('spam')`` in #: :meth:`read` -- as :meth:`ESP.read ` @@ -1363,8 +1356,7 @@ class its module does not define. if entry is None: return default if isinstance(entry, ModuleDescriptor): - with ProtocolBase._registry_lock: - return entry.klass + return entry.klass return entry @classmethod diff --git a/pcapkit/utilities/importlib.py b/pcapkit/utilities/importlib.py index 974a0794fa..a082ebd5d6 100644 --- a/pcapkit/utilities/importlib.py +++ b/pcapkit/utilities/importlib.py @@ -9,19 +9,31 @@ the first time the name is reached, rather than with the package itself. Every package under :mod:`pcapkit.const` imports its enumerations that way (:issue:`2729`), the mechanism :mod:`pcapkit.const.reg.apptype` had for its TCP -and UDP registries (:issue:`1538`) made general. +and UDP registries (:issue:`1538`) made general. It also holds :data:`import_lock`, +which every such first import, and that of a +:class:`~pcapkit.corekit.module.ModuleDescriptor`, is made under. .. versionadded:: 1.6.0 """ import importlib +import threading import types from typing import TYPE_CHECKING if TYPE_CHECKING: from typing import Any, Callable, Iterable, Mapping, Optional -__all__ = ['lazy_exports'] +__all__ = ['lazy_exports', 'import_lock'] + +# NOTE: no pcapkit module resolves a lazy name or a descriptor on import: imported directly, +# it would wait on this lock holding its own import lock, which the holder may need (#2771). +#: threading.RLock: Held by every first import :func:`lazy_exports` +#: makes, and by :attr:`ModuleDescriptor.klass +#: ` when its module is not imported +#: yet, so threads make them one at a time; unserialised, they deadlock the import +#: system (:issue:`2771`). Re-entrant, as one first import can nest another. +import_lock = threading.RLock() def lazy_exports(package: 'str', namespace: 'dict[str, Any]', @@ -52,6 +64,14 @@ def lazy_exports(package: 'str', namespace: 'dict[str, Any]', :mod:`pickle` of an object defined in a submodule all behave as an eager import would, apart from *when* the import happens. + That import holds :data:`import_lock`, shared by every package using this + function, so threads reaching names at once resolve them one at a time and + a resolution may nest another. Unserialised, they deadlock the import + system (:issue:`2771`): a family member's protocol module imports a module of + its base's package, taking that module's import lock before waiting on the + package, while a thread importing the package takes the two the other way. + A resolved name is bound in ``namespace``, so reaching it again takes no lock. + ``__dir__`` lists what the eager import would have bound: the names the import system sets, ``__all__``, every export, every submodule an export comes from, and any other submodule already imported. The package's own @@ -80,12 +100,17 @@ def lazy_exports(package: 'str', namespace: 'dict[str, Any]', def __getattr__(name: 'str') -> 'Any': if name in table: - module, attribute = table[name] - value = getattr(importlib.import_module(f'{package}.{module}'), attribute) - namespace[name] = value - return value + with import_lock: + # NOTE: another thread may have bound it while this one waited. + if name in namespace: + return namespace[name] + module, attribute = table[name] + value = getattr(importlib.import_module(f'{package}.{module}'), attribute) + namespace[name] = value + return value if name in submodules: - return importlib.import_module(f'{package}.{name}') + with import_lock: + return importlib.import_module(f'{package}.{name}') raise AttributeError(f'module {package!r} has no attribute {name!r}') def __dir__() -> 'list[str]': diff --git a/tests/utilities/test_lazy_exports_threads_2771_unit.py b/tests/utilities/test_lazy_exports_threads_2771_unit.py new file mode 100644 index 0000000000..9b91981007 --- /dev/null +++ b/tests/utilities/test_lazy_exports_threads_2771_unit.py @@ -0,0 +1,242 @@ +# -*- coding: utf-8 -*- +"""Threads making first imports at once all get what they asked for (GitHub issue #2771). + +:func:`pcapkit.utilities.importlib.lazy_exports` imports a name the first time it is +reached, and :attr:`ModuleDescriptor.klass ` +the module of a next layer, or of a :meth:`ProtocolBase.get_protocol +` entry, the first time it is +resolved. Unserialised, two threads making such imports at once deadlock the import +system: InARP's protocol module imports ARP's ``data`` module, whose import lock is +taken before waiting on ARP's package, while a thread importing that package takes +the two the other way. So every such first import holds one re-entrant lock, +:data:`pcapkit.utilities.importlib.import_lock` (#2773 dropped the registry's own). + +Each trial is a fresh interpreter, since the race only exists on a cold import, and +starts one thread per job behind a barrier. A thread that has not returned by +``DEADLINE`` counts as hung, which is how a lock that is not re-entrant fails: a +:mod:`pcapkit` name resolves through :mod:`pcapkit.protocols` and a layer package, +nesting lazy resolutions on one thread. Every name resolved must then be bound on its +package, so reaching it again never calls ``__getattr__`` and takes no lock. And every +first import of a :mod:`pcapkit` module a trial makes must hold the lock, so a path +that imports a module of its own accord fails on every run, not only on a lost race. + +""" + +import json +import os +import pathlib +import struct +import subprocess +import sys +import unittest +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): races threads on the import system's locks, which differ between interpreters. +__tier__ = 'ALL' + +ROOT = pathlib.Path(__file__).resolve().parents[2] + +#: Fresh interpreters per test. Without the lock, measured per trial, nearly every one +#: deadlocked on CPython 3.14; on 3.12 about a third of the name races and few of the +#: dispatch races did, which the audit of unlocked first imports makes up for. +TRIALS = 4 + +#: Seconds a trial's threads have to finish before the rest count as hung. +DEADLINE = 60 + +#: The child: from a thread each, all at once, reach every unbound export of +#: ``packages``, every unbound submodule of ``submodules`` an export comes from, and +#: each of ``names``; parse each of ``frames`` from an Ethernet header; and look up +#: each protocol not imported yet with ``get_protocol`` if ``registry`` is set. Then +#: print what failed, hung, was left unbound, parsed or resolved to the wrong class, +#: or was imported for the first time without the lock held. +CHILD = ''' +import importlib, io, json, os, sys, threading, time + +spec = json.loads(sys.argv[1]) +import pcapkit +assert pcapkit.__file__.startswith(spec['root']), pcapkit.__file__ +from pcapkit.protocols.protocol import ProtocolBase +# A tree without the lock still runs, so that it fails on the race rather than on the import. +import_lock = getattr(importlib.import_module('pcapkit.utilities.importlib'), 'import_lock', None) +if spec['frames']: + from pcapkit.protocols.link.ethernet.protocol import Ethernet + +jobs = [('attr',) + tuple(target) for target in spec['names']] +for package in spec['packages']: + module = importlib.import_module(package) + jobs.extend(('attr', package, name) for name in module.__all__ if name not in vars(module)) +for package in spec['submodules']: + module = importlib.import_module(package) + jobs.extend(('attr', package, name) for name in dir(module) + if not name.startswith('__') and name not in module.__all__ and name not in vars(module)) +for member, octets in spec['frames']: + jobs.extend([('parse', member, octets)] * 3) +if spec['registry']: + jobs.extend(('get', name, entry) for name, entry in ProtocolBase.__registry__.items() + if entry.module not in sys.modules) +barrier = threading.Barrier(len(jobs)) +errors, values, wrong, unlocked = [], {}, [], [] + + +class Audit: + """Note each first import of a :mod:`pcapkit` module made without the lock.""" + + @staticmethod + def find_spec(name, path=None, target=None): + if name.startswith('pcapkit.') and import_lock is not None and not import_lock._is_owned(): + unlocked.append(name) + + +sys.meta_path.insert(0, Audit) + + +def run(kind, key, arg): + if kind == 'attr': + values[key, arg] = getattr(importlib.import_module(key), arg) + elif kind == 'parse': + layer, names = Ethernet(io.BytesIO(bytes.fromhex(arg)), len(arg) // 2), [] + for _ in range(8): + names.append(type(layer).__name__) + layer = layer.payload + if key not in names: + wrong.append(f'{key}: {names}') + elif ProtocolBase.get_protocol(key) is not getattr(sys.modules[arg.module], arg.name): + wrong.append(key) + + +def reach(*job): + barrier.wait() + try: + run(*job) + except BaseException as error: + errors.append(f'{job[0]} {job[1]}: {error!r}') + + +threads = [threading.Thread(target=reach, args=job, name=f'{job[0]} {job[1]}', daemon=True) for job in jobs] +# Switching threads more often widens the race, which some interpreters lose rarely. +sys.setswitchinterval(1e-6) +for thread in threads: + thread.start() +deadline = time.monotonic() + spec['deadline'] +for thread in threads: + thread.join(max(0, deadline - time.monotonic())) +hung = [thread.name for thread in threads if thread.is_alive()] +unbound = [f'{package}.{name}' for (package, name), value in list(values.items()) + if vars(sys.modules[package]).get(name) is not value] +print(json.dumps({'threads': len(jobs), 'errors': errors, 'hung': hung, 'unbound': unbound, 'wrong': wrong, + 'unlocked': sorted(set(unlocked)), 'kinds': sorted({job[0] for job in jobs})})) +sys.stdout.flush() +# A hung thread holds import locks, which interpreter shutdown could wait on. +os._exit(0) +''' + + +def checksum(octets: 'bytes') -> 'bytes': + """The Internet checksum of ``octets``.""" + octets += bytes(len(octets) % 2) + total = sum(struct.unpack(f'>{len(octets) // 2}H', octets)) + while total >> 16: + total = (total & 0xffff) + (total >> 16) + return struct.pack('>H', ~total & 0xffff) + + +def ipv4(proto: 'int', payload: 'bytes') -> 'bytes': + """An IPv4 header from 10.0.0.1 to 224.0.0.1, with its checksum, carrying ``payload``.""" + head = struct.pack('>BBHHHBBH4s4s', 0x45, 0, 20 + len(payload), 0, 0, 64, proto, 0, + bytes([10, 0, 0, 1]), bytes([224, 0, 0, 1])) + return head[:10] + checksum(head) + head[12:] + payload + + +def igmp(kind: 'int', rest: 'bytes' = bytes(4)) -> 'bytes': + """An IGMP message of type ``kind``, with its checksum.""" + body = bytes([kind, 0, 0, 0]) + rest + return body[:2] + checksum(body) + body[4:] + + +def ether(kind: 'int', body: 'bytes') -> 'bytes': + """An Ethernet frame of EtherType ``kind``.""" + return bytes([0xff] * 6) + bytes(range(6)) + struct.pack('>H', kind) + body + + +def arp(operation: 'int') -> 'bytes': + """An ARP message of ``operation``, for IPv4 over Ethernet, its addresses zero.""" + return struct.pack('>HHBBH', 1, 0x0800, 6, 4, operation) + bytes(20) + + +def udp(port: 'int', payload: 'bytes' = b'') -> 'bytes': + """A UDP datagram from port 1701 to ``port``.""" + return struct.pack('>HHHH', 1701, port, 8 + len(payload), 0) + payload + + +#: A frame per family member, each of which its first parse dispatches to: the +#: member's own module imports its base's. +FRAMES = [ + ('ARP', ether(0x0806, arp(1))), + ('RARP', ether(0x8035, arp(3))), + ('C_Tag', ether(0x8100, bytes([0, 1, 0x08, 0]) + ipv4(17, udp(53)))), + ('S_Tag', ether(0x88a8, bytes([0, 1, 0x08, 0]) + ipv4(17, udp(53)))), + ('IGMPv1', ether(0x0800, ipv4(2, igmp(0x12)))), + ('IGMPv2', ether(0x0800, ipv4(2, igmp(0x16)))), + ('IGMPv3', ether(0x0800, ipv4(2, igmp(0x22)))), + ('L2TPv2', ether(0x0800, ipv4(17, udp(1701, struct.pack('>HHH', 0x0002, 1, 1))))), +] + + +def run_trial(packages: 'list[str]', submodules: 'list[str]', names: 'tuple[tuple[str, str], ...]' = (), *, + frames: 'bool' = False, registry: 'bool' = False) -> 'dict[str, Any]': + """Run :data:`CHILD` in a fresh interpreter; return what it printed.""" + environ = dict(os.environ) + environ.pop('PYTHONSAFEPATH', None) + environ['PYTHONPATH'] = os.pathsep.join( + [str(ROOT), environ['PYTHONPATH']] if environ.get('PYTHONPATH') else [str(ROOT)]) + spec = json.dumps({'root': str(ROOT), 'packages': packages, 'submodules': submodules, 'names': names, + 'frames': [(member, octets.hex()) for member, octets in FRAMES] if frames else [], + 'registry': registry, 'deadline': DEADLINE}) + result = subprocess.run([sys.executable, '-c', CHILD, spec], # nosec: B603 + cwd=str(ROOT), env=environ, capture_output=True, text=True, + timeout=DEADLINE + 240, check=False) + if result.returncode != 0: + raise AssertionError(f'child exited {result.returncode}:\n{result.stderr}') + return json.loads(result.stdout.splitlines()[-1]) + + +class ConcurrentFirstImportTests(unittest.TestCase): + """Every job run from many threads at once, on a cold import, gets what it asked for.""" + + def check(self, *args: 'Any', **kwargs: 'Any') -> 'list[dict[str, Any]]': + """Run :data:`TRIALS` trials of :data:`CHILD`; fail on one where a job went wrong.""" + outs = [run_trial(*args, **kwargs) for _ in range(TRIALS)] + keys = ('errors', 'hung', 'unbound', 'wrong', 'unlocked') + failed = [{key: out[key][:3] for key in keys if out[key]} for out in outs if any(out[key] for key in keys)] + if failed: + self.fail(f'{len(failed)} of {TRIALS} trials failed; first: {failed[0]}') + for out in outs: + self.assertGreaterEqual(out['threads'], 16) + return outs + + def test_threads_reaching_different_names_resolve_them_all(self) -> 'None': + """:mod:`pcapkit.protocols`, two layer packages and two const packages, all at once.""" + self.check(['pcapkit.protocols', 'pcapkit.protocols.link', + 'pcapkit.const.ipv4', 'pcapkit.const.ipv6'], ['pcapkit.protocols.internet']) + + def test_a_resolution_nesting_another_package_resolves(self) -> 'None': + """A :mod:`pcapkit` name nests the resolutions of :mod:`pcapkit.protocols` and a layer package.""" + self.check(['pcapkit'], []) + + def test_threads_dispatching_to_different_family_members_parse_them(self) -> 'None': + """Each frame's first parse imports a family member, through its next-layer descriptor.""" + self.check([], [], frames=True) + + def test_get_protocol_racing_first_dispatches_resolves_every_name(self) -> 'None': + """Every protocol not imported yet, InARP and DRARP among them, looked up as frames dispatch.""" + outs = self.check([], [], frames=True, registry=True) + for out in outs: + self.assertEqual(out['kinds'], ['get', 'parse']) + + +if __name__ == '__main__': + unittest.main() From 89626e4d9dea886810420c81cbb33846e6462cf6 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 4 Nov 2026 14:55:49 -0500 Subject: [PATCH 2/2] fix(protocols): resolve the remaining member and helper imports under the shared import lock (#2771) - Function-level imports of modules `import pcapkit` leaves unloaded made their first import without import_lock, so they could still deadlock against a locked one. - They resolve through module-level ModuleDescriptor constants instead, as IGMP's do: HTTP's and LinuxSLL's members and data models, L2TP's member, the TCP and UDP port registries of the __index__ hooks, the ip_pseudo helpers of IPv4, IPv6, ICMPv6, TCP, UDP and DCCP, verify_upper's AH, and the PCAP and PCAP-NG engines' toolkits. - The race test makes these first resolutions at once with checksums verified, and a new project test fails on any function body importing a module first, bar the sites it allows with their reasons. --- pcapkit/foundation/engines/pcap.py | 14 +- pcapkit/foundation/engines/pcapng.py | 14 +- .../protocols/application/dtls/protocol.py | 13 +- pcapkit/protocols/application/ftp/protocol.py | 14 +- .../application/ftp_data/protocol.py | 14 +- .../protocols/application/http/protocol.py | 63 +++++--- pcapkit/protocols/application/tls/protocol.py | 13 +- pcapkit/protocols/internet/icmpv6/protocol.py | 9 +- pcapkit/protocols/internet/ipv4/protocol.py | 16 +- pcapkit/protocols/internet/ipv6/protocol.py | 16 +- pcapkit/protocols/link/l2tp/protocol.py | 13 +- pcapkit/protocols/link/l2tpv2/protocol.py | 13 +- pcapkit/protocols/link/linux_sll/protocol.py | 37 +++-- .../misc/checksum/ip_pseudo/protocol.py | 13 +- pcapkit/protocols/transport/dccp/protocol.py | 8 +- pcapkit/protocols/transport/tcp/protocol.py | 8 +- pcapkit/protocols/transport/udp/protocol.py | 8 +- ..._function_level_first_imports_2771_unit.py | 151 ++++++++++++++++++ .../test_lazy_exports_threads_2771_unit.py | 82 +++++++++- 19 files changed, 432 insertions(+), 87 deletions(-) create mode 100644 tests/project/test_function_level_first_imports_2771_unit.py diff --git a/pcapkit/foundation/engines/pcap.py b/pcapkit/foundation/engines/pcap.py index 55f38992df..8603f0e681 100644 --- a/pcapkit/foundation/engines/pcap.py +++ b/pcapkit/foundation/engines/pcap.py @@ -13,6 +13,7 @@ from typing import TYPE_CHECKING from pcapkit.corekit.io import SeekableReader +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.foundation.engines.engine import EngineBase from pcapkit.protocols.misc.pcap.frame.protocol import Frame from pcapkit.protocols.misc.pcap.frame.schema import RECORD_HEADER_LEN @@ -23,6 +24,8 @@ __all__ = ['PCAP'] if TYPE_CHECKING: + from typing import Any + from pcapkit.const.reg.linktype import LinkType as Enum_LinkType from pcapkit.corekit.version import VersionInfo @@ -30,6 +33,13 @@ #: :data:`pcapkit.utilities.logging.logger`. logger = get_logger(__name__) +#: The toolkit functions :meth:`PCAP.read_frame` hands each frame to, resolved +#: there rather than imported, so their first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TOOLKIT = tuple(ModuleDescriptor('pcapkit.toolkit.pcap', name) for name in ( + 'dtls_reassembly', 'ipv4_reassembly', 'ipv6_reassembly', 'tcp_reassembly', 'tcp_traceflow', +)) # type: tuple[ModuleDescriptor[Any], ...] + class PCAP(EngineBase[Frame]): """PCAP file extraction support. @@ -155,8 +165,8 @@ def read_frame(self) -> 'Frame': Parsed frame instance. """ - from pcapkit.toolkit.pcap import (dtls_reassembly, ipv4_reassembly, ipv6_reassembly, - tcp_reassembly, tcp_traceflow) + dtls_reassembly, ipv4_reassembly, ipv6_reassembly, tcp_reassembly, tcp_traceflow = ( + descriptor.klass for descriptor in _TOOLKIT) ext = self._extractor # read frame header diff --git a/pcapkit/foundation/engines/pcapng.py b/pcapkit/foundation/engines/pcapng.py index d9a918cbb5..bc36a9764b 100644 --- a/pcapkit/foundation/engines/pcapng.py +++ b/pcapkit/foundation/engines/pcapng.py @@ -16,6 +16,7 @@ from pcapkit.const.pcapng.block_type import BlockType as Enum_BlockType from pcapkit.corekit.infoclass import Info, info_final from pcapkit.corekit.io import SeekableReader +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.schema.padding import pad_length from pcapkit.dumpkit.pcap import PCAPIO from pcapkit.foundation.engines.engine import EngineBase @@ -33,7 +34,7 @@ logger = get_logger(__name__) if TYPE_CHECKING: - from typing import Literal, Optional + from typing import Any, Literal, Optional from pcapkit.foundation.extraction import Extractor from pcapkit.protocols.misc.pcapng.data import PCAPNG as Data_PCAPNG @@ -50,6 +51,13 @@ SystemdJournalExportBlock as Data_SystemdJournalExportBlock from pcapkit.protocols.misc.pcapng.data import UnknownBlock as Data_UnknownBlock +#: The toolkit functions :meth:`PCAPNG.read_frame` hands each frame to, resolved +#: there rather than imported, so their first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TOOLKIT = tuple(ModuleDescriptor('pcapkit.toolkit.pcapng', name) for name in ( + 'dtls_reassembly', 'ipv4_reassembly', 'ipv6_reassembly', 'tcp_reassembly', 'tcp_traceflow', +)) # type: tuple[ModuleDescriptor[Any], ...] + @info_final class Context(Info): @@ -211,8 +219,8 @@ def read_frame(self) -> 'P_PCAPNG': Parsed PCAP-NG block. """ - from pcapkit.toolkit.pcapng import (dtls_reassembly, ipv4_reassembly, ipv6_reassembly, - tcp_reassembly, tcp_traceflow) + dtls_reassembly, ipv4_reassembly, ipv6_reassembly, tcp_reassembly, tcp_traceflow = ( + descriptor.klass for descriptor in _TOOLKIT) ext = self._extractor while True: diff --git a/pcapkit/protocols/application/dtls/protocol.py b/pcapkit/protocols/application/dtls/protocol.py index 98212c083d..a2940cf2fe 100644 --- a/pcapkit/protocols/application/dtls/protocol.py +++ b/pcapkit/protocols/application/dtls/protocol.py @@ -109,6 +109,7 @@ from pcapkit.const.tls.extension_type import ExtensionType as Enum_ExtensionType from pcapkit.corekit.context import ProtocolContext from pcapkit.corekit.decorators import beholder +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.application.dtls.data import DTLS as Data_DTLS from pcapkit.protocols.application.dtls.data import DTLSCIDCiphertext as Data_DTLSCIDCiphertext from pcapkit.protocols.application.dtls.data import DTLSCiphertext as Data_DTLSCiphertext @@ -126,12 +127,19 @@ from typing_extensions import Literal + from pcapkit.const.reg.apptype.udp import UDP as AppType_UDP from pcapkit.const.tls.handshake_type import HandshakeType as Enum_HandshakeType from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema import Schema __all__ = ['DTLS', 'DTLSContext'] +#: The UDP port registry :meth:`DTLS.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_UDP: 'ModuleDescriptor[AppType_UDP]' = ModuleDescriptor('pcapkit.const.reg.apptype.udp', 'UDP') + #: Length of a legacy record header -- content type, legacy version, epoch, #: sequence number and length (:rfc:`9147#section-4`). _LEGACY_HEADER_LENGTH = 13 @@ -768,10 +776,7 @@ def __index__(cls) -> 'int': # type: ignore[override] .. _IANA: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml """ - # imported here, as ``import pcapkit`` leaves the UDP registry unloaded (#1538) - from pcapkit.const.reg.apptype.udp import UDP # pylint: disable=import-outside-toplevel - - return int(UDP.coaps) # type: ignore[call-overload] + return int(_UDP.klass.coaps) # type: ignore[call-overload] ########################################################################## # Utilities. diff --git a/pcapkit/protocols/application/ftp/protocol.py b/pcapkit/protocols/application/ftp/protocol.py index 415d2f3feb..5c775ff06f 100644 --- a/pcapkit/protocols/application/ftp/protocol.py +++ b/pcapkit/protocols/application/ftp/protocol.py @@ -35,6 +35,7 @@ from pcapkit.const.ftp.command import Command as Enum_Command from pcapkit.const.ftp.return_code import ReturnCode as Enum_ReturnCode from pcapkit.corekit.enum import EnumLookup +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.application.application import Application from pcapkit.protocols.application.ftp.data import FTP as Data_FTP from pcapkit.protocols.application.ftp.data import Request as Data_Request @@ -49,8 +50,16 @@ from typing_extensions import Literal + from pcapkit.const.reg.apptype.tcp import TCP as AppType_TCP + __all__ = ['FTP'] +#: The TCP port registry :meth:`FTP.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TCP: 'ModuleDescriptor[AppType_TCP]' = ModuleDescriptor('pcapkit.const.reg.apptype.tcp', 'TCP') + # regex for FTP, per :rfc:`959#section-4` (replies) and :rfc:`959#section-5` # (commands): a command is `` [ ] ``, a reply line is # `` [ ] ``, and the first line of a multi-line reply is @@ -290,10 +299,7 @@ def __index__(cls) -> 'int': # type: ignore[override] :exc:`~pcapkit.utilities.exceptions.IntError` (:issue:`2627`). """ - # imported here, as ``import pcapkit`` leaves the TCP registry unloaded (#1538) - from pcapkit.const.reg.apptype.tcp import TCP # pylint: disable=import-outside-toplevel - - return int(TCP.ftp) # type: ignore[call-overload] + return int(_TCP.klass.ftp) # type: ignore[call-overload] ########################################################################## # Utilities. diff --git a/pcapkit/protocols/application/ftp_data/protocol.py b/pcapkit/protocols/application/ftp_data/protocol.py index 12dbe6b9bb..c49fc1b4ed 100644 --- a/pcapkit/protocols/application/ftp_data/protocol.py +++ b/pcapkit/protocols/application/ftp_data/protocol.py @@ -22,14 +22,23 @@ """ from typing import TYPE_CHECKING +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.application.application import Application from pcapkit.protocols.misc.raw.protocol import Raw if TYPE_CHECKING: from typing_extensions import Literal + from pcapkit.const.reg.apptype.tcp import TCP as AppType_TCP + __all__ = ['FTP_DATA'] +#: The TCP port registry :meth:`FTP_DATA.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TCP: 'ModuleDescriptor[AppType_TCP]' = ModuleDescriptor('pcapkit.const.reg.apptype.tcp', 'TCP') + class FTP_DATA(Application, Raw): # pylint: disable=abstract-method """This class implements FTP data channel transmission. @@ -85,7 +94,4 @@ def __index__(cls) -> 'int': # type: ignore[override] :exc:`~pcapkit.utilities.exceptions.IntError` (:issue:`2627`). """ - # imported here, as ``import pcapkit`` leaves the TCP registry unloaded (#1538) - from pcapkit.const.reg.apptype.tcp import TCP # pylint: disable=import-outside-toplevel - - return int(TCP.ftp_data) # type: ignore[call-overload] + return int(_TCP.klass.ftp_data) # type: ignore[call-overload] diff --git a/pcapkit/protocols/application/http/protocol.py b/pcapkit/protocols/application/http/protocol.py index cbc24ab2d5..0f39a14a96 100644 --- a/pcapkit/protocols/application/http/protocol.py +++ b/pcapkit/protocols/application/http/protocol.py @@ -34,6 +34,7 @@ import struct from typing import TYPE_CHECKING, Generic, cast +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.application.application import Application from pcapkit.protocols.misc.no_payload.protocol import NoPayload @@ -45,10 +46,21 @@ from typing_extensions import Literal, Self + from pcapkit.const.reg.apptype.tcp import TCP as AppType_TCP + from pcapkit.protocols.application.httpv1.data import HTTPv1 as Data_HTTPv1 + from pcapkit.protocols.application.httpv1.protocol import HTTPv1 + from pcapkit.protocols.application.httpv2.data import HTTPv2 as Data_HTTPv2 + from pcapkit.protocols.application.httpv2.protocol import HTTPv2 from pcapkit.protocols.protocol import ProtocolBase __all__ = ['HTTP', 'test_start_line'] +#: The TCP port registry :meth:`HTTP.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TCP: 'ModuleDescriptor[AppType_TCP]' = ModuleDescriptor('pcapkit.const.reg.apptype.tcp', 'TCP') + #: The HTTP/2 connection preface (:rfc:`9113#section-3.4`). A client opens every #: HTTP/2 connection -- with prior knowledge, over TLS, or after an upgrade -- #: by sending exactly these 24 octets, and the sequence is *designed* to be @@ -146,6 +158,18 @@ def _test_http2_frame(data: 'bytes') -> 'bool': # preface never parses as HTTP/1. _HTTP2_PREFACE_HEADER = _HTTP2_PREFACE.split(b'\r\n\r\n', maxsplit=1)[0] +#: The member :class:`HTTP` hands an HTTP/1.* message to; it, the HTTP/2 member and +#: their data models are resolved where used rather than imported, so a member's first +#: import holds :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_HTTPV1: 'ModuleDescriptor[HTTPv1]' = \ + ModuleDescriptor('pcapkit.protocols.application.httpv1.protocol', 'HTTPv1') +_HTTPV2: 'ModuleDescriptor[HTTPv2]' = \ + ModuleDescriptor('pcapkit.protocols.application.httpv2.protocol', 'HTTPv2') +_DATA_HTTPV1: 'ModuleDescriptor[Data_HTTPv1]' = \ + ModuleDescriptor('pcapkit.protocols.application.httpv1.data', 'HTTPv1') +_DATA_HTTPV2: 'ModuleDescriptor[Data_HTTPv2]' = \ + ModuleDescriptor('pcapkit.protocols.application.httpv2.data', 'HTTPv2') + def test_start_line(data: 'bytes') -> 'bool': """Whether ``data`` opens with an HTTP/1.* start line. @@ -336,9 +360,9 @@ def read(self, length: 'Optional[int]' = None, *, http = self._guess_version(length, **kwargs) else: if version == 1: - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 as protocol # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + protocol = _HTTPV1.klass # type: Type[HTTP] elif version == 2: - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 as protocol # type: ignore[assignment] # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + protocol = _HTTPV2.klass else: raise ProtocolError(f'HTTP: invalid version: {version!r}') @@ -451,10 +475,9 @@ def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': if cls is not HTTP: return super().from_data(data, **kwargs) - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv1.data import HTTPv1 as Data_HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv2.data import HTTPv2 as Data_HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + HTTPv1 = _HTTPV1.klass # pylint: disable=invalid-name + HTTPv2 = _HTTPV2.klass # pylint: disable=invalid-name + Data_HTTPv1, Data_HTTPv2 = _DATA_HTTPV1.klass, _DATA_HTTPV2.klass # pylint: disable=invalid-name if isinstance(data, Data_HTTPv1): return cast('Self', HTTPv1.from_data(data, **kwargs)) @@ -489,10 +512,7 @@ def __index__(cls) -> 'int': # type: ignore[override] :exc:`~pcapkit.utilities.exceptions.IntError` (:issue:`2627`). """ - # imported here, as ``import pcapkit`` leaves the TCP registry unloaded (#1538) - from pcapkit.const.reg.apptype.tcp import TCP # pylint: disable=import-outside-toplevel - - return int(TCP.http) # type: ignore[call-overload] + return int(_TCP.klass.http) # type: ignore[call-overload] def __init__(self, file: 'Optional[IO[bytes] | bytes]' = None, length: 'Optional[int]' = None, *, @@ -519,8 +539,7 @@ def __init__(self, file: 'Optional[IO[bytes] | bytes]' = None, if version is not None: kwargs['version'] = version else: - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=import-outside-toplevel + HTTPv1, HTTPv2 = _HTTPV1.klass, _HTTPV2.klass # pylint: disable=invalid-name if version is not None and not isinstance(self, self._member_of_version(version)): if not isinstance(self, (HTTPv1, HTTPv2)): @@ -604,10 +623,9 @@ def _make_data(cls, data: '_PT') -> 'dict[str, Any]': # type: ignore[override] # HTTP/1.*'s lives in ``receipt`` as the start line's string, and HTTP/2 # has none -- so the version is the class of ``data`` that :meth:`read` # returned from the versioned parser. - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv1.data import HTTPv1 as Data_HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.application.httpv2.data import HTTPv2 as Data_HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + HTTPv1 = _HTTPV1.klass # pylint: disable=invalid-name + HTTPv2 = _HTTPV2.klass # pylint: disable=invalid-name + Data_HTTPv1, Data_HTTPv2 = _DATA_HTTPV1.klass, _DATA_HTTPV2.klass # pylint: disable=invalid-name if isinstance(data, Data_HTTPv1): return {'version': 1, **HTTPv1._make_data(data)} @@ -696,7 +714,7 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': # preface must not be claimed out of octets outside this payload. preface_len = len(_HTTP2_PREFACE) if self._test_preface(length): - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + HTTPv2 = _HTTPV2.klass # pylint: disable=invalid-name # The preface is not a frame -- the frames begin after it # (:rfc:`9113#section-3.4` requires a ``SETTINGS`` frame immediately @@ -733,7 +751,7 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': self._preface_length = preface_len return http - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel + HTTPv1 = _HTTPV1.klass # pylint: disable=invalid-name # NOTE: The second identification: an HTTP/1 ``request-line`` or # ``status-line``, tested with the very patterns ``httpv1.HTTPv1`` parses @@ -799,9 +817,8 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP': if not _test_http2_frame(self._data[:length]): raise ProtocolError('HTTP: unknown version') - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel with contextlib.suppress(ProtocolError, struct.error): - return HTTPv2(self._data, length, **kwargs) + return _HTTPV2.klass(self._data, length, **kwargs) raise ProtocolError('HTTP: unknown version') @@ -821,8 +838,7 @@ def _member_of_version(version: 'Any') -> 'Type[HTTP]': :obj:`bool` or a :obj:`float` is refused even where it equals one. """ - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=import-outside-toplevel + HTTPv1, HTTPv2 = _HTTPV1.klass, _HTTPV2.klass # pylint: disable=invalid-name if isinstance(version, int) and not isinstance(version, bool): if version == 1: @@ -874,8 +890,7 @@ def _dispatch_version(cls, data: 'bytes', length: 'int') -> 'Type[ProtocolBase]' So this does not pick that class for it, and the two agree. """ - from pcapkit.protocols.application.httpv1.protocol import HTTPv1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.application.httpv2.protocol import HTTPv2 # isort: skip # pylint: disable=import-outside-toplevel + HTTPv1, HTTPv2 = _HTTPV1.klass, _HTTPV2.klass # pylint: disable=invalid-name from pcapkit.protocols.misc.raw.protocol import Raw # isort: skip # pylint: disable=import-outside-toplevel data = data[:length] diff --git a/pcapkit/protocols/application/tls/protocol.py b/pcapkit/protocols/application/tls/protocol.py index e64c42efae..17dd896401 100644 --- a/pcapkit/protocols/application/tls/protocol.py +++ b/pcapkit/protocols/application/tls/protocol.py @@ -68,6 +68,7 @@ from pcapkit.const.tls.content_type import ContentType as Enum_ContentType from pcapkit.corekit.decorators import beholder +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.application.ssl.protocol import (_TLS_VERSION_MAJOR, _WIRE_CONTENT_TYPES, SSL, TLS13_VERSION) from pcapkit.protocols.application.sslv2.data import ClientHello as SSLv2_ClientHello @@ -85,11 +86,18 @@ from typing_extensions import Literal + from pcapkit.const.reg.apptype.tcp import TCP as AppType_TCP from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema import Schema __all__ = ['TLS'] +#: The TCP port registry :meth:`TLS.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_TCP: 'ModuleDescriptor[AppType_TCP]' = ModuleDescriptor('pcapkit.const.reg.apptype.tcp', 'TCP') + #: Length of the record header -- content type, legacy version and length #: (:rfc:`9846#section-5.1`). _RECORD_HEADER_LENGTH = 5 @@ -369,10 +377,7 @@ def __index__(cls) -> 'int': # type: ignore[override] .. _IANA: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml """ - # imported here, as ``import pcapkit`` leaves the TCP registry unloaded (#1538) - from pcapkit.const.reg.apptype.tcp import TCP # pylint: disable=import-outside-toplevel - - return int(TCP.https_443) # type: ignore[call-overload] + return int(_TCP.klass.https_443) # type: ignore[call-overload] ########################################################################## # Utilities. diff --git a/pcapkit/protocols/internet/icmpv6/protocol.py b/pcapkit/protocols/internet/icmpv6/protocol.py index 97b35c188c..6bf90b7771 100644 --- a/pcapkit/protocols/internet/icmpv6/protocol.py +++ b/pcapkit/protocols/internet/icmpv6/protocol.py @@ -27,6 +27,7 @@ from pcapkit.const.icmpv6.type import Type as Enum_Type from pcapkit.const.reg.transtype import TransType as Enum_TransType +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.internet.icmp.protocol import ICMP from pcapkit.protocols.internet.icmpv6.data import \ @@ -65,6 +66,12 @@ __all__ = ['ICMPv6'] +#: The pseudo-header a payload's checksum is computed over in :meth:`ICMPv6.make`, +#: resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_PSEUDO_HEADER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'pseudo_header') + class ICMPv6(ICMP[Data_ICMPv6, Schema_ICMPv6], schema=Schema_ICMPv6, data=Data_ICMPv6): @@ -292,7 +299,7 @@ def make(self, self._checksum_pending = True else: from pcapkit.corekit.checksum.internet import compute_checksum # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import pseudo_header # isort: skip # pylint: disable=import-outside-toplevel + pseudo_header = _PSEUDO_HEADER.klass octets = schema.pack() schema.checksum = compute_checksum(octets, prefix=pseudo_header(pseudo, len(octets))) return schema diff --git a/pcapkit/protocols/internet/ipv4/protocol.py b/pcapkit/protocols/internet/ipv4/protocol.py index dba1198793..6f1003b425 100644 --- a/pcapkit/protocols/internet/ipv4/protocol.py +++ b/pcapkit/protocols/internet/ipv4/protocol.py @@ -56,6 +56,7 @@ from pcapkit.const.ipv4.tos_thr import ToSThroughput as Enum_ToSThroughput from pcapkit.const.ipv4.ts_flag import TSFlag as Enum_TSFlag from pcapkit.const.reg.transtype import TransType as Enum_TransType +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.corekit.schema.ipaddress import parse_ip_address from pcapkit.corekit.schema.padding import pad_length @@ -120,6 +121,7 @@ from pcapkit.protocols.internet.ipv4.data import Option as Data_Option from pcapkit.protocols.internet.ipv4.schema import Option as Schema_Option + from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import IP_Pseudo from pcapkit.protocols.protocol import ProtocolBase Option = OrderedMultiDict[Enum_OptionNumber, Data_Option] @@ -129,6 +131,15 @@ __all__ = ['IPv4'] +#: The pseudo-header :meth:`IPv4.make` computes a payload's checksum over, and the +#: check :meth:`IPv4.__post_init__` runs on the upper layer, resolved there rather +#: than imported, so their first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_IP_PSEUDO: 'ModuleDescriptor[IP_Pseudo]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'IP_Pseudo') +_VERIFY_UPPER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'verify_upper') + class IPv4(IP[Data_IPv4, Schema_IPv4], schema=Schema_IPv4, data=Data_IPv4): @@ -617,7 +628,7 @@ def make(self, # pseudo-header, unless a fragment, whose checksum covers the datagram (#2471). if (config.CHECKSUM_COMPUTE and getattr(payload, '_checksum_pending', False) and cast('ProtocolBase', payload).__index__() == proto and not (offset or mf)): - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import IP_Pseudo # isort: skip # pylint: disable=import-outside-toplevel + IP_Pseudo = _IP_PSEUDO.klass # pylint: disable=invalid-name # NOTE: The header is parsed back for a source route's destination, # quietly, as this layer parses it again (cf. rebuilding). @@ -730,8 +741,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, """ super().__post_init__(file, length, **kwargs) if config.CHECKSUM_VERIFY: - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import verify_upper # isort: skip # pylint: disable=import-outside-toplevel - verify_upper(self) + _VERIFY_UPPER.klass(self) ########################################################################## # Utilities. diff --git a/pcapkit/protocols/internet/ipv6/protocol.py b/pcapkit/protocols/internet/ipv6/protocol.py index a4c300ecf0..ed2b1059f2 100644 --- a/pcapkit/protocols/internet/ipv6/protocol.py +++ b/pcapkit/protocols/internet/ipv6/protocol.py @@ -33,6 +33,7 @@ from pcapkit.const.ipv6.extension_header import ExtensionHeader as Enum_ExtensionHeader from pcapkit.const.reg.transtype import TransType as Enum_TransType from pcapkit.corekit.decorators import beholder +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.corekit.protochain import ProtoChain from pcapkit.corekit.schema.jumbogram import jumbo_payload_length @@ -51,11 +52,21 @@ from typing_extensions import Literal + from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import IP_Pseudo from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema import Schema __all__ = ['IPv6'] +#: The pseudo-header :meth:`IPv6.make` computes a payload's checksum over, and the +#: check :meth:`IPv6.__post_init__` runs on the upper layer, resolved there rather +#: than imported, so their first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_IP_PSEUDO: 'ModuleDescriptor[IP_Pseudo]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'IP_Pseudo') +_VERIFY_UPPER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'verify_upper') + #: Length of the fixed IPv6 header, i.e. the offset of the first extension #: header, :rfc:`8200#section-3`. The one home of this value (:issue:`1739`); it #: is fixed by the RFC, so it is never configurable. @@ -338,7 +349,7 @@ def make(self, # pseudo-header; see pcapkit.protocols.misc.checksum.ip_pseudo.protocol (#2471). if (config.CHECKSUM_COMPUTE and getattr(payload, '_checksum_pending', False) and cast('ProtocolBase', payload).__index__() == next_val): - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import IP_Pseudo # isort: skip # pylint: disable=import-outside-toplevel + IP_Pseudo = _IP_PSEUDO.klass # pylint: disable=invalid-name # NOTE: The payload follows this header directly, so no extension # header moves an address; it puts its own length in the pseudo-header. @@ -420,8 +431,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, """ super().__post_init__(file, length, **kwargs) if config.CHECKSUM_VERIFY: - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import verify_upper # isort: skip # pylint: disable=import-outside-toplevel - verify_upper(self) + _VERIFY_UPPER.klass(self) ########################################################################## # Utilities. diff --git a/pcapkit/protocols/link/l2tp/protocol.py b/pcapkit/protocols/link/l2tp/protocol.py index 48458c6978..2115a5afd0 100644 --- a/pcapkit/protocols/link/l2tp/protocol.py +++ b/pcapkit/protocols/link/l2tp/protocol.py @@ -100,6 +100,7 @@ """ from typing import TYPE_CHECKING, Generic, cast +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.link.link import Link from pcapkit.protocols.protocol import _PT, _ST from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall @@ -109,8 +110,16 @@ from typing_extensions import Literal, Self + from pcapkit.protocols.link.l2tpv2.protocol import L2TPv2 + __all__ = ['L2TP'] +#: The member :meth:`L2TP._member_of_version` picks for version ``2``, resolved +#: there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_L2TPV2: 'ModuleDescriptor[L2TPv2]' = \ + ModuleDescriptor('pcapkit.protocols.link.l2tpv2.protocol', 'L2TPv2') + class L2TP(Link[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstract-method """This class implements all protocols in L2TP family. @@ -280,8 +289,6 @@ def _member_of_version(version: 'Any') -> 'Type[L2TP]': implemented. """ - from pcapkit.protocols.link.l2tpv2.protocol import L2TPv2 # isort: skip # pylint: disable=import-outside-toplevel - if isinstance(version, int) and not isinstance(version, bool) and version == 2: - return cast('Type[L2TP]', L2TPv2) + return cast('Type[L2TP]', _L2TPV2.klass) raise ProtocolError(f'L2TP: invalid version: {version!r}') diff --git a/pcapkit/protocols/link/l2tpv2/protocol.py b/pcapkit/protocols/link/l2tpv2/protocol.py index f911c8dc4a..5c9acff31d 100644 --- a/pcapkit/protocols/link/l2tpv2/protocol.py +++ b/pcapkit/protocols/link/l2tpv2/protocol.py @@ -54,6 +54,7 @@ from typing import TYPE_CHECKING from pcapkit.const.l2tp.type import Type as Enum_Type +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.link.l2tp.data import L2TP as Data_L2TP from pcapkit.protocols.link.l2tp.data import Flags as Data_Flags from pcapkit.protocols.link.l2tp.protocol import L2TP @@ -65,11 +66,18 @@ from typing_extensions import Literal + from pcapkit.const.reg.apptype.udp import UDP as AppType_UDP from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema import Schema __all__ = ['L2TPv2'] +#: The UDP port registry :meth:`L2TPv2.__index__` reads its port from, resolved +#: there rather than imported, as ``import pcapkit`` leaves it unloaded +#: (:issue:`1538`), so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_UDP: 'ModuleDescriptor[AppType_UDP]' = ModuleDescriptor('pcapkit.const.reg.apptype.udp', 'UDP') + # NOTE: ``schema`` and ``data`` are stated here rather than inherited from # :class:`~pcapkit.protocols.link.l2tp.protocol.L2TP`, which declares none: the base is @@ -413,10 +421,7 @@ def __index__(cls) -> 'int': # type: ignore[override] :exc:`~pcapkit.utilities.exceptions.UnsupportedCall` (:issue:`2627`). """ - # imported here, as ``import pcapkit`` leaves the UDP registry unloaded (#1538) - from pcapkit.const.reg.apptype.udp import UDP # pylint: disable=import-outside-toplevel - - return int(UDP.l2tp) # type: ignore[call-overload] + return int(_UDP.klass.l2tp) # type: ignore[call-overload] ########################################################################## # Utilities. diff --git a/pcapkit/protocols/link/linux_sll/protocol.py b/pcapkit/protocols/link/linux_sll/protocol.py index c67f586943..2a00e6840a 100644 --- a/pcapkit/protocols/link/linux_sll/protocol.py +++ b/pcapkit/protocols/link/linux_sll/protocol.py @@ -33,6 +33,10 @@ from typing_extensions import Literal, Self from pcapkit.corekit.enum import EnumRegistry + from pcapkit.protocols.link.linux_sll1.data import LinuxSLL1 as Data_LinuxSLL1 + from pcapkit.protocols.link.linux_sll1.protocol import LinuxSLL1 + from pcapkit.protocols.link.linux_sll2.data import LinuxSLL2 as Data_LinuxSLL2 + from pcapkit.protocols.link.linux_sll2.protocol import LinuxSLL2 from pcapkit.protocols.protocol import ProtocolBase __all__ = ['LinuxSLL'] @@ -41,6 +45,19 @@ #: :class:`~pcapkit.protocols.link.ethernet.schema.Ethernet` resolves its ``type``. _ETHERTYPE_FIELD = EnumField(length=2, namespace=Enum_EtherType) +#: The member :class:`LinuxSLL` hands a version 1 header to; it, the version 2 +#: member and their data models are resolved where used rather than imported, so a +#: member's first import holds :data:`~pcapkit.utilities.importlib.import_lock` +#: (:issue:`2771`). +_LINUX_SLL1: 'ModuleDescriptor[LinuxSLL1]' = \ + ModuleDescriptor('pcapkit.protocols.link.linux_sll1.protocol', 'LinuxSLL1') +_LINUX_SLL2: 'ModuleDescriptor[LinuxSLL2]' = \ + ModuleDescriptor('pcapkit.protocols.link.linux_sll2.protocol', 'LinuxSLL2') +_DATA_LINUX_SLL1: 'ModuleDescriptor[Data_LinuxSLL1]' = \ + ModuleDescriptor('pcapkit.protocols.link.linux_sll1.data', 'LinuxSLL1') +_DATA_LINUX_SLL2: 'ModuleDescriptor[Data_LinuxSLL2]' = \ + ModuleDescriptor('pcapkit.protocols.link.linux_sll2.data', 'LinuxSLL2') + def _resolve_next_layer_proto(registry: 'DefaultDict[int, Any]', arphrd_type: 'int', protocol_type: 'int') -> 'Enum_EtherType | int': @@ -236,15 +253,10 @@ def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': if cls is not LinuxSLL: return super().from_data(data, **kwargs) - from pcapkit.protocols.link.linux_sll1.data import LinuxSLL1 as Data_LinuxSLL1 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.link.linux_sll2.data import LinuxSLL2 as Data_LinuxSLL2 # isort: skip # pylint: disable=line-too-long,import-outside-toplevel - from pcapkit.protocols.link.linux_sll1.protocol import LinuxSLL1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.link.linux_sll2.protocol import LinuxSLL2 # isort: skip # pylint: disable=import-outside-toplevel - - if isinstance(data, Data_LinuxSLL1): - return cast('Self', LinuxSLL1.from_data(data, **kwargs)) - if isinstance(data, Data_LinuxSLL2): - return cast('Self', LinuxSLL2.from_data(data, **kwargs)) + if isinstance(data, _DATA_LINUX_SLL1.klass): + return cast('Self', _LINUX_SLL1.klass.from_data(data, **kwargs)) + if isinstance(data, _DATA_LINUX_SLL2.klass): + return cast('Self', _LINUX_SLL2.klass.from_data(data, **kwargs)) raise ProtocolError('LinuxSLL: invalid data: must be a LinuxSLL1 or LinuxSLL2 data model') ########################################################################## @@ -399,12 +411,9 @@ def _member_of_version(version: 'Any') -> 'Type[LinuxSLL]': :obj:`bool` or a :obj:`float` is refused even where it equals one. """ - from pcapkit.protocols.link.linux_sll1.protocol import LinuxSLL1 # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.link.linux_sll2.protocol import LinuxSLL2 # isort: skip # pylint: disable=import-outside-toplevel - if isinstance(version, int) and not isinstance(version, bool): if version == 1: - return cast('Type[LinuxSLL]', LinuxSLL1) + return cast('Type[LinuxSLL]', _LINUX_SLL1.klass) if version == 2: - return cast('Type[LinuxSLL]', LinuxSLL2) + return cast('Type[LinuxSLL]', _LINUX_SLL2.klass) raise ProtocolError(f'LinuxSLL: invalid version: {version!r}') diff --git a/pcapkit/protocols/misc/checksum/ip_pseudo/protocol.py b/pcapkit/protocols/misc/checksum/ip_pseudo/protocol.py index ad6bfadf2b..670870f97c 100644 --- a/pcapkit/protocols/misc/checksum/ip_pseudo/protocol.py +++ b/pcapkit/protocols/misc/checksum/ip_pseudo/protocol.py @@ -61,6 +61,7 @@ from pcapkit.const.ipv6.extension_header import ExtensionHeader as Enum_ExtensionHeader from pcapkit.const.ipv6.option import Option as Enum_Option from pcapkit.const.reg.transtype import TransType as Enum_TransType +from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain from pcapkit.corekit.schema.ipaddress import parse_ip_address from pcapkit.corekit.schema.jumbogram import jumbo_payload_length @@ -80,8 +81,15 @@ from typing_extensions import Literal + from pcapkit.protocols.internet.ah.protocol import AH + __all__ = ['IP_Pseudo', 'pseudo_header', 'verify_upper'] +#: The Authentication Header :func:`verify_upper` looks past to the upper layer, +#: resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_AH: 'ModuleDescriptor[AH]' = ModuleDescriptor('pcapkit.protocols.internet.ah.protocol', 'AH') + #: Extension headers that end the chain, as the upper layer -- the Mobility Header #: (:rfc:`6275#section-6.1.1`) and HIP (:rfc:`7401#section-5.1.1`). _TERMINAL_HEADERS = (Enum_ExtensionHeader.Mobility_Header, Enum_ExtensionHeader.HIP) @@ -666,11 +674,10 @@ def verify_upper(ip: 'IPv4 | IPv6') -> 'None': ip: Parsed IPv4 or IPv6 layer. """ - from pcapkit.protocols.internet.ah.protocol import AH # isort: skip # pylint: disable=import-outside-toplevel - + ah = _AH.klass upper = ip.payload # NOTE: IPv6 walks an AH as an extension header; IPv4 carries it as its payload. - while isinstance(upper, AH): + while isinstance(upper, ah): upper = upper.payload verify = getattr(upper, '_verify_pseudo_checksum', None) if verify is None: diff --git a/pcapkit/protocols/transport/dccp/protocol.py b/pcapkit/protocols/transport/dccp/protocol.py index 244a37afdf..a85776b6fe 100644 --- a/pcapkit/protocols/transport/dccp/protocol.py +++ b/pcapkit/protocols/transport/dccp/protocol.py @@ -113,6 +113,12 @@ __all__ = ['DCCP'] +#: The pseudo-header a payload's checksum is computed over in :meth:`DCCP.make`, +#: resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_PSEUDO_HEADER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'pseudo_header') + #: Longest a DCCP header can be -- a Data Offset of 255 words, :rfc:`4340#section-5`. DCCP_MAX_HEADER_LEN = 1020 @@ -465,7 +471,7 @@ def make(self, self._checksum_pending = True else: from pcapkit.corekit.checksum.internet import compute_checksum # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import pseudo_header # isort: skip # pylint: disable=import-outside-toplevel + pseudo_header = _PSEUDO_HEADER.klass octets = schema.pack() coverage = self._checksum_coverage(cscov, hdr_len, len(octets)) diff --git a/pcapkit/protocols/transport/tcp/protocol.py b/pcapkit/protocols/transport/tcp/protocol.py index 661586dcea..f9bd43c286 100644 --- a/pcapkit/protocols/transport/tcp/protocol.py +++ b/pcapkit/protocols/transport/tcp/protocol.py @@ -169,6 +169,12 @@ __all__ = ['TCP'] +#: The pseudo-header a payload's checksum is computed over in :meth:`TCP.make`, +#: resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_PSEUDO_HEADER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'pseudo_header') + class TCP(Transport[Data_TCP, Schema_TCP], schema=Schema_TCP, data=Data_TCP): @@ -776,7 +782,7 @@ def make(self, self._checksum_pending = True else: from pcapkit.corekit.checksum.internet import compute_checksum # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import pseudo_header # isort: skip # pylint: disable=import-outside-toplevel + pseudo_header = _PSEUDO_HEADER.klass octets = schema.pack() schema.checksum = compute_checksum(octets, prefix=pseudo_header(pseudo, len(octets))) return schema diff --git a/pcapkit/protocols/transport/udp/protocol.py b/pcapkit/protocols/transport/udp/protocol.py index 74b6fd94bc..4ffdad5283 100644 --- a/pcapkit/protocols/transport/udp/protocol.py +++ b/pcapkit/protocols/transport/udp/protocol.py @@ -46,6 +46,12 @@ __all__ = ['UDP'] +#: The pseudo-header a payload's checksum is computed over in :meth:`UDP.make`, +#: resolved there rather than imported, so its first import holds +#: :data:`~pcapkit.utilities.importlib.import_lock` (:issue:`2771`). +_PSEUDO_HEADER: 'ModuleDescriptor[Any]' = \ + ModuleDescriptor('pcapkit.protocols.misc.checksum.ip_pseudo.protocol', 'pseudo_header') + class UDP(Transport[Data_UDP, Schema_UDP], schema=Schema_UDP, data=Data_UDP): @@ -303,7 +309,7 @@ def make(self, self._checksum_pending = True else: from pcapkit.corekit.checksum.internet import compute_checksum # isort: skip # pylint: disable=import-outside-toplevel - from pcapkit.protocols.misc.checksum.ip_pseudo.protocol import pseudo_header # isort: skip # pylint: disable=import-outside-toplevel + pseudo_header = _PSEUDO_HEADER.klass # NOTE: Over the datagram the Length field gives, or all but the trailer # for a jumbogram's zero one (RFC 2675); a computed zero is all ones. diff --git a/tests/project/test_function_level_first_imports_2771_unit.py b/tests/project/test_function_level_first_imports_2771_unit.py new file mode 100644 index 0000000000..109922c9ef --- /dev/null +++ b/tests/project/test_function_level_first_imports_2771_unit.py @@ -0,0 +1,151 @@ +# -*- coding: utf-8 -*- +"""No function body first-imports a module ``import pcapkit`` leaves unloaded (GitHub issue #2771). + +Such an import runs without :data:`pcapkit.utilities.importlib.import_lock`, so it can +deadlock the import system against a thread importing the same package under the lock: +one takes a module's import lock before waiting on its package, the other the package's +before the module's. A function body reaches such a module through a +:class:`~pcapkit.corekit.module.ModuleDescriptor` instead, whose first resolution holds +the lock, as the members of HTTP, LinuxSLL, L2TP and IGMP, the port registries, the +IPv4 and IPv6 pseudo-header helpers and the default engines' toolkits do. + +The sweep reads every module under :file:`pcapkit/` with :mod:`ast`, so an import is +seen whether or not anything calls the function holding it, and asks a fresh +interpreter which modules ``import pcapkit`` loads. A package the importing module sits +in is always loaded before it runs. ``ALLOWED_MODULES`` and ``ALLOWED_SITES`` hold what +is left as an import, each with its reason, and an entry that matches no import fails. + +""" +from __future__ import annotations + +import ast +import json +import os +import pathlib +import subprocess +import sys +import unittest +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Iterator + +#: Test tier (#1958): reads the sources, and asks one interpreter what ``import pcapkit`` loads. +__tier__ = 'LATEST' + +ROOT = pathlib.Path(__file__).resolve().parents[2] + +#: Modules a function body may import first, and why. +ALLOWED_MODULES = { + 'pcapkit.corekit.checksum.internet': ( + 'imports nothing from pcapkit.protocols (#2580), and every import of it names the ' + 'module itself, so the import locks it takes always come in one order'), +} + +#: Function bodies that may import a module first, by file and function, and why. +ALLOWED_SITES = { + ('pcapkit/protocols/link/ethernet/schema.py', 'callback_payload'): ( + "its own package's protocol module, which the package imports before its schema"), + ('pcapkit/interface/misc.py', 'follow_tcp_stream'): ( + 'the toolkit of the engine named, as 1.5.1 imported it; the third-party engines are ' + 'off the dissection path'), + **{(f'pcapkit/foundation/engines/{engine}.py', function): ( + 'a third-party engine imports its toolkit as 1.5.1 did, off the dissection path') + for engine, functions in { + 'dpkt': ('_pcap_reader', '_read_packet', '_read_simple', 'read_frame', 'run'), + 'pcap_ct': ('read_frame', 'run'), 'pypcap': ('read_frame', 'run'), + 'pypcapfile': ('read_frame', 'run'), 'pyshark': ('read_frame',), + 'scapy': ('_reader_type', 'read_frame', 'run'), + }.items() for function in functions}, +} + + +def loaded() -> 'set[str]': + """The modules a fresh ``import pcapkit`` loads, from this tree.""" + environ = dict(os.environ) + environ.pop('PYTHONSAFEPATH', None) + environ['PYTHONPATH'] = os.pathsep.join( + [str(ROOT), environ['PYTHONPATH']] if environ.get('PYTHONPATH') else [str(ROOT)]) + code = ('import json, sys, pcapkit\n' + 'print(json.dumps([pcapkit.__file__, sorted(sys.modules)]))') + result = subprocess.run([sys.executable, '-c', code], cwd=str(ROOT), env=environ, # nosec: B603 + capture_output=True, text=True, timeout=300, check=True) + origin, names = json.loads(result.stdout.splitlines()[-1]) + assert origin.startswith(str(ROOT)), origin + return set(names) + + +#: Every module under :file:`pcapkit/`, by dotted name, a package by its ``__init__``. +MODULES = frozenset('.'.join(path.relative_to(ROOT).with_suffix('').parts) + for path in (ROOT / 'pcapkit').rglob('*.py')) + + +def _is_type_checking(node: 'ast.AST') -> 'bool': + """Whether ``node`` is an ``if TYPE_CHECKING:`` or ``if typing.TYPE_CHECKING:`` block.""" + test = getattr(node, 'test', None) + return isinstance(node, ast.If) and (isinstance(test, ast.Name) and test.id == 'TYPE_CHECKING' + or isinstance(test, ast.Attribute) and test.attr == 'TYPE_CHECKING') + + +def imports(path: 'pathlib.Path') -> 'Iterator[tuple[str, str]]': + """Each function in ``path`` with a :mod:`pcapkit` module its body imports, outside ``TYPE_CHECKING``.""" + def walk(node: 'ast.AST', function: 'str') -> 'Iterator[tuple[str, str]]': + for child in ast.iter_child_nodes(node): + if _is_type_checking(child): + continue + name = function + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)): + name = child.name if not function else f'{function}.{child.name}' + if name and isinstance(child, ast.Import): + yield from ((name, alias.name) for alias in child.names if alias.name.startswith('pcapkit')) + if name and isinstance(child, ast.ImportFrom) and not child.level and child.module \ + and child.module.startswith('pcapkit'): + yield name, child.module + yield from ((name, f'{child.module}.{alias.name}') for alias in child.names + if f'{child.module}.{alias.name}' in MODULES + or f'{child.module}.{alias.name}.__init__' in MODULES) + yield from walk(child, name) + + yield from walk(ast.parse(path.read_text(encoding='utf-8')), '') + + +def first_imports() -> 'dict[tuple[str, str], set[str]]': + """Each function, by file and name, with the modules its body imports first.""" + warm, found = loaded(), {} # type: set[str], dict[tuple[str, str], set[str]] + for path in sorted((ROOT / 'pcapkit').rglob('*.py')): + rel = path.relative_to(ROOT).as_posix() + parts = path.relative_to(ROOT).with_suffix('').parts + packages = {'.'.join(parts[:index]) for index in range(1, len(parts))} + for function, module in imports(path): + if module not in warm and module not in packages: + found.setdefault((rel, function), set()).add(module) + return found + + +class FunctionLevelFirstImportTests(unittest.TestCase): + """Every first import a function body makes is a descriptor's, or allowed.""" + + @classmethod + def setUpClass(cls) -> 'None': + cls.found = first_imports() + + def test_no_function_body_imports_a_module_first(self) -> 'None': + unexpected = sorted(f'{rel}:{function} imports {module}' + for (rel, function), modules in self.found.items() + if (rel, function) not in ALLOWED_SITES + for module in sorted(modules) if module not in ALLOWED_MODULES) + self.assertEqual(unexpected, [], 'resolve these through a ModuleDescriptor, whose first ' + 'resolution holds pcapkit.utilities.importlib.import_lock') + + def test_every_allowance_names_an_import(self) -> 'None': + modules = set().union(*self.found.values()) + self.assertEqual(sorted(set(ALLOWED_MODULES) - modules), []) + self.assertEqual(sorted(set(ALLOWED_SITES) - set(self.found)), []) + + def test_the_sweep_sees_function_level_imports(self) -> 'None': + source = ROOT / 'pcapkit' / 'protocols' / 'link' / 'ethernet' / 'schema.py' + self.assertIn(('callback_payload', 'pcapkit.protocols.link.ethernet.protocol'), set(imports(source))) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/utilities/test_lazy_exports_threads_2771_unit.py b/tests/utilities/test_lazy_exports_threads_2771_unit.py index 9b91981007..55568f1b3f 100644 --- a/tests/utilities/test_lazy_exports_threads_2771_unit.py +++ b/tests/utilities/test_lazy_exports_threads_2771_unit.py @@ -28,6 +28,7 @@ import struct import subprocess import sys +import tempfile import unittest from typing import TYPE_CHECKING @@ -49,10 +50,11 @@ #: The child: from a thread each, all at once, reach every unbound export of #: ``packages``, every unbound submodule of ``submodules`` an export comes from, and -#: each of ``names``; parse each of ``frames`` from an Ethernet header; and look up -#: each protocol not imported yet with ``get_protocol`` if ``registry`` is set. Then -#: print what failed, hung, was left unbound, parsed or resolved to the wrong class, -#: or was imported for the first time without the lock held. +#: each of ``names``; parse each of ``frames`` from an Ethernet header; look up each +#: protocol not imported yet with ``get_protocol`` if ``registry`` is set; and, given a +#: ``capture``, make each of its ``calls``. Then print what failed, hung, was left +#: unbound, parsed or resolved to the wrong class, or was imported for the first time +#: without the lock held. CHILD = ''' import importlib, io, json, os, sys, threading, time @@ -65,6 +67,36 @@ if spec['frames']: from pcapkit.protocols.link.ethernet.protocol import Ethernet +calls = {} +if spec['capture']: + # NOTE: imported first without the lock on purpose: its package imports nothing + # from pcapkit.protocols, so no import-lock cycle runs through it. + import pcapkit.corekit.checksum.internet + from pcapkit.protocols.application.http.protocol import HTTP + from pcapkit.protocols.internet.ipv4.protocol import IPv4 + from pcapkit.protocols.link.linux_sll.protocol import LinuxSLL + from pcapkit.protocols.transport.udp.protocol import UDP + from pcapkit.utilities.exceptions import ProtocolError + + def from_data(): + try: + HTTP.from_data(object()) + except ProtocolError: + return + raise AssertionError('HTTP.from_data took an object') + + calls = { + 'members': lambda: [LinuxSLL._member_of_version(1), LinuxSLL._member_of_version(2), + ProtocolBase.get_protocol('L2TP')._member_of_version(2)], + 'from_data': from_data, + 'index': lambda: [ProtocolBase.get_protocol(name).__index__() + for name in ('DTLS', 'FTP', 'FTP_DATA', 'HTTP', 'TLS', 'L2TPv2')], + 'compute': lambda: IPv4(src='10.0.0.1', dst='10.0.0.2', protocol=17, + payload=UDP(srcport=1, dstport=2, payload=b'ab')), + 'extract': lambda: pcapkit.extract(fin=spec['capture'], nofile=True, reassembly=True, + verbose=False), + } + jobs = [('attr',) + tuple(target) for target in spec['names']] for package in spec['packages']: module = importlib.import_module(package) @@ -78,6 +110,8 @@ if spec['registry']: jobs.extend(('get', name, entry) for name, entry in ProtocolBase.__registry__.items() if entry.module not in sys.modules) +for name in calls: + jobs.extend([('call', name, None)] * 2) barrier = threading.Barrier(len(jobs)) errors, values, wrong, unlocked = [], {}, [], [] @@ -104,6 +138,8 @@ def run(kind, key, arg): layer = layer.payload if key not in names: wrong.append(f'{key}: {names}') + elif kind == 'call': + calls[key]() elif ProtocolBase.get_protocol(key) is not getattr(sys.modules[arg.module], arg.name): wrong.append(key) @@ -185,17 +221,35 @@ def udp(port: 'int', payload: 'bytes' = b'') -> 'bytes': ('L2TPv2', ether(0x0800, ipv4(17, udp(1701, struct.pack('>HHH', 0x0002, 1, 1))))), ] +#: A frame per helper its first parse resolves with checksums verified: HTTP's member +#: for an HTTP/1.1 request, and the pseudo-header check over TCP, over UDP, and past +#: an Authentication Header. +HELPER_FRAMES = [ + ('HTTPv1', ether(0x0800, ipv4(6, struct.pack('>HHIIBBHHH', 1234, 80, 1, 0, 5 << 4, 0x18, 1024, 0, 0) + + b'GET / HTTP/1.1\r\nHost: a\r\n\r\n'))), + ('AH', ether(0x0800, ipv4(51, bytes([17, 1, 0, 0]) + struct.pack('>II', 1, 1) + udp(53)))), + ('UDP', ether(0x0800, ipv4(17, udp(53, b'abcd')))), +] + def run_trial(packages: 'list[str]', submodules: 'list[str]', names: 'tuple[tuple[str, str], ...]' = (), *, - frames: 'bool' = False, registry: 'bool' = False) -> 'dict[str, Any]': - """Run :data:`CHILD` in a fresh interpreter; return what it printed.""" + frames: 'bool' = False, registry: 'bool' = False, capture: 'str' = '') -> 'dict[str, Any]': + """Run :data:`CHILD` in a fresh interpreter; return what it printed. + + With a ``capture``, the frames are :data:`HELPER_FRAMES`, parsed with checksums + verified, and the calls are made, one of which extracts ``capture``. + + """ environ = dict(os.environ) environ.pop('PYTHONSAFEPATH', None) environ['PYTHONPATH'] = os.pathsep.join( [str(ROOT), environ['PYTHONPATH']] if environ.get('PYTHONPATH') else [str(ROOT)]) + if capture: + environ['PCAPKIT_CHECKSUM_VERIFY'] = '1' + chosen = HELPER_FRAMES if capture else FRAMES if frames else [] spec = json.dumps({'root': str(ROOT), 'packages': packages, 'submodules': submodules, 'names': names, - 'frames': [(member, octets.hex()) for member, octets in FRAMES] if frames else [], - 'registry': registry, 'deadline': DEADLINE}) + 'frames': [(member, octets.hex()) for member, octets in chosen], + 'registry': registry, 'capture': capture, 'deadline': DEADLINE}) result = subprocess.run([sys.executable, '-c', CHILD, spec], # nosec: B603 cwd=str(ROOT), env=environ, capture_output=True, text=True, timeout=DEADLINE + 240, check=False) @@ -237,6 +291,18 @@ def test_get_protocol_racing_first_dispatches_resolves_every_name(self) -> 'None for out in outs: self.assertEqual(out['kinds'], ['get', 'parse']) + def test_member_and_helper_first_imports_hold_the_lock(self) -> 'None': + """Members, data models, port registries, checksum helpers and toolkit, each first resolved at once.""" + with tempfile.TemporaryDirectory() as directory: + capture = os.path.join(directory, 'helpers.pcap') + with open(capture, 'wb') as file: + file.write(struct.pack('