python / python/cpython

make marshal output not dependent on reference count

Open
#98,819 4 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

interpreter-core type-bug
Dominant language
Python
Stars
77.2k
Forks
36k
PR merge metrics
PR metrics pending

Description

Bug report

Currently the marshal module will emit a previously-unseen object flagged as a potential reference from later objects, unless the object has a reference count of 1. See https://github.com/python/cpython/blob/b27b57c6e44a276c8a9843fd37d4cf65b2827d5c/Python/marshal.c#L305

This is an overly-conservative heuristic -- it's easy to construct cases where an object has a reference count >1 but is not actually referenced by any other object about to be marshaled, so FLAG_REF is set when it does not need to be.

This makes marshal output unstable depending on accidents of reference counting behavior in the code calling marshal.dumps.

I ran into this because the Cinder JIT is able to reduce unnecessary increfs, and that resulted in some importlib tests failing on comparison of marshal output at https://github.com/python/cpython/blob/b27b57c6e44a276c8a9843fd37d4cf65b2827d5c/Lib/test/test_importlib/test_abc.py#L870-L871 because under Cinder JIT the reference count of code_object in that method is 1.

This previously caused issues in distutils reproducibility, resulting in a partial fix that applies only to interned strings: https://github.com/python/cpython/pull/8226

It would be better if marshal would actually determine which objects have multiple parents in the DAG and deterministically use FLAG_REF or not based on that.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Python/marshal.c at the referenced reference-count heuristic and inspect the affected assertions in Lib/test/test_importlib/test_abc.py at lines 870-871. Trace how marshal.dumps decides FLAG_REF, then establish tests showing output is independent of caller reference counts and confirm the importlib tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.