Skip to content

Fix reference leak when defining a Struct type - #1194

Merged
Siyet merged 2 commits into
mainfrom
fix-struct-annotations-leak
Sep 29, 2026
Merged

Siyet merged 2 commits into
mainfrom
fix-struct-annotations-leak

Conversation

@Siyet

@Siyet Siyet commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

structmeta_collect_fields owns annotations, and also module_ns when a string ClassVar annotation is present. Both were released only on the failure paths, so every successful struct class definition that carries annotations retained its annotations dict, and a class carrying a string ClassVar annotation additionally retained the namespace of its module.

Release both on the success path. The regression tests use weakrefs rather than reference counts, so they are meaningful on free-threaded builds as well.

Fixes #1193.

provinzkraut
provinzkraut previously approved these changes Sep 20, 2026
@Siyet

Siyet commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, my push dropped your approval. The fix itself is unchanged; the extra commit only touches tests.

Both original tests went through borrowed __annotations__ branch, so a leak confined to the __annotate__ branch, which a plain class body takes on 3.14, would have slipped past both of them and past the rest of the suite. The new cases close that, add the typing.ClassVar spelling as the second call site that reaches the module namespace, and assert the annotation is still a string, so the case fails instead of passing vacuously if that ever changes.

Could you take another look when you have a moment?

@Siyet
Siyet added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 7930780 Sep 29, 2026
24 checks passed
@Siyet
Siyet deleted the fix-struct-annotations-leak branch September 29, 2026 10:22
pull Bot pushed a commit to Mu-L/msgspec that referenced this pull request Sep 29, 2026
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 <4410812+Siyet@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
docs-preview — d82d1c94 Deployed Sep 21, 2026 by Siyet via Deploy preview #508
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.

Every Struct class definition leaks its annotations dict

2 participants