Skip to content

marshal: refuse a back-reference to an object still being written - #8519

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:marshal-recursion-guard
Aug 14, 2026
Merged

marshal: refuse a back-reference to an object still being written#8519
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:marshal-recursion-guard

Conversation

@youknowone

@youknowone youknowone commented Aug 14, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

w_ref handed out a back-reference as soon as an object was registered, so a
container that reached itself while still being written serialized a r
back-reference to an entry the reader had not finished building.

CPython guards this in Python/marshal.c: w_ref marks a code or slice entry
incomplete when it reserves the index and w_complete clears the mark once the
body is written; a back-reference taken in between raises
ValueError: cannot marshal recursion %T objects (gh-148653).

This mirrors that structure:

  • WriterRefEntry carries an incomplete flag.
  • reserve sets it for the kinds whose immutable representation cannot be
    rebuilt from a back-reference — code and slice.
  • try_ref returns Err(()) while the flag is set, which
    write_object_depth turns into the ValueError.
  • complete clears it after the body is written.

Three TODO: RUSTPYTHON markers in Lib/test/test_marshal.py are removed:

test before after
test_unmarshallable skip (panic) pass
test_reference_loop_code skip (panic) pass
test_reference_loop_slice expectedFailure pass

test_marshal runs 75 tests / 16 skipped (was 18 skipped).

Summary by CodeRabbit

  • Bug Fixes
    • Improved serialization reference tracking for code and slice objects.
    • Prevented references to objects that are still being serialized, avoiding incomplete or invalid output.
    • Added clearer errors when serialization exceeds supported recursion depth.

@youknowone
marshal: refuse a back-reference to an object still being written
fef60e9 
`w_ref` marks a code or slice entry incomplete until `w_complete`, because
the reader rebuilds both from their fields and a `TYPE_REF` issued while
those fields are still on the wire names an object that does not exist
yet.  `WriterRefTable` carries that marker and `write_object_depth`
raises `cannot marshal recursion <type> objects` instead of emitting the
reference.

`test_reference_loop_code`, `test_unmarshallable` and
`test_reference_loop_slice` lose their RustPython markers; `test_marshal`
is 75 run, 16 skipped.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28e1a06e-86fb-4170-aa29-aa36228eb902

📥 Commits

Reviewing files that changed from the base of the PR and between fd7d107 and fef60e9.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_marshal.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/vm/src/stdlib/marshal.rs
📝 Walkthrough

Walkthrough

Marshal serialization now tracks whether referenced objects are incomplete. Recursive references return value errors. Code and slice objects become referenceable after their contents are serialized.

Changes

Marshal reference tracking

Layer / File(s) Summary
Reference table state
crates/vm/src/stdlib/marshal.rs		 Reference entries now store an index and incomplete status. Lookup rejects incomplete references, and completion updates entries for later back-references.
Recursive object serialization
crates/vm/src/stdlib/marshal.rs		 Serialization reports recursion-specific value errors. Code and slice objects are marked complete after their contents are written.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: shaharnaveh

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_marshal.py (TODO: 5)

dependencies:

dependent tests: (25 tests)

  • marshal: test_bool test_exceptions test_importlib test_inspect test_marshal test_zipimport
    • importlib._bootstrap_external: test_importlib test_unittest
      • modulefinder: test_importlib test_modulefinder
      • py_compile: test_argparse test_cmd_line_script test_compileall test_importlib test_multiprocessing_main_handling test_py_compile test_pydoc test_runpy
      • pydoc: test_enum
    • pkgutil: test_pkgutil test_pyrepl
    • profile: test_profile
    • pstats: test_pstats
    • zipimport: test_importlib test_zipimport_support

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone
youknowone marked this pull request as ready for review August 14, 2026 03:58
@youknowone
youknowone merged commit 2690c16 into RustPython:main Aug 14, 2026
28 of 29 checks passed
@youknowone
youknowone deleted the marshal-recursion-guard branch August 14, 2026 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone