diff --git a/docs/source/pcapkit/foundation/engines/3rdparty.rst b/docs/source/pcapkit/foundation/engines/3rdparty.rst index 988c5e17ef..394fd5c742 100644 --- a/docs/source/pcapkit/foundation/engines/3rdparty.rst +++ b/docs/source/pcapkit/foundation/engines/3rdparty.rst @@ -489,6 +489,11 @@ cycle. .. automethod:: describe +.. autoclass:: pcapkit.foundation.engines._pcap_backend.Memo + :no-members: + + .. autoattribute:: found + .. autofunction:: pcapkit.foundation.engines._pcap_backend.probe .. autofunction:: pcapkit.foundation.engines._pcap_backend.identify diff --git a/pcapkit/foundation/engines/_pcap_backend.py b/pcapkit/foundation/engines/_pcap_backend.py index 8708c67042..eda46e1b96 100644 --- a/pcapkit/foundation/engines/_pcap_backend.py +++ b/pcapkit/foundation/engines/_pcap_backend.py @@ -27,6 +27,7 @@ nothing from :mod:`pcapkit`, so it cannot introduce an import cycle. """ +import contextvars import importlib import importlib.metadata import sys @@ -36,7 +37,7 @@ __all__ = [ 'PYPCAP', 'PCAP_CT', 'DISTRIBUTIONS', 'ENGINE_NAMES', - 'Probe', 'probe', 'identify', 'installed_distributions', + 'Probe', 'Memo', 'probe', 'identify', 'installed_distributions', 'wrong_backend_reason', 'collision_reason', ] @@ -190,14 +191,51 @@ def _purge() -> 'None': del sys.modules[name] +class Memo: + """One extraction's :func:`probe`, taken at most once (:issue:`2113`). + + Engine selection asks + :meth:`~pcapkit.foundation.engines.engine.EngineBase.unsupported_reason` in + the :class:`~pcapkit.foundation.extraction.Extractor` constructor and again in + :meth:`~pcapkit.foundation.extraction.Extractor.run`, and the engine's own + constructor probes once more. While a memo is entered, :func:`probe` answers + from it, probing on first use. Each extraction holds its own memo, so nothing + is cached across extractions. + + """ + + __slots__ = ('found', '_tokens') + + def __init__(self) -> 'None': + #: The probe taken under this memo, or :data:`None` before the first. + self.found = None # type: Optional[Probe] + self._tokens = [] # type: list[contextvars.Token[Optional[Memo]]] + + def __enter__(self) -> 'Memo': + self._tokens.append(_MEMO.set(self)) + return self + + def __exit__(self, *exc: 'object') -> 'None': + _MEMO.reset(self._tokens.pop()) + + +#: The :class:`Memo` entered in this context, if any. A +#: :class:`~contextvars.ContextVar` so that one thread's extraction never answers +#: another's. +_MEMO: 'contextvars.ContextVar[Optional[Memo]]' = contextvars.ContextVar('_MEMO', default=None) + + def probe() -> 'Probe': """Import |pcap|_ and report what was found. - Not cached. The cost after a successful first call is a :data:`sys.modules` - lookup plus the ``dist-info`` scan in :func:`installed_distributions`, and a - cache would make the answer depend on when it was first asked -- - which the tests, and anything that manipulates :data:`sys.path`, would have to - work around. It is idempotent instead, via :func:`_purge`. + Not cached, except within a :class:`Memo`, which an extraction enters so + that its engine selection and engine constructor share one probe + (:issue:`2113`). The cost after a successful first call is a + :data:`sys.modules` lookup plus the ``dist-info`` scan in + :func:`installed_distributions`, and a process-wide cache would make the + answer depend on when it was first asked -- which the tests, and anything that + manipulates :data:`sys.path`, would have to work around. It is idempotent + instead, via :func:`_purge`. Note: The bare ``except Exception`` is deliberate and is much of the point of @@ -214,6 +252,16 @@ def probe() -> 'Probe': A :class:`Probe` describing the outcome. """ + memo = _MEMO.get() + if memo is None: + return _probe() + if memo.found is None: + memo.found = _probe() + return memo.found + + +def _probe() -> 'Probe': + """:func:`probe`, without consulting a :class:`Memo`.""" installed = installed_distributions() try: diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index 0705360d88..64a2116564 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -30,6 +30,7 @@ from pcapkit.corekit.io import PeekableStream, SeekableReader from pcapkit.corekit.module import ModuleDescriptor from pcapkit.dumpkit.common import make_dumper +from pcapkit.foundation.engines import _pcap_backend from pcapkit.foundation.engines.engine import Engine, EngineBase from pcapkit.foundation.engines.pcap import PCAP as PCAP_Engine from pcapkit.foundation.engines.pcapng import PCAPNG as PCAPNG_Engine @@ -277,6 +278,9 @@ class Extractor(Generic[_P]): _exnam: 'Engines' #: Extraction engine instance. _exeng: 'EngineBase[_P]' + #: The pcap backend probe, shared by engine selection and the engine's + #: constructor so that an extraction probes once (:issue:`2113`). + _exprb: '_pcap_backend.Memo' #: Input file object. _ifile: 'BufferedReader' @@ -865,7 +869,8 @@ def run(self) -> 'None': # pylint: disable=inconsistent-return-statements eng = self._select_engine() if eng is not None: logger.debug('using engine %s (%s)', eng.name, eng.module) - self._exeng = eng(self) + with self._exprb: + self._exeng = eng(self) self._exeng.run() self._open_output() @@ -927,7 +932,13 @@ def _select_engine(self) -> 'Optional[Type[EngineBase]]': # failure to escape from the engine's constructor instead. The base # class answers :data:`None`, so only engines with a real limitation # override it. - reason = eng.unsupported_reason() + # + # NOTE: one probe serves both calls of this method and the engine's + # constructor in ``run``, rather than one each (#2113). + if not hasattr(self, '_exprb'): + self._exprb = _pcap_backend.Memo() + with self._exprb: + reason = eng.unsupported_reason() if reason is not None: logger.debug('engine %s is unavailable: %s', eng.name, reason) warn(f'engine {eng.name} is not supported on this interpreter ' diff --git a/tests/foundation/engines/test_pcap_backend_probe_once_2113_unit.py b/tests/foundation/engines/test_pcap_backend_probe_once_2113_unit.py new file mode 100644 index 0000000000..ab0e26b75a --- /dev/null +++ b/tests/foundation/engines/test_pcap_backend_probe_once_2113_unit.py @@ -0,0 +1,118 @@ +# -*- coding: utf-8 -*- +"""An extraction probes the pcap backend once, not three times (#2113). + +Engine selection asked ``unsupported_reason`` in the constructor and again in +``run``, and the engine's constructor probed a third time; each probe scans the +installed distributions. The probe is shared within one extraction only, so a +later extraction, or a direct ``unsupported_reason`` call, still asks afresh. + +""" + +from __future__ import annotations + +import importlib.util +import sys +import types +import unittest +import warnings +from unittest import mock + +from tests._support import reimport_once_per_class, sample_path + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + + +class FakeHandle: + """Stand-in for :class:`pcap.pcap`, yielding one ``(timestamp, bytes)`` pair.""" + + def __init__(self, name=None, promisc=False) -> None: + self._iter = iter([(1.5, b'payload')]) + + def datalink(self) -> int: + return 1 # LINKTYPE_ETHERNET + + def __iter__(self): + return self + + def __next__(self): + return next(self._iter) + + def close(self) -> None: + pass + + +def fake_pcap_modules(*, is_pcap_ct: bool) -> 'dict[str, types.ModuleType]': + """A stand-in :mod:`pcap`, shaped as ``pcap-ct`` or as upstream ``pypcap``.""" + module = types.ModuleType('pcap') + module.pcap = FakeHandle # type: ignore[attr-defined] + module.__version__ = '1.3.0b3' if is_pcap_ct else '1.3.0' # type: ignore[attr-defined] + module.__file__ = ('/stub/site-packages/pcap/__init__.py' if is_pcap_ct + else '/stub/site-packages/pcap.cpython-310.so') + modules = {'pcap': module} + if is_pcap_ct: + submodule = types.ModuleType('pcap._pcap') + module._pcap = submodule # type: ignore[attr-defined] + modules['pcap._pcap'] = submodule + return modules + + +@unittest.skipUnless(HAS_RUNTIME, 'pcapkit runtime dependencies are not installed') +class TestProbeOnce(unittest.TestCase): + """One ``installed_distributions`` scan per extraction through a pcap engine.""" + + CASES = ( + ('pypcap', 'PyPCAP', ('pypcap',), False), + ('pcap_ct', 'PCAP_CT', ('pcap-ct',), True), + ) + + def setUp(self) -> None: + reimport_once_per_class(self) + + def _extract(self, engine: str, installed: 'tuple[str, ...]', is_pcap_ct: bool): + """Extract ``in.pcap`` with ``engine``; return it, the scan count, and warnings.""" + import pcapkit + from pcapkit.foundation.engines import _pcap_backend + + with mock.patch.dict(sys.modules, fake_pcap_modules(is_pcap_ct=is_pcap_ct)), \ + mock.patch.object(_pcap_backend, 'installed_distributions', + return_value=installed) as scan, \ + warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + extraction = pcapkit.extract(fin=sample_path('in.pcap'), engine=engine, + store=True, nofile=True) + return extraction, scan.call_count, [str(w.message) for w in caught] + + def test_one_extraction_probes_once(self) -> None: + for engine, name, installed, is_pcap_ct in self.CASES: + with self.subTest(engine=engine): + extraction, scans, _ = self._extract(engine, installed, is_pcap_ct) + self.assertEqual(type(extraction.engine).__engine_name__, name) + self.assertEqual(scans, 1) + + def test_collision_is_still_warned_once(self) -> None: + # The engine's constructor warns from the shared probe, not a fresh one. + _, scans, messages = self._extract('pcap_ct', ('pypcap', 'pcap-ct'), True) + self.assertEqual(scans, 1) + collisions = [m for m in messages if 'installed together' in m] + self.assertEqual(len(collisions), 1, messages) + self.assertTrue(collisions[0].startswith("'Extractor(engine=pcap_ct)': pypcap and " + 'pcap-ct are installed together')) + + def test_probe_is_not_cached_across_extractions(self) -> None: + from pcapkit.foundation.engines import _pcap_backend + from pcapkit.foundation.engines.pcap_ct import PCAP_CT + + for _ in range(2): + self.assertEqual(self._extract('pcap_ct', ('pcap-ct',), True)[1], 1) + # Outside an extraction the public hook asks afresh every time. + with mock.patch.dict(sys.modules, fake_pcap_modules(is_pcap_ct=True)), \ + mock.patch.object(_pcap_backend, 'installed_distributions', + return_value=('pcap-ct',)) as scan: + PCAP_CT.unsupported_reason() + PCAP_CT.unsupported_reason() + self.assertEqual(scan.call_count, 2) + + +if __name__ == '__main__': + unittest.main()