Skip to content

fix: GET of a never-put memo index is an error (#47) - #59

Merged
jensens merged 2 commits into
mainfrom
fix/memo-gap
Sep 30, 2026
Merged

jensens merged 2 commits into
mainfrom
fix/memo-gap

Conversation

@jensens

@jensens jensens commented Sep 30, 2026

Copy link
Copy Markdown
Member

Fixes #47

A GET/BINGET/LONG_BINGET of a memo index that was never put yielded None when a higher index had been put before: memo_put grows the memo vector with placeholders, and a gap below the highest index read as one. CPython raises Memo value not found at index N.

What changes

The memo is a Vec<Option<PickleValue>>: growth fills with None, every store writes Some, and memo_get maps a None to the memo index N not found error the out-of-range case already raised. The dirty-flush sites (a memoized container mutated after the put) write Some. Option<PickleValue> uses the enum's niche, so the element stays 56 bytes and the vector does not grow; the FileStorage decode and PG pipeline A/B against main are within the noise floor.

No pickler writes such a stream; this is corruption handling, and the pre-scan optimisation (puts of indices never read are skipped) is unaffected because the gap error is path-independent.

Evidence

  • 241 Rust and 403 Python tests; the tests cover the gap on the pre-scan path, on the fallback path (a trailing byte after STOP), in protocol 0 text opcodes, a dirty entry after a gap put, every Python entry point, and the memo shared across the two pickles of a ZODB record (a class pickle with its own puts makes index 0 valid in the state pickle).
  • The 1,692-record sample is byte-identical to main.
  • A fresh reviewer ran 80,000 random memo opcode streams against CPython: on every stream without MEMOIZE the branch now matches CPython's error decision exactly (832 streams that main decoded to a value are errors in both), and it reports the same index. The remaining divergence, MEMOIZE after a re-put of a filled slot, predates this branch (Decode regression since 1.6.0 (memo dirty-sync clones every container); pre-scan memo reads to skip unread puts #22) and is filed as a follow-up issue.

Assisted-by: Claude Fable 5.1

The memo is a `Vec<Option<PickleValue>>`: a put above the current length
grows it with `None`, every store writes `Some`, and `memo_get` maps a
`None` to the `memo index N not found` error the out-of-range case already
raised. The dirty-flush sites write `Some`. No new bookkeeping; the sample
database decodes identically and the FileStorage decode A/B against main is
within noise (0.97).

Assisted-by: Claude Fable 5.1
…ts (#47)

Fresh review: ready to merge (80,000 differential memo streams against
CPython match on every non-MEMOIZE case, sample identical, Option niche at
56 bytes). Minors folded in: `decode_zodb_record_for_pg` joins the
parametrized test, a trailing byte after STOP pins the gap on the
MemoNeeds::All path, and the size comments say 56 bytes.

Assisted-by: Claude Fable 5.1
@jensens
jensens merged commit f610255 into main Sep 30, 2026
5 checks passed
@jensens
jensens deleted the fix/memo-gap branch September 30, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decoder: GET of a never-put memo index below the highest stored one yields None instead of an error

1 participant