Skip to content

gh-159184: Make the marshal order of set elements with equal dumps stable - #159197

Merged
gpshead merged 4 commits into
python:mainfrom
gpshead:marshal-set-tie-order
Nov 1, 2026
Merged

gpshead merged 4 commits into
python:mainfrom
gpshead:marshal-set-tie-order

Conversation

@gpshead

@gpshead gpshead commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

This is the second half of gh-159184. The first half, #159196 (merged), stopped marshal from comparing set elements, which fixed the TypeError for complex NaNs. This PR fixes the order such elements are written in.

Set elements with the same dump are NaNs that are distinct objects. marshal leaves them in set iteration order, which is the order of their addresses. That is invisible unless one of them is also referenced elsewhere in the dump; then the reference lands at a different place among them from run to run:

>>> nans = [float('nan') for _ in range(3)]
>>> len({marshal.dumps([n, set(nans)]) for n in nans})
3                             # the same value every time; 1 with this PR

Change

flowchart TB
  subgraph before["main"]
    direction TB
    b1["list of (dump, index, element)"] --> b2["PyList_Sort()"]
    b2 --> b3{"run of equal dumps?"}
    b3 -- no --> b4["ordered by dump"]
    b3 -- yes --> b5["left in set order<br>= address order"]
  end
  subgraph after["this PR"]
    direction TB
    a1["array of {element, dump}"] --> a2["qsort() on the dump bytes"]
    a2 --> a3{"run of equal dumps?"}
    a3 -- no --> a4["ordered by dump"]
    a3 -- yes --> a5["w_same_set_items()"]
    a5 --> a6["already written in this dump:<br>first, in the order written"]
    a5 --> a7["floats and complex not written yet:<br>interchangeable; which one is which<br>is decided when one is referenced again"]
  end
Loading

Not covered

Elements that are containers with equal dumps, not written before the set (for example {(nan1,), (nan2,)} with one of the NaNs also used elsewhere), are still written in set order. A canonical order for that case amounts to graph canonisation. The compiler cannot produce it: every folded NaN is a new object.

Tests

ReferenceTestCase.test_nan_in_set_order checks exact bytes for sets of float and complex NaNs with references before, after and through other sets. test_nan_in_set also covers complex NaNs.

test_nan_constants_in_code compiles source whose constant folding produces NaNs (1e999 - 1e999) as a plain constant, in a tuple constant and in a frozenset constant (x in {...}, the only kind of set the compiler creates), and checks that the code can be marshalled and gives the same bytes in other processes. The compiler cannot make a NaN which is in a set and also referenced elsewhere, so the order fixed here is not observable from compiled code; the test documents that and would not be part of a backport.

Authored using Claude, reviewed by me.

@read-the-docs-community

read-the-docs-community Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #35080091 | 📁 Comparing a4b8ab9 against main (e878e1e)

  🔍 Preview build  

2 files changed
± c-api/marshal.html
± whatsnew/changelog.html

…mps stable

Set elements with the same dump (NaNs which are distinct objects) were
written in set iteration order, which depends on their addresses.  When
one of them is also referenced elsewhere in the dump, the position of
the reference among them changed from run to run.

Elements with equal dumps are now ordered by when they were first
written in this dump; floats and complex numbers not written yet are
interchangeable, and which one is which is decided when one of them is
referenced again.  This relies on the writer knowing exactly which
objects it has written (pythongh-129724).

The elements are sorted with qsort() on the bytes of their dumps,
in an array instead of a list of tuples.
@gpshead
gpshead force-pushed the marshal-set-tie-order branch from a4b8ab9 to b3dea86 Compare October 13, 2026 04:41
@gpshead
gpshead marked this pull request as ready for review October 13, 2026 04:43
@gpshead
gpshead marked this pull request as draft October 13, 2026 04:47
@gpshead
gpshead marked this pull request as ready for review October 13, 2026 05:01
@gpshead
gpshead requested a review from methane October 25, 2026 07:30
Comment thread Python/marshal.c Resolved
Code run while a later element is dumped (its __buffer__() method, an
audit hook) can remove an element from the set and add another one, so
that the size does not change.  The removed element is still written,
but it is then referenced only by the writer, so w_ref() does not enter
it in the hashtable, and w_tie_set_items() dereferenced a NULL entry.
Such an element cannot be written again, so it is skipped.

Reported by Inada Naoki in the review of pythonGH-159197.
@gpshead
gpshead merged commit 3c23ebf into python:main Nov 1, 2026
56 checks passed
@gpshead
gpshead deleted the marshal-set-tie-order branch November 1, 2026 03:22
@gpshead gpshead added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Nov 1, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @gpshead, I could not cleanly backport this to 3.14 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker 3c23ebfdf7750cff757e50e6e8c753f89ce849c6 3.14

@miss-islington-app

Copy link
Copy Markdown

Sorry, @gpshead, I could not cleanly backport this to 3.15 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker 3c23ebfdf7750cff757e50e6e8c753f89ce849c6 3.15

@gpshead gpshead removed the needs backport to 3.14 bugs and security fixes label Nov 1, 2026
@gpshead gpshead removed the needs backport to 3.15 bugs and security fixes label Nov 1, 2026
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.

2 participants