Reject `gc=False` on struct types with a weakref slot by Siyet · Pull Request #1207 · msgspec/msgspec

Siyet · 2026-09-25T06:15:16Z

On Python 3.12 and later, releasing an instance of a struct type declared with both gc=False and weakref=True goes wrong. On default builds it reads memory outside the instance's allocation, and creating one instance is enough, since the read happens even when no weak reference was ever taken:

import msgspec

class Ex(msgspec.Struct, gc=False, weakref=True):
    x: int

Ex(1)

The process usually crashes. On free-threaded builds there is no immediate crash: the instance is released from the wrong address, the allocator's own state is corrupted, and the failure arrives later and somewhere else.

Since 3.12 the __weakref__ slot is not inside the object. It sits at a fixed negative offset, in the pre-header that is allocated in front of the instance. A gc=False type clears Py_TPFLAGS_HAVE_GC and releases the instance with PyObject_Free on the object itself, so the block is released from the wrong address. Outside free-threaded builds that pre-header is also smaller, which leaves the slot before the start of the allocation.

This rejects the combination at class creation, the way gc=False with dict=True is already rejected. The check also covers the other routes to it: defstruct, inheriting either option from a base class, and a gc=False struct whose base is a plain mixin with __slots__ = ("__weakref__",). The structs guide documents the combination as unsupported, and why.

The restriction is unconditional, so it also applies to 3.10 and 3.11, where the slot is inside the object and the combination works. That is deliberate: the same class definition then behaves the same way on every supported version, and an upgrade of Python cannot turn working code into an error.

An earlier revision of this branch kept such a type GC enabled instead, which also fixed the crash. The discussion below records why the restriction was chosen over it.

Since Python 3.12 the `__weakref__` slot is not inside the object. It sits at a fixed negative offset, in the pre-header, next to the `PyGC_Head`. Clearing `Py_TPFLAGS_HAVE_GC` for a `gc=False` type removes that `PyGC_Head` and shrinks the pre-header, while the slot's offset stays the same, so the slot ends up just below the allocation. Keep such a type GC enabled so the offset stays valid, and untrack instances explicitly in each construction path and on attribute assignment. Python 3.10, 3.11 and free-threaded builds keep the layout they had.

A `__weakref__` slot at a negative offset is in the pre-header on free-threaded builds as well. Clearing `Py_TPFLAGS_HAVE_GC` there releases the instance from the wrong address, which corrupts the allocator's state instead of crashing at once, so the check is on the offset alone.

A `gc=False` struct type whose weakref slot lives in the GC pre-header stays GC enabled, so its instances start out tracked. Untracking them at the end of each construction path left a window during `__post_init__` where `gc.is_tracked` returned True, and a copy taken there inherited that state. Untrack once in `Struct_alloc` instead, so there is no such window. Also add coverage for the deallocation path, which was untested: releasing such an instance with `PyObject_Free` leaves the young generation counter climbing. Group the new tests with the other `gc=False` ones.

sobolevn · 2026-09-25T11:08:08Z

+            if cls.__weakrefoffset__ < 0:
+                assert cls.__flags__ & Py_TPFLAGS_HAVE_GC
+            else:
+                assert not cls.__flags__ & Py_TPFLAGS_HAVE_GC


Suggested change

assert not cls.__flags__ & Py_TPFLAGS_HAVE_GC

assert cls.__flags__ & Py_TPFLAGS_HAVE_GC == 0

sobolevn · 2026-09-25T11:08:41Z

+                assert cls.__flags__ & Py_TPFLAGS_HAVE_GC
+            else:
+                assert not cls.__flags__ & Py_TPFLAGS_HAVE_GC
+            t = cls([1, 2, 3])


Suggested change

t = cls([1, 2, 3])

inst = cls([1, 2, 3])

t looks like a type variable to me :)

sobolevn · 2026-09-25T11:10:22Z

+                ref = weakref.ref(Test(1))
+                assert ref() is None
+            grew = gc.get_count()[0] - before
+        assert grew < 100


I find this test a bit off: why does it use 100 as the threshold? Do we even need to test the GC count here? Why only the youngest generation?

sobolevn

I asked about this problem several of my collegues who understand gc and weakrefs better than I do :)

It was proposed to ban this combination alltogether. See python/cpython#134786

provinzkraut · 2026-09-25T13:55:09Z

It was proposed to ban this combination alltogether. See python/cpython#134786

Seems reasonable. Maybe we could do this as well? Simply raise an error when both are being set, and explicitly document it as not supported due to ?

The earlier approach on this branch kept such a type GC enabled so that the pre-header stayed valid. That fixed the crash, but on 3.12 and later left the combination with no memory saving and no reduction in collection frequency, and an instance of a `gc=False` class could still end up tracked through `__class__` assignment or through resurrection from `__del__`. Raise `ValueError` at class creation instead, in the same place as the existing rejection of `gc=False` with `dict=True`. The check also covers a weakref slot inherited from a base class.

Siyet · 2026-09-27T07:29:28Z

Done, thanks for digging into this and for asking around. The structs guide documents it as unsupported, and why.

I had argued against the ban in the first version of the description, and one finding still cuts that way: the combination is not inert on 3.12+, since untracked instances keep each collection cheaper. But it saves no memory. An instance of a gc=False class could also end up tracked, through __class__ assignment and through resurrection from __del__.

The in-object slot that python/cpython#134786 suggests would not cover every route: an inherited weakref slot keeps the offset fixed by its base class.

I threw a pile of AI agents at GitHub code search and found no one using both options together outside this repository. That guarantees nothing, of course, but it is mildly reassuring.

@sobolevn your three comments were on tests this revision removes, so I have not carried them over.

sobolevn · 2026-09-27T08:37:43Z

            class Test(Base, **opts):
                pass

+    @pytest.mark.parametrize(


If you have to parametrize cases, we can just make different tests :)

And just make the assert_whatever method to unify the check code.

Done: each case is now its own test, and the check is in a shared assert_gc_false_with_weakref_rejected helper.

sobolevn · 2026-09-27T08:39:19Z

            goto cleanup;
        }
+        else if (info.weakref == OPT_TRUE || info.already_has_weakref) {
+            PyErr_SetString(PyExc_ValueError, "Cannot set gc=False and weakref=True");


I suggest using

`gc=False` and `weakref=True`

adding a small qualifier why (something that says they interfere with each other) would also help. Cannot set gc=False and weakref=True might be interpreted that, for some reason, those two attributes are not settable, but it's unclear why.

Updated both: the options are in backticks now, and the mesage includes the reason:

Cannot set `gc=False` and `weakref=True`: on Python 3.12+ the weakref slot is stored in a pre-header in front of the instance, and releasing a `gc=False` instance with that slot corrupts memory

Quote the options in the message and state why they cannot be combined. In the message, the structs guide and the changelog, describe the weakref slot as stored in a pre-header in front of the instance, since on free-threaded builds that pre-header holds no GC header. Replace the parametrized test with one test per route, with the check in a shared helper.

Siyet · 2026-09-28T21:10:31Z

@sobolevn @provinzkraut, the trusted publisher is configured now (#1134), so 0.22.0 can be released. Since this PR is a breaking change I'd like to land it before we cut 0.22.0 rather than leave it for the next minor release. It should affect almost no one: I found no public project on GitHub that uses msgspec with both gc=False and weakref=True, and on 3.12+ releasing an instance of such a class already corrupts memory. The latest review comments are addressed. Could you take another look?

sobolevn · 2026-09-28T21:36:52Z

+        else if (info.weakref == OPT_TRUE || info.already_has_weakref) {
+            PyErr_SetString(
+                PyExc_ValueError,
+                "Cannot set `gc=False` and `weakref=True`: on Python 3.12+ the "


sorry, my suggestion was incorrect, no other error messages use backticks. the reason also seems a bit too verbose. I propose this wording:

Cannot set gc=False and weakref=True due to possible memory corruption

I would still prefer something like Cannot set gc=False and weakref=True at the same time, due to possible memory corruption or Cannot set gc=False while also setting weakref=True, due to possible memory corruption, just to be very explicit that it's an issue with setting both at the same time.

Changed it to Cannot set gc=False and weakref=True at the same time, due to possible memory corruption: no backticks, a short reason instead of the long one, and "at the same time" to make it clear that the combination is the problem.

sobolevn · 2026-09-28T21:37:23Z

+        assert not gc.is_tracked(NoGC([1]))
+        assert not gc.is_tracked(SubNoGC([1], [2]))
+
+        obj = WeakRef(1)


assert that it is tracked with the gc :)

Done. An instance that holds only an int is not tracked, so the field is a list now, and there is an assertion that the instance is tracked.

sobolevn · 2026-09-28T21:37:49Z

+        class SubNoGC(NoGC, gc=False):
+            y: list
+
+        class WeakRef(Struct, weakref=True):


Suggested change

class WeakRef(Struct, weakref=True):

class WithWeakRef(Struct, weakref=True):

I was a bit confused that WeakRef is a struct at first :)

Renamed, thx. It reads beter this way.

Drop the backticks, as in the nearby `gc=False` messages, replace the detailed reason with a short one and state that the two options cannot be set at the same time. In the test for each option on its own, rename the weakref struct, give it a `list` field and check that its instance is tracked, since an instance that holds only an `int` is not.

sobolevn

Thank you!

Entries for msgspec#1194, msgspec#1196, msgspec#1197, msgspec#1199 and msgspec#1209 are missing from the changelog of the 0.22.0 release. Of the changes merged after msgspec#1211 moved the unreleased entries into the 0.22.0 section, only msgspec#1207 added its own entry. This adds the missing entries next to the related entries in that section, and marks msgspec#1196 as a breaking change: objects that expose `__dataclass_fields__` only through instance attribute access are no longer encoded as dataclasses. Co-authored-by: Tseluiko Aleksandr <[email protected]>

Siyet added 3 commits September 25, 2026 09:14

Siyet requested a review from sobolevn September 25, 2026 11:16

sobolevn reviewed Sep 25, 2026

View reviewed changes

Siyet changed the title ~~Fix a crash on Python 3.12+ for gc=False structs with a weakref slot~~ Reject gc=False on struct types with a weakref slot Sep 27, 2026

sobolevn reviewed Sep 27, 2026

View reviewed changes

Siyet added 2 commits September 28, 2026 09:49

Merge main into fix-gc-false-weakref-crash

8399e02

sobolevn reviewed Sep 28, 2026

View reviewed changes

Siyet and others added 2 commits September 29, 2026 09:46

Merge branch 'main' into fix-gc-false-weakref-crash

18bd470

sobolevn approved these changes Sep 29, 2026

View reviewed changes

sobolevn added this pull request to the merge queue Sep 29, 2026

Merged via the queue into main with commit b2d1b7a Sep 29, 2026
24 checks passed

sobolevn deleted the fix-gc-false-weakref-crash branch September 29, 2026 07:26

Siyet mentioned this pull request Sep 29, 2026

Add the missing entries to the 0.22.0 changelog #1215

Merged

	assert not cls.__flags__ & Py_TPFLAGS_HAVE_GC
	assert cls.__flags__ & Py_TPFLAGS_HAVE_GC == 0

	class WeakRef(Struct, weakref=True):
	class WithWeakRef(Struct, weakref=True):

Conversation

Siyet commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

provinzkraut commented Sep 25, 2026

Uh oh!

Siyet commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Siyet commented Sep 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Siyet commented Sep 25, 2026 •

edited

Loading

Siyet commented Sep 27, 2026 •

edited

Loading