protocols: stop option, chunk and block dispatch writing to shared registries by JarryShaw · Pull Request #428 · JarryShaw/PyPCAPKit

JarryShaw · 2026-09-17T03:23:31Z

Closes #425.

The issue named eight registries; there are sixteen

The eight in #425 were a sample. Enumerating every class-level defaultdict used for method-name dispatch, then grepping every read of each, gives 16 registries across 39 read sites in 8 files — the issue's read-site list had only sampled the parse side of each, missing the _make_* twins.

Beyond the eight named: TCP.__mp_option__, SCTP.__parameter__, IPv6_Route.__routing__, MH.__message__, MH.__extension__, HTTPv2.__frame__, PCAPNG.__record__, PCAPNG.__secrets__.

File	Registries	Read sites
`transport/tcp.py`	`__option__`, `__mp_option__`	5
`transport/sctp.py`	`__chunk__`, `__parameter__`, `__cause__`	6
`internet/hopopt.py`	`__option__`	3
`internet/ipv6_opts.py`	`__option__`	3
`internet/ipv6_route.py`	`__routing__`	2
`internet/mh.py`	`__message__`, `__option__`, `__extension__`	8
`application/httpv2.py`	`__frame__`	2
`misc/pcapng.py`	`__block__`, `__option__`, `__record__`, `__secrets__`	10

Two that look like family members and are not: IPv4.register_option and HIP.register_parameter dispatch by setattr on the class rather than through a dict, so there is nothing to leak — even though register_ipv4_option/register_hip_parameter exist and warn on overwrite. esp.py:285's registry[candidate] is an enum class, not a defaultdict. Extractor.__output__ and TraceFlow.__output__ are untouched: their injection-on-miss is load-bearing memoisation, tried and deliberately reverted earlier.

Before → after

All 16, on a git archive of origin/main versus this branch:

BEFORE                                          AFTER
[LEAK] TCP.__option__         leaked=['156']    [ok] leaked=[]  register=[]
[LEAK] TCP.__mp_option__      leaked=['15']     [ok] leaked=[]  register=[]
[LEAK] SCTP.__chunk__         leaked=['200']    [ok] leaked=[]  register=[]
[LEAK] SCTP.__parameter__     leaked=['65520']  [ok] leaked=[]  register=[]
[LEAK] SCTP.__cause__         leaked=['65520']  [ok] leaked=[]  register=[]
[LEAK] HOPOPT.__option__      leaked=['30']     [ok] leaked=[]  register=[]
[LEAK] IPv6_Opts.__option__   leaked=['30']     [ok] leaked=[]  register=[]
[LEAK] IPv6_Route.__routing__ leaked=['253']    [ok] leaked=[]  register=[]
[LEAK] MH.__message__         leaked=['200']    [ok] leaked=[]  register=[]
[LEAK] MH.__option__          leaked=['64']     [ok] leaked=[]  register=[]
[LEAK] MH.__extension__       leaked=['255']    [ok] leaked=[]  register=[]
[LEAK] HTTPv2.__frame__       leaked=['240']    [ok] leaked=[]  register=[]
[LEAK] PCAPNG.__block__       leaked=['7']      [ok] leaked=[]  register=[]
[LEAK] PCAPNG.__option__      leaked=[('opt_unknown', 42)]  [ok] leaked=[]
[LEAK] PCAPNG.__record__      leaked=['2989']   [ok] leaked=[]  register=[]
[LEAK] PCAPNG.__secrets__     leaked=['195935983']  [ok] leaked=[]
16 leaked                                       0 leaked

Every "before" register_* warned ('chunk 200 already registered, overwriting', 'PCAP-NG: [Type 7] block already registered', …); every "after" one is silent. I spot-checked three of these independently on both trees — TCP.__option__, HOPOPT.__option__, MH.__message__ — and they reproduce exactly.

What was generalised

One helper, ProtocolBase._lookup_registry(registry, code) — the non-recording defaultdict read — with #426's _lookup_next_layer rewritten on top of it. #426's helper did two things and only the lookup generalises, because these registries hold method names or (parser, constructor) pairs with no ModuleDescriptor to resolve. So the lookup became the primitive and resolution stayed in _lookup_next_layer.

Its write-back is now explicitly guarded by if proto in registry: a descriptor obtained from the default factory has no key to memoise under, and writing it back would recreate the very bug.

Two hand-written # type: str | tuple[...] comments are gone, since the helper is typed off each registry's own annotation.

One commit, because the change is a helper plus the same substitution 39 times; split per family, seven of eight commits would be that substitution with the helper already landed.

Verification

30/30 byte-identical dumps — 15 captures × tree+json with ip=True, tcp=True, reassembly=True, against a git archive of origin/main, diff -rq clean.
Suite 865 passed, 17 skipped, 848 subtests, 0 failed (re-run independently). Baseline with the fix but no new tests is 857/17, so +8 tests and nothing regressed.
All eight new tests fail on a pristine tree — copied in and checked, 17 failures counting subtests, rather than assumed.
mypy identical set; isort unchanged; pylint identical except R0401, whose cycle enumeration is unstable on the pristine tree too (127 then 116 across two runs).

Three things found and deliberately not fixed

The schema-side EnumSchema.__enum__ registries leak in lock-step. The same kind-156 segment also inserts into schema.transport.tcp.Option.registry; the same HOPOPT datagram into schema.internet.hopopt.Option.registry. Read sites are corekit/fields/collections.py:264 and ~12 schema selector callbacks. Different layer, so the helper's home differs — and EnumSchema.register does not warn on overwrite, so the symptom is accretion and an un-inspectable registry rather than a spurious warning. Wants a follow-up issue, split the same way Option, chunk and block registries leak on lookup miss, the same way __proto__ did #425 was split out of protocols: stop next-layer dispatch writing to shared registries, and fix two transport defects #426.
MPTCPUnknown.data and CGAParameter.extensions raise KeyError: 'length' before their registry is even read: both size themselves from pkt['length'], a key the nested packet context does not carry (corekit/fields/misc.py:619 passes {'__packet__': packet}). So an unknown MPTCP subtype and any CGA Parameters option fail in the schema layer. Two separate latent defects, each worth its own issue.

…gistries Closes #425. Same defect as #421, on the sibling registry family: every `__option__` / `__mp_option__` / `__chunk__` / `__parameter__` / `__cause__` / `__routing__` / `__message__` / `__extension__` / `__frame__` / `__block__` / `__record__` / `__secrets__` read indexed a class-level `defaultdict`, so a lookup **miss inserted the key**. One packet with an unrecognised code was enough to grow a registry shared by every instance in the process, and to make the next genuine registration report an overwrite that never happened. The issue names eight registries. A sweep by inspection found **sixteen**, across 39 read sites in 8 files -- both the parse and the construct side of each dispatch, which the read-site list in the issue only sampled: TCP.__option__ __mp_option__ SCTP.__chunk__ __parameter__ __cause__ HOPOPT.__option__ IPv6_Opts.__option__ IPv6_Route.__routing__ MH.__message__ __option__ __extension__ HTTPv2.__frame__ PCAPNG.__block__ __option__ __record__ __secrets__ IPv4 and HIP have `register_ipv4_option` / `register_hip_parameter` and so look like members of the family, but they dispatch by `setattr` on the class rather than through a dict, so neither has the defect. **One generalised helper, `ProtocolBase._lookup_registry(registry, code)`**, rather than a sibling of `_lookup_next_layer`. What #426 put in the base was two things: a lookup that does not record a miss, and the next-layer-specific resolution of a `ModuleDescriptor` into a class. Only the first generalises -- these registries hold method names, or a `(parser, constructor)` pair, with nothing to resolve -- so the first is now the helper and `_lookup_next_layer` layers the resolution on top of it. Its write-back is confined to a hit, since memoising the fallback's resolution would recreate the very insertion the helper exists to avoid. **One commit, because the change is uniform**: a helper, then the same substitution at 39 sites. Split per family, seven of the eight commits would be that one substitution repeated with the helper already landed. Two of the sixteen are reachable only through construction, and the reason is a separate defect in each case: `MPTCPUnknown.data` and `CGAParameter.extensions` both size themselves from `pkt['length']`, a key the nested packet context does not carry, so an unknown MPTCP subtype and the CGA Parameters option raise in the schema layer before the registry is ever read. Filed separately; this change fixes the leak at the sibling read site in `_make_mode_mp` and `_make_cga_extensions`. Verification, all re-run rather than reasoned about: * Reproduction before -> after, per registry: 16 of 16 leaked one key per unrecognised code and made `register_*` warn; 0 of 16 now leak and every `register_*` is silent. `TCP.__option__` is the issue's own case -- parsing one segment with option kind 156 no longer leaves `Option.Reserved_156` behind, and `register_tcp_option(Option(156), ...)` no longer warns. * Eight new tests, one per protocol, asserting the key set is unchanged after a parse. All eight fail against a pristine `origin/main` -- checked, not assumed. * Byte comparison: 15 captures x `tree` + `json`, `ip=True, tcp=True, reassembly=True`, against a `git archive` of `origin/main`. **30/30 byte-identical**; this fix is not expected to change any output and does not. * Suite: **865 passed, 17 skipped, 848 subtests, 0 failed** (857/17 before the new tests; 839/35 on the pristine copy, where the extra 18 skips are `test_tier_guard.py` declining to run outside a git checkout). * mypy: 128 errors, the identical set to baseline, differing only in line numbers. Two hand-written `# type: str | tuple[...]` comments are dropped because the helper is typed off each registry's own annotation. isort: same three pre-existing findings. pylint: identical findings except `R0401`, whose cycle enumeration is unstable run to run (127 then 116 on the pristine tree alone, 122 here). Also noted, not fixed: the schema-side `EnumSchema.__enum__` registries leak in lock-step with these -- the same kind-156 segment inserts `Option.Reserved_156` into `schema.transport.tcp.Option.registry` too, via `OptionField.unpack` and the schema selector callbacks. Different layer, different owner, no `register` that warns; filed separately rather than folded in here.

Copilot

🟡 Changes recommended

A newly added TCP unit test constructs a segment with a header data offset that does not match the provided bytes, making the test depend on lenient parsing and potentially passing a negative payload length downstream.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR addresses #425 by preventing class-level defaultdict dispatch registries (options/chunks/blocks/etc.) from being mutated on lookup misses during parsing/construction, eliminating “phantom registrations” and follow-on overwrite warnings.

Changes:

Introduces ProtocolBase._lookup_registry(registry, code) for non-recording reads of defaultdict registries and refactors _lookup_next_layer to build on it.
Replaces registry[code] reads with _lookup_registry(...) across TCP/SCTP/IPv6 extensions/MH/HTTPv2/PCAPNG dispatch sites.
Adds targeted unit tests asserting that unregistered codes do not mutate class registries across the affected protocol families.

File summaries

File	Description
`pcapkit/protocols/protocol.py`	Adds `_lookup_registry` and rewrites `_lookup_next_layer` to avoid insertion-on-miss and constrain memoisation to hits.
`pcapkit/protocols/transport/tcp.py`	Routes TCP option and MPTCP subtype dispatch lookups through `_lookup_registry`.
`pcapkit/protocols/transport/sctp.py`	Routes SCTP chunk/parameter/cause dispatch lookups through `_lookup_registry`.
`pcapkit/protocols/internet/hopopt.py`	Uses `_lookup_registry` for IPv6 HOPOPT option dispatch.
`pcapkit/protocols/internet/ipv6_opts.py`	Uses `_lookup_registry` for IPv6 destination options dispatch.
`pcapkit/protocols/internet/ipv6_route.py`	Uses `_lookup_registry` for IPv6 routing type dispatch.
`pcapkit/protocols/internet/mh.py`	Uses `_lookup_registry` for MH message/option/extension dispatch.
`pcapkit/protocols/application/httpv2.py`	Uses `_lookup_registry` for HTTP/2 frame dispatch.
`pcapkit/protocols/misc/pcapng.py`	Uses `_lookup_registry` for PCAP-NG block/option/record/secrets dispatch.
`tests/protocols/test_protocol_base_unit.py`	Adds unit test coverage for `_lookup_registry` behavior (miss does not record).
`tests/protocols/transport/test_tcp_udp_unit.py`	Adds TCP `__option__` and `__mp_option__` non-mutation tests.
`tests/protocols/transport/test_sctp_unit.py`	Adds SCTP sub-registry non-mutation tests for chunk/parameter/cause.
`tests/protocols/internet/test_ipv6_extension_unit.py`	Adds IPv6 extension registries non-mutation tests (HOPOPT/Opts/Route).
`tests/protocols/internet/test_mh_unit.py`	Adds MH registries non-mutation tests (message/option/extension).
`tests/protocols/application/test_http_unit.py`	Adds HTTP/2 frame registry non-mutation test for an extension frame type.
`tests/protocols/misc/test_pcapng_unit.py`	Adds PCAP-NG registries non-mutation tests for block/option/record/secrets.
`docs/source/pcapkit/protocols/protocol.rst`	Documents the new `_lookup_registry` helper in protocol API docs.

Review details

Files reviewed: 17/17 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…the lengths Copilot's finding on #428, and it is correct. The segment carried offset+flags `0x70`, i.e. a data offset of 7 words = 28 octets, while supplying 24. The bytes came from #425's reproduction and the miscount came with them; the correct offset for 20 octets of fixed header plus a 4-octet option is 6 words, so `0x60`. Checked rather than assumed that the corrected segment still reproduces the leak, since a malformed offset driving the parser somewhere it would not otherwise go would have meant the test was passing for the wrong reason and needed rebuilding rather than patching. It does not: against a pristine `origin/main`, both offsets give `hdr_len` as declared, the same option list `[156, 0]`, the same `UnassignedOption(kind=156, length=2, data=b'')`, `leaked=['156']` and `register_tcp_option` warning `'option 156 already registered, overwriting'`. On this branch both give `leaked=[]` and a silent registration. The offset was incidental to the defect, so the correction is a patch. Audited the other seven new tests for the same class of error -- every field that has to agree with the octets supplied, checked by arithmetic on the bytes rather than by re-reading the constant. All 44 checks agree: SCTP's three chunk lengths and their nested parameter and cause lengths, the IPv6 payload lengths and `(hdr ext len + 1) * 8` for all three extension headers plus their TLVs, MH's `(header len + 1) * 8` and 8-octet alignment for both messages and the BRR's option, and PCAP-NG's six blocks head and tail, their option and record value padding and 32-bit alignment. PCAP-NG could not have drifted anyway -- its block lengths are computed by the test's own builder, not typed in. So that a reviewer is not the mechanism that catches this next time, the constraint is now asserted where the packet is built: `hdr_len == len(packet)` for TCP, the chunk length against the octets supplied for SCTP, and `(header len + 1) * 8` against them for MH. One that is *not* a miscount, now that the comment says so: the HTTP/2 frame declares 13 for a 13-octet frame because this library counts the whole frame where :rfc:`9113#section-4.1` counts the payload alone. `make` writes `payload + 9` (httpv2.py:292), the readers recover it as `length - 9` (httpv2.py:658,668), and `read` rejects anything under 9, so the wire-accurate 4 raises `ProtocolError`. Declaring 13 is what reaches the registry lookup. That mismatch is a separate defect, filed rather than folded in, and the test now says which of the two conventions it is following and why. Verification re-run, not carried over: byte comparison 30/30 byte-identical against a `git archive` of `origin/main`; suite 865 passed, 17 skipped, 848 subtests, 0 failed; all eight new tests still fail on a pristine tree, the TCP one with `Items in the first set but not the second: <Option.Reserved_156: 156>`. Tests only -- no change under `pcapkit/`.

Copilot

🟢 Approval recommended

The change is narrowly scoped, consistently applied across all identified read sites, and is backed by focused regression tests covering each affected registry family.

Review details

Files reviewed: 17/17 changed files
Comments generated: 0 new
Review effort level: Lite

Brings in #427 (the option-parse shortcut), #428 (dispatch no longer writing to shared registries) and #430 (the generated typed `__init__`). One conflict, in `tests/protocols/transport/test_tcp_udp_unit.py`, where both sides appended test methods to the end of `TCPUDPUnitTests`. Both kept. `pcapkit/corekit/fields/collections.py` auto-merged, which is the file worth checking by hand rather than trusting: #427 rewrote the head of the `OptionField.unpack` loop and this branch rewrote its tail, so the two touch the same function without overlapping. The merged loop reads the type field through #427's shortcut, rewinds by its `consumed`, then subtracts `len(data)`, breaks on the end-of-option-list code, and only then checks that the option moved the stream. `git diff origin/main -- pcapkit/corekit/fields/collections.py` is additions only -- none of #427's work is lost, and the guard is still after the `eool` break, which is where #431's last commit had to move it.

) (#560) Reading X.registry[code] for a code nobody registered inserted the default schema under that code, permanently: Option.registry is a class-level defaultdict, so one bare subscript -- Option.registry[OptionNumber(156)] -- made an unassigned TCP option number read back as registered for the rest of the process. Identical to the __proto__ dispatch defect #421/#425/#428 fixed one layer down, at the schema layer instead of the protocol layer. The fallback is deliberate -- it is how an unknown option, chunk or block falls back to its Unknown*/Unassigned* schema -- so only the retention is removed, not the fallback. - Add _EnumRegistry, a defaultdict subclass whose __missing__ returns the default without writing it. - EnumSchema.__init_subclass__ builds __enum__ as one for the common auto-created case, and swaps a manually-declared plain defaultdict (as PCAPNG.Option's outer namespaced mapping and TCP.MPTCP use) for the same safe type the moment the class body finishes -- before any external code can hold a reference to the original object, so .registry keeps returning the identical object on every later read. - Document the new class in docs/source/pcapkit/protocols/index.rst, alongside SchemaMeta/EnumMeta, since EnumMeta.registry and EnumSchema.registry's docstrings now cross-reference it. Does not cover PCAPNG.Option's per-namespace inner dicts, which are nested values rather than the __enum__ attribute itself and remain a plain, retention-unsafe defaultdict; fixing those belongs to pcapng.py. Adds tests/protocols/schema/test_enum_schema_registry_unit.py, each proven to fail without the fix. Closes #555.

@Ts-Boom

…port_module (#574) (#586) Proposed by @Ts-Boom in #563. A next layer code nobody registered resolves to the fallback ModuleDescriptor the registry's default factory produces, and _lookup_next_layer deliberately does not write that back -- recording a miss in a class-level defaultdict is the defect #425/#428 fixed at this layer and #560 fixed at the schema layer. So every unrecognised frame resolved the same descriptor again: 48 of the 52 ModuleDescriptor.klass resolutions an extraction of many_interfaces.pcapng performs, each re-entering importlib.import_module for a module sys.modules already held. - ModuleDescriptor.klass now reads sys.modules first, and enters import_module only when the module is not loaded yet -- or when the loaded module does not have the attribute, which is a body still executing (a circular import, or another thread part way through importing it) and is what import_module's per-module lock exists to wait for. Measured on CPython 3.14.7: klass 436 -> 117 ns, the whole miss path 883 -> 526 ns, and import_module calls during extract() 48 -> 0 on many_interfaces.pcapng and 4 -> 0 on ipv4.pcap. The hit path is untouched (124 -> 127 ns, inside noise), since it is already memoised by the registry write-back. - Nothing memoises the resolved class, which is the deliberate part. The class is re-read with getattr on every access, so sys.modules stays the only module cache in play and its invalidation is the interpreter's: importlib.reload rebinds the class inside the same module object, and sys.modules.pop() replaces the object outright. #563's class-level _MODULE_CACHE followed neither, and an instance built from the class it kept fails isinstance against the live one. - Records in _lookup_next_layer's docstring why no memo lives there, so the next reader does not add one. Honest about the scale: this is not measurable in extract() wall clock. 48 avoided calls is ~17 us against a ~37 ms extraction, two orders of magnitude inside this host's single-digit-millisecond run-to-run variance, and repeated A/B pairs flipped sign. #563's "~40% of cumulative time" does not reproduce: _import_next_layer's self time is 0.58% against its 90.2% cumulative, because it is a recursive-descent dispatcher that has the whole nested parse beneath it. aenum.extend_enum at 16.7% self time is where the real time is (#575). Also found, not fixed here: the function-level `from ... import NoPayload` statements on the protocol layer, one of which is _import_next_layer's length == 0 fast path, run 890 times on http.pcap at ~162 ns against ~58 ns for the sys.modules equivalent -- ~92 us on a ~540 ms extraction, so left alone rather than paid for with a second resolution path. Adds tests/protocols/test_dispatch_default_resolution_unit.py, and four cases to tests/corekit/test_module.py. The two that assert the saving fail on the pre-fix code (5 import_module calls for 5 lookups); the three guards fail against the designs this one rejects -- the reload guard against #563's cache, and all three against writing the resolved fallback back under the missed code. Closes #574.

@Ts-Boom

…port_module (#574) (#586) Proposed by @Ts-Boom in #563. A next layer code nobody registered resolves to the fallback ModuleDescriptor the registry's default factory produces, and _lookup_next_layer deliberately does not write that back -- recording a miss in a class-level defaultdict is the defect #425/#428 fixed at this layer and #560 fixed at the schema layer. So every unrecognised frame resolved the same descriptor again: 48 of the 52 ModuleDescriptor.klass resolutions an extraction of many_interfaces.pcapng performs, each re-entering importlib.import_module for a module sys.modules already held. - ModuleDescriptor.klass now reads sys.modules first, and enters import_module only when the module is not loaded yet -- or when the loaded module does not have the attribute, which is a body still executing (a circular import, or another thread part way through importing it) and is what import_module's per-module lock exists to wait for. Measured on CPython 3.14.7: klass 436 -> 117 ns, the whole miss path 883 -> 526 ns, and import_module calls during extract() 48 -> 0 on many_interfaces.pcapng and 4 -> 0 on ipv4.pcap. The hit path is untouched (124 -> 127 ns, inside noise), since it is already memoised by the registry write-back. - Nothing memoises the resolved class, which is the deliberate part. The class is re-read with getattr on every access, so sys.modules stays the only module cache in play and its invalidation is the interpreter's: importlib.reload rebinds the class inside the same module object, and sys.modules.pop() replaces the object outright. #563's class-level _MODULE_CACHE followed neither, and an instance built from the class it kept fails isinstance against the live one. - Records in _lookup_next_layer's docstring why no memo lives there, so the next reader does not add one. Honest about the scale: this is not measurable in extract() wall clock. 48 avoided calls is ~17 us against a ~37 ms extraction, two orders of magnitude inside this host's single-digit-millisecond run-to-run variance, and repeated A/B pairs flipped sign. #563's "~40% of cumulative time" does not reproduce: _import_next_layer's self time is 0.58% against its 90.2% cumulative, because it is a recursive-descent dispatcher that has the whole nested parse beneath it. aenum.extend_enum at 16.7% self time is where the real time is (#575). Also found, not fixed here: the function-level `from ... import NoPayload` statements on the protocol layer, one of which is _import_next_layer's length == 0 fast path, run 890 times on http.pcap at ~162 ns against ~58 ns for the sys.modules equivalent -- ~92 us on a ~540 ms extraction, so left alone rather than paid for with a second resolution path. Adds tests/protocols/test_dispatch_default_resolution_unit.py, and four cases to tests/corekit/test_module.py. The two that assert the saving fail on the pre-fix code (5 import_module calls for 5 lookups); the three guards fail against the designs this one rejects -- the reload guard against #563's cache, and all three against writing the resolved fallback back under the missed code. Closes #574.

@Ts-Boom

…port_module (#574) (#586) Proposed by @Ts-Boom in #563. A next layer code nobody registered resolves to the fallback ModuleDescriptor the registry's default factory produces, and _lookup_next_layer deliberately does not write that back -- recording a miss in a class-level defaultdict is the defect #425/#428 fixed at this layer and #560 fixed at the schema layer. So every unrecognised frame resolved the same descriptor again: 48 of the 52 ModuleDescriptor.klass resolutions an extraction of many_interfaces.pcapng performs, each re-entering importlib.import_module for a module sys.modules already held. - ModuleDescriptor.klass now reads sys.modules first, and enters import_module only when the module is not loaded yet -- or when the loaded module does not have the attribute, which is a body still executing (a circular import, or another thread part way through importing it) and is what import_module's per-module lock exists to wait for. Measured on CPython 3.14.7: klass 436 -> 117 ns, the whole miss path 883 -> 526 ns, and import_module calls during extract() 48 -> 0 on many_interfaces.pcapng and 4 -> 0 on ipv4.pcap. The hit path is untouched (124 -> 127 ns, inside noise), since it is already memoised by the registry write-back. - Nothing memoises the resolved class, which is the deliberate part. The class is re-read with getattr on every access, so sys.modules stays the only module cache in play and its invalidation is the interpreter's: importlib.reload rebinds the class inside the same module object, and sys.modules.pop() replaces the object outright. #563's class-level _MODULE_CACHE followed neither, and an instance built from the class it kept fails isinstance against the live one. - Records in _lookup_next_layer's docstring why no memo lives there, so the next reader does not add one. Honest about the scale: this is not measurable in extract() wall clock. 48 avoided calls is ~17 us against a ~37 ms extraction, two orders of magnitude inside this host's single-digit-millisecond run-to-run variance, and repeated A/B pairs flipped sign. #563's "~40% of cumulative time" does not reproduce: _import_next_layer's self time is 0.58% against its 90.2% cumulative, because it is a recursive-descent dispatcher that has the whole nested parse beneath it. aenum.extend_enum at 16.7% self time is where the real time is (#575). Also found, not fixed here: the function-level `from ... import NoPayload` statements on the protocol layer, one of which is _import_next_layer's length == 0 fast path, run 890 times on http.pcap at ~162 ns against ~58 ns for the sys.modules equivalent -- ~92 us on a ~540 ms extraction, so left alone rather than paid for with a second resolution path. Adds tests/protocols/test_dispatch_default_resolution_unit.py, and four cases to tests/corekit/test_module.py. The two that assert the saving fail on the pre-fix code (5 import_module calls for 5 lookups); the three guards fail against the designs this one rejects -- the reload guard against #563's cache, and all three against writing the resolved fallback back under the missed code. Closes #574.

…issue (#719) - Nine citations in tests/ called a pull request "GitHub issue" (#921, #764, #906, #815, #936, #721, #501, #983) or lumped PR #428 in with issue #425; each now names the right kind. - test_dispatch_default_resolution_unit: issue #425 reported the registry leak and PR #428 fixed it, so the sentence says "reported" and "fixed" instead of crediting the issue with the fix. - Prose only: docstrings and comments, no assertion or logic touched.

… count (#719) - tests/protocols/test_dispatch_default_resolution_unit.py: this layer's reporter is #421 and its fix #426 (next-layer __proto__); #428 extended it to the option/chunk/block registries and #560 is the schema layer. The paragraph is re-wrapped to ~80 columns. - pcapkit/protocols/protocol.py (_lookup_next_layer): the same note cited #425 as this layer; it now cites #421 here, #425 for the sibling option, chunk and block registries, and #555 for the schema layer. - pcapkit/corekit/sentinels.py: the pointer to that note tracks it. - docs/source/contributing/pep.rst: ten protocols parse a checksum field, not eight -- IPX and MH were missing; "the other seven" is now nine. Prose only. Docs build warning set identical to origin/main.

…t layer * Three separate registry-leak defects were fixed at three layers -- :issue:`421`/:pr:`426` for next-layer dispatch, :issue:`425`/:pr:`428` for the option, chunk and block registries, :issue:`555`/:pr:`560` at the schema layer. Four sites credited the wrong pair. * `1.5.0.rst:540` named :issue:`425`/:pr:`428` as the fix "at this layer" in a passage about next-layer dispatch; it now names all three. * `schema/schema.py` twice called #421 and #425 jointly "the protocol-layer ``__proto__`` family"; #425 is the option, chunk and block family. Two test docstrings carried the same conflation, and one called #421/#425/#428 the schema-layer form, which is #555. * `protocol.py`'s "are all that same defect" overstated its clause: the three fixed registries retaining a lookup *miss*, where the sentence is about a memoised *resolved class* going stale under reload. * `corekit/sentinels.py` cited that note, by those three issue numbers, as evidence that reload staleness is tracked. Narrowing the note would have made the two contradict, so the pointer now names what the note argues. * Regenerated `CHANGELOG.md`. tests/project/ plus both edited test files: 284 passed, 864 subtests.

…t layer * Three separate registry-leak defects were fixed at three layers -- :issue:`421`/:pr:`426` for next-layer dispatch, :issue:`425`/:pr:`428` for the option, chunk and block registries, :issue:`555`/:pr:`560` at the schema layer. Four sites credited the wrong pair. * `1.5.0.rst:540` named :issue:`425`/:pr:`428` as the fix "at this layer" in a passage about next-layer dispatch; it now names all three. * `schema/schema.py` twice called #421 and #425 jointly "the protocol-layer ``__proto__`` family"; #425 is the option, chunk and block family. Two test docstrings carried the same conflation, and one called #421/#425/#428 the schema-layer form, which is #555. * `protocol.py`'s "are all that same defect" overstated its clause: the three fixed registries retaining a lookup *miss*, where the sentence is about a memoised *resolved class* going stale under reload. * `corekit/sentinels.py` cited that note, by those three issue numbers, as evidence that reload staleness is tracked. Narrowing the note would have made the two contradict, so the pointer now names what the note argues. * Regenerated `CHANGELOG.md`. tests/project/ 268 passed / 864 subtests; tests/protocols/schema/ test_enum_schema_registry_unit.py and tests/protocols/ test_dispatch_default_resolution_unit.py 16 passed; tests/protocols/misc/test_pcapng_unit.py 93 passed / 1773 subtests.

JarryShaw requested a lite review from Copilot September 17, 2026 03:26

Copilot started reviewing on behalf of JarryShaw September 17, 2026 03:27 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread tests/protocols/transport/test_tcp_udp_unit.py Outdated

JarryShaw mentioned this pull request Sep 17, 2026

Migrate IPv4 and HIP option dispatch from setattr/getattr to the registry pattern #429

Closed

JarryShaw mentioned this pull request Sep 17, 2026

Option, chunk and block registries leak on lookup miss, the same way __proto__ did #425

Closed

JarryShaw requested a lite review from Copilot September 17, 2026 12:48

Copilot started reviewing on behalf of JarryShaw September 17, 2026 12:49 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

JarryShaw added 2 commits September 17, 2026 09:19

Merge branch 'main' into fix/option-registry-leak

cfd0bba

Merge branch 'main' into fix/option-registry-leak

2c1c4ab

JarryShaw merged commit f137a5b into main Sep 17, 2026
24 checks passed

JarryShaw deleted the fix/option-registry-leak branch September 18, 2026 20:22

This was referenced Sep 20, 2026

EnumSchema registries retain every looked-up code, so a lookup miss leaks and contaminates later parses #555

Closed

fix(schema): stop EnumSchema.registry from retaining a lookup miss (#555) #560

Merged

JarryShaw added the fix Pull requests that fix a defect (fix: subject prefix) label Sep 22, 2026

This was referenced Sep 23, 2026

Generalise #681's registry overwrite guard: the schema half of every registrar is silent, and the seven code-keyed ones do not say what they displaced #692

Closed

fix(corekit): make EnumRegistry.get's no-default marker an identity sentinel (#857) #859

Merged

JarryShaw mentioned this pull request Oct 3, 2026

docs(tests): stop calling pull requests issues in test prose #1006

Merged

5 tasks done

JarryShaw mentioned this pull request Oct 3, 2026

docs: sweep all prose for concision and accuracy, and drop timed context outside the changelog #719

Open

This was referenced Oct 3, 2026

docs(pep,tests): correct the checksum protocol count and the registry-leak lineage #1007

Merged

docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

Merged

JarryShaw mentioned this pull request Oct 3, 2026

docs(changelog,protocols): credit the registry-leak fixes to the right layer #1009

Merged

4 of 12 tasks

JarryShaw added this to the 1.5 milestone Oct 6, 2026

Navigation Menu

Uh oh!

protocols: stop option, chunk and block dispatch writing to shared registries - #428

protocols: stop option, chunk and block dispatch writing to shared registries#428
JarryShaw merged 4 commits into
mainfrom
fix/option-registry-leak

JarryShaw commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JarryShaw commented Sep 17, 2026

The issue named eight registries; there are sixteen

Before → after

What was generalised

Verification

Three things found and deliberately not fixed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants