fix(reassembly): make Deferred picklable under protocol 0 and 1 by JarryShaw · Pull Request #2424 · JarryShaw/PyPCAPKit

JarryShaw · 2026-10-29T01:07:21Z

Please follow the guide below

Searched for similar pull requests
Followed the coding style (mypy/pylint/isort clean on the touched file, using this project's own CI flags)
Tests pass, and a test case covers the change
Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md

What is the purpose of your pull request?

fix — corrects a defect

Description of your pull request and other information

Deferred (pcapkit/foundation/reassembly/data/data.py) declares __slots__
with no __getstate__/__setstate__, so pickle.dumps(d, protocol=0) and
protocol=1 raised TypeError for any instance. Protocols 2+ already
worked. Independent of #2417 (also on Deferred, different concern) and
#2421 (different, unrelated class).

Two cross-review rounds raised further defects on the same fix, now tracked
as #2425 and folded into this same PR:

the first revision's __getstate__ returned a flat {slot: value} dict —
a different shape from the (dict_or_None, slots) pair every released
build already used. Now mirrors the native shape; __setstate__ still
accepts the flat shape too.
self.__slots__ resolves through the MRO like any other attribute, and a
hand-rolled MRO walk also mishandled a bare-string __slots__ = 'name'
and a private, name-mangled slot. Replaced with copyreg._slotnames, the
standard library's own helper for exactly this.
the test file's own comparison helper used object.__getstate__, 3.11+
only; now reads the same state via __reduce_ex__(2)[2], and the file's
tier is ALL rather than LATEST, matching house convention for
pickling/copying behaviour.

tests/foundation/reassembly/test_deferred_pickle_2423_unit.py covers
protocol 0/1, the native state shape, backward compatibility with both
pickle shapes, MRO-aware subclassing (unset slots, __weakref__, a bare
__dict__), and real pickle round trips for bare-string and name-mangled
subclass slots. Confirmed every new/changed test fails against each prior
revision by stashing just that code change and re-running.

Closes #2423
Closes #2425

… entry

JarryShaw · 2026-10-29T01:18:01Z

Review: GOOD TO GO at 6cf10fa19 (Sonnet). Reproduced the original TypeError on protocols 0/1 directly on main, confirmed the fix round-trips protocols 0–5 with value equality (including a real-shaped Deferred), confirmed corekit/enum.py/const/reg/apptype/* untouched (separate #2421 concern), fail-without-fix reproduced, mypy/pylint/isort clean, changelog check passes. Tests: 149 passed (base is 145; delta is exactly the 4 new tests).

Two non-blocking findings worth tracking, neither reachable in the repo today (no stored Deferred pickles, no subclasses exist):

The fix changes the pickle format for protocols 2+ as well as fixing 0/1 — a pickle written by the current released code won't load under the fix, and vice versa. Only matters if something has ever persisted a Deferred across a version boundary.
self.__slots__ isn't MRO-aware, so a hypothetical subclass with its own slots or a __dict__ would silently lose state (it didn't before this fix).

Filing a lightweight follow-up for both; not blocking this PR.

JarryShaw · 2026-10-29T01:56:30Z

Review: NEEDS CHANGES at 43f7e2b19 (Sonnet, round 2). Mechanism and compat claims hold up — pickles from v1.5.0b1, b14 and pre-fix main load under this PR and are byte-identical going the other way; the MRO/shadowing claim is real (confirmed a subclass's own __slots__ does shadow the base's). Two real defects, both verified fixable across Python 3.8–3.14:

The two new native-shape tests use 3.11+-only object.__getstate__, so they error (not fail — AttributeError) on 3.8/3.9/3.10. tests/_tiers.py marks pickling/copying ALL-tier, so CI's latest-only run hides this. Fix: build the stand-in shape via stand_in.__reduce_ex__(2)[2] instead, which works on all seven interpreters tested.
__getstate__ drops a subclass's slot when declared as a bare string (__slots__ = 'extra', not a tuple) or as a private/mangled name — 2 of 12 subclass shapes tested lose that slot where pre-fix main kept it on protocols 2–5. Fix: collect via copyreg._slotnames(type(self)) instead of hand-rolling the MRO walk — matches all 12 shapes.

Full review in the task transcript if useful. Re-requesting changes on the current head.

JarryShaw · 2026-10-29T01:56:53Z

Review: NEEDS CHANGES at 43f7e2b19 (Sonnet). One real regression in the MRO-awareness fix for #2425.

Deferred.__getstate__'s hand-rolled MRO walk silently drops state for a subclass declaring a bare-string slot (__slots__ = 'extra', valid Python — a single string isn't iterated character-by-character here) or a private name-mangled slot (__slots__ = ('__secret',)). Reproduced on CPython 3.8–3.15: the pre-fix class (protocols 2/5) correctly restored both; this PR's head raises AttributeError for both. This contradicts the PR's own "matches Python's own state" claim — it's a regression against released behavior, not just an unfixed gap. Fix: replace the hand-rolled loop with copyreg._slotnames(type(self)), which handles both spellings correctly (verified in a scratch tree — all 13 existing PR tests still pass, protocols 2-5 bytes unchanged). Add a real-pickle test for each slot spelling.

Everything else verified independently and holds: single commit on current main; pickle format backward-compat confirmed both directions on 3.8-3.15 (protocol 2-5 bytes byte-identical to both the pre-fix class and a real v1.5.0b14 instance); base case and copy/deepcopy unaffected; docstring overclaim fixed; scope limited to the 4 intended files; tests/foundation/reassembly 158 passed/854 subtests; mypy/pylint/isort clean; changelog and changelog_md.py --check pass; CI green on this head (49 succeeded, 0 failed).

Deferred declares __slots__ with no __getstate__/__setstate__, so protocol 0 and 1 (which copyreg refuses outright once __getstate__ is still the inherited default) raised TypeError for any Deferred instance, regardless of its analyze/proto/payload values. Independent of #2417, a different concern on the same class. Two cross-review rounds before this merged raised further defects on the same fix (#2425), all folded in here: - __getstate__ mirrors the (dict_or_None, slots) pair Python's own default slotted-class reduction already produced for every released build, instead of a bespoke flat dict. A Deferred pickled by any released version still unpickles, and __setstate__ additionally still accepts the first revision's own flat shape. - slot names come from copyreg._slotnames(type(self)) rather than a hand-rolled walk of type(self).__mro__ and each class's __slots__, which got three cases wrong: a subclass's own __slots__ shadowing the base class's (self.__slots__ resolves through the MRO), a bare-string __slots__ = 'name' read character by character, and a private, name-mangled slot read back under its unmangled spelling. - the pickle test file's own native-shape comparison helper used object.__getstate__, which is 3.11+ only; it now reads the identical state through __reduce_ex__(2)[2], available on every interpreter this project supports, and the file's tier is 'ALL' rather than 'LATEST', matching the house convention for pickling/copying behaviour. Covered by tests/foundation/reassembly/test_deferred_pickle_2423_unit.py: protocol 0/1, the native state shape, backward compatibility with both pickle shapes, MRO-aware subclassing (unset slots, a __weakref__ slot, a bare __dict__), and real pickle round trips for a bare-string and a name-mangled subclass slot. Confirmed every new/changed test fails against each prior revision by stashing just the relevant code change and re-running. Build: brazil-build not applicable (GitHub project); ran coverage run -m pytest, mypy and pylint with this project's own CI flags, and isort -- all clean on the touched files. Closes #2423 Closes #2425

JarryShaw · 2026-10-29T02:23:27Z

Review: GOOD TO GO at 03f828e6f. Verified directly (not delegated — three prior delegation attempts for this exact round immediately spawned unauthorized sub-agents regardless of brief wording; see the session's memory for the root cause, since resolved).

Diff vs the prior reviewed head (43f7e2b19) is scoped to exactly the two claimed fixes, 4 files (data.py, its test, the changelog and generated CHANGELOG.md).

MRO/slot-shadowing fix: copyreg._slotnames(type(self)) replaces the hand-rolled walk. Reproduced both named failure cases directly — a bare-string slot (__slots__ = 'extra') and a private/mangled slot (__slots__ = ('__secret',)) — both now round-trip correctly at every pickle protocol 0-5.
3.8-3.10 compat fix: confirmed object.__getstate__ no longer appears anywhere in the test file; the __reduce_ex__(2)[2]-based comparison helper is in place.
Backward-compat: built a pickle using the pre-fix class's old (dict, {mro-walked slots}) shape, confirmed it loads correctly under this PR's copyreg-based class.
Full tests/foundation/reassembly/ suite: 161 passed, 866 subtests. mypy/pylint/isort clean on touched files (mypy's 83 reported errors are the same pre-existing set in unrelated files, confirmed identical against a baseline run on main). Changelog entry cites fix(reassembly): Deferred cannot be pickled under protocol 0 or 1 #2423, fix(reassembly): Deferred pickle format/subclass gaps left by #2424 #2425, fix(reassembly): make Deferred picklable under protocol 0 and 1 #2424; changelog_md.py --check passes.

No scope creep, no regressions found.

JarryShaw · 2026-10-29T02:24:44Z

Review: GOOD TO GO at 03f828e6f. Verified directly (main thread, after three consecutive dispatch attempts for this round immediately spawned unauthorized sub-agents — a fork left over from an earlier task, not a brief-wording problem; traced and documented).

Both round-3 fixes confirmed:

copyreg._slotnames(type(self)) correctly preserves a bare-string slot (__slots__ = 'extra') and a private/mangled slot (__slots__ = ('__secret',)) across all pickle protocols 0-5 — reproduced both failing cases from the prior round and confirmed the fix resolves both.
stand_in.__reduce_ex__(2)[2] is a byte-identical substitute for object.__getstate__() where the latter exists (3.11+), and confirmed object.__getstate__ genuinely doesn't exist on Python 3.8 (AttributeError, reproduced directly) — the fix is correctly portable.

Full tests/foundation/reassembly/ suite: 161 passed, 866 subtests. mypy clean, pylint 10.00/10, isort clean, changelog_md.py --check passes. Diff vs the prior head is scoped to exactly the two fixes (4 files: data.py, the test file, CHANGELOG.md, 1.5.0.rst).

Round 10, the closing-condition test: a full re-grep across the entire repo found exactly 3 items, all outside the carve-outs round 9 already settled. * pcapkit/protocols/protocol.py + foundation/registry/protocols.py: RegistryError said "protocol must be a Protocol subclass" while the adjacent issubclass() check and the Raises: docstring both say ProtocolBase -- fixed the message text to match, with the three test assertions that locked the old string. * tests/foundation/reassembly/test_deferred_pickle_2423_unit.py: a docstring said "this pull request's earlier commit", stale now that #2424 landed as one squashed commit on main -- reworded to cite the issue instead of the PR's own history. * tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py: a cross-reference cited the nonexistent module pcapkit.corekit.fields.enums.EnumField; the real path is pcapkit.corekit.fields.numbers.EnumField. No executable logic changed: an AST structural diff (ignoring string constants) against origin/main is identical for every touched file, and the RegistryError fix only changes the text of a raised message, not what triggers it. Re-verified and rejected two relayed findings that didn't survive a direct check: a transport.py:188-189 and a registry/protocols.py:370 cross-reference claim both already read correctly on this base. Re-swept vendor/process() for siblings of #2429/#2430 -- none found. docs/source and the rest of tests/ came back clean. Build: tests/protocols/test_register_class_guard_unit.py, tests/foundation/registry/test_protocols.py, tests/foundation/reassembly/test_deferred_pickle_2423_unit.py, tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py and tests/project/test_docs_issue_pr_roles_1103_unit.py pass. Closes #1103.

…pickling copyreg._reduce_ex, which object.__reduce_ex__ hands protocol 0/1 to, walks cls.__mro__ for a base to compute state from, before __getstate__ is ever consulted. On CPython that walk passes through Generic and lands on object (state=None); on GraalPy 25.4 it stops at Generic itself and calls Generic(self), raising "TypeError: typing.Generic() takes no arguments" -- independent of #2424/#2425's __getstate__/__setstate__ fix, since those only supply the dict half of a reduction the walk fails to even build. - Deferred.__reduce_ex__ now hard-codes base=object, state=None for protocol 0/1 -- the only outcome the walk ever reached for this class -- and builds the reduction tuple directly via copyreg._reconstructor, with __getstate__()'s own (dict_or_None, slots) pair restored through __setstate__ exactly as before. Protocol >= 2 defers to object.__reduce_ex__ unchanged. - Dropping Generic[_KT] instead was considered and rejected: test_deferred_proto_type_2417_unit.py reads Deferred.__parameters__ at runtime, which only exists because Generic is a real base, not a TYPE_CHECKING-only annotation. No longer invokes copyreg._reduce_ex's MRO walk for protocol<2 at all, so Generic is no longer part of the route pickling takes -- expected to resolve the GraalPy failure the same way on that interpreter, though it cannot be run here to confirm directly. Build/test: mypy 98 errors/32 files (baseline, unchanged), pylint 10.00/10, isort clean; tests/foundation/reassembly (173 tests/882 subtests) and tests/corekit (866 tests/7649 subtests) pass; 100% coverage on the changed file. Closes #2433

…pickling copyreg._reduce_ex, which object.__reduce_ex__ hands protocol 0/1 to, walks cls.__mro__ for a base to compute state from, before __getstate__ is ever consulted. On CPython that walk passes through Generic (a heap type whose __new__ is just the inherited, differently-bound object.__new__) and stops at object (state=None); on GraalPy 25.4 Generic is itself a non-heap (built-in) type, so the walk's first check breaks there directly and calls Generic(self), raising "TypeError: typing.Generic() takes no arguments" -- independent of #2424/#2425's __getstate__/__setstate__ fix, since those only supply the dict half of a reduction the walk fails to even build. - Deferred.__reduce_ex__ now hard-codes base=object, state=None for protocol 0/1 -- the only outcome the walk ever reached for this class -- and builds the reduction tuple directly via copyreg._reconstructor, with __getstate__()'s own (dict_or_None, slots) pair restored through __setstate__ exactly as before. Protocol >= 2 defers to object.__reduce_ex__ unchanged. - It also checks for, and defers to, a subclass's own __reduce__ override before building that hard-coded tuple, matching what object.__reduce_ex__ itself already does for every protocol. Without the check, a Deferred subclass overriding __reduce__ had it silently dropped at protocol 0/1 while protocol 2+ (via object.__reduce_ex__ unchanged) kept honouring it -- a regression caught in cross-review, now covered by a new test alongside the GraalPy-failure regression test. - Dropping Generic[_KT] instead was considered and rejected: test_deferred_proto_type_2417_unit.py reads Deferred.__parameters__ at runtime, which only exists because Generic is a real base, not a TYPE_CHECKING-only annotation. No longer invokes copyreg._reduce_ex's MRO walk for protocol<2 at all, so Generic is no longer part of the route pickling takes. Confirmed on real GraalPy 25.4.4 by cross-review: test_deferred_pickle_2423_unit.py goes from 8 failed on main to 16 passed/18 subtests on this fix, and the full tests/foundation/reassembly/ suite passes 173/854 subtests, 0 failed. Build/test (CPython): mypy 98 errors/32 files (baseline, unchanged), pylint 10.00/10, isort clean; tests/foundation/reassembly (176 tests/890 subtests, including the two new regression tests) and tests/corekit (693 tests/6767 subtests, unaffected by this change) pass; 100% coverage on the changed file. Closes #2433

…pickling (#2435) copyreg._reduce_ex, which object.__reduce_ex__ hands protocol 0/1 to, walks cls.__mro__ for a base to compute state from, before __getstate__ is ever consulted. On CPython that walk passes through Generic (a heap type whose __new__ is just the inherited, differently-bound object.__new__) and stops at object (state=None); on GraalPy 25.4 Generic is itself a non-heap (built-in) type, so the walk's first check breaks there directly and calls Generic(self), raising "TypeError: typing.Generic() takes no arguments" -- independent of #2424/#2425's __getstate__/__setstate__ fix, since those only supply the dict half of a reduction the walk fails to even build. - Deferred.__reduce_ex__ now hard-codes base=object, state=None for protocol 0/1 -- the only outcome the walk ever reached for this class -- and builds the reduction tuple directly via copyreg._reconstructor, with __getstate__()'s own (dict_or_None, slots) pair restored through __setstate__ exactly as before. Protocol >= 2 defers to object.__reduce_ex__ unchanged. - It also checks for, and defers to, a subclass's own __reduce__ override before building that hard-coded tuple, matching what object.__reduce_ex__ itself already does for every protocol. Without the check, a Deferred subclass overriding __reduce__ had it silently dropped at protocol 0/1 while protocol 2+ (via object.__reduce_ex__ unchanged) kept honouring it -- a regression caught in cross-review, now covered by a new test alongside the GraalPy-failure regression test. - Dropping Generic[_KT] instead was considered and rejected: test_deferred_proto_type_2417_unit.py reads Deferred.__parameters__ at runtime, which only exists because Generic is a real base, not a TYPE_CHECKING-only annotation. No longer invokes copyreg._reduce_ex's MRO walk for protocol<2 at all, so Generic is no longer part of the route pickling takes. Confirmed on real GraalPy 25.4.4 by cross-review: test_deferred_pickle_2423_unit.py goes from 8 failed on main to 16 passed/18 subtests on this fix, and the full tests/foundation/reassembly/ suite passes 173/854 subtests, 0 failed. Build/test (CPython): mypy 98 errors/32 files (baseline, unchanged), pylint 10.00/10, isort clean; tests/foundation/reassembly (176 tests/890 subtests, including the two new regression tests) and tests/corekit (693 tests/6767 subtests, unaffected by this change) pass; 100% coverage on the changed file. Closes #2433

github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 29, 2026

JarryShaw added this to PyPCAPKit Oct 29, 2026

JarryShaw moved this to In review in PyPCAPKit Oct 29, 2026

JarryShaw added a commit that referenced this pull request Oct 29, 2026

docs(changelog): cite PR #2424 alongside #2423 in the Deferred pickle…

6cf10fa

… entry

JarryShaw mentioned this pull request Oct 29, 2026

fix(reassembly): Deferred pickle format/subclass gaps left by #2424 #2425

Closed

JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 29, 2026

JarryShaw force-pushed the fix-deferred-pickle-2423 branch from 6cf10fa to 43f7e2b Compare October 29, 2026 01:34

JarryShaw force-pushed the fix-deferred-pickle-2423 branch from 43f7e2b to 03f828e Compare October 29, 2026 02:04

JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 29, 2026

JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 29, 2026

JarryShaw merged commit a3b6d31 into main Oct 29, 2026
65 checks passed

JarryShaw deleted the fix-deferred-pickle-2423 branch October 29, 2026 02:30

github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 29, 2026

JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 29, 2026

JarryShaw mentioned this pull request Oct 29, 2026

docs: #1103 round 10 -- close the whole-repo prose sweep #2432

Merged

5 tasks done

This was referenced Oct 29, 2026

fix(reassembly): Deferred(Generic[_KT]) still fails to pickle under protocol 0/1 on GraalPy #2433

Closed

fix(reassembly): bypass copyreg's MRO walk for Deferred's protocol<2 pickling #2435

Merged

Navigation Menu

Repository navigation

fix(reassembly): make Deferred picklable under protocol 0 and 1 - #2424

fix(reassembly): make Deferred picklable under protocol 0 and 1#2424
JarryShaw merged 1 commit into
mainfrom
fix-deferred-pickle-2423

JarryShaw commented Oct 29, 2026 •

edited

Loading

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

JarryShaw commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Please follow the guide below

What is the purpose of your pull request?

Description of your pull request and other information

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

JarryShaw commented Oct 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

JarryShaw commented Oct 29, 2026 •

edited

Loading