Skip to content

Commit 7930780

Browse files
authored
Fix reference leak when defining a Struct type (#1194)
`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. --------- Co-authored-by: Tseluiko Aleksandr <[email protected]>
1 parent 55f5cdd commit 7930780

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

‎src/msgspec/_core.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6206,6 +6206,8 @@ structmeta_collect_fields(StructMetaInfo *info, MsgspecState *mod, bool kwonly)
62066206

62076207
if (structmeta_process_default(info, field) < 0) goto error;
62086208
}
6209+
Py_DECREF(annotations);
6210+
Py_XDECREF(module_ns);
62096211
return 0;
62106212
error:
62116213
Py_DECREF(annotations);

‎tests/unit/test_struct.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,69 @@ class Test(Struct):
946946
assert sys.getrefcount(data) <= 4
947947

948948

949+
def test_struct_definition_does_not_leak_annotations():
950+
"""Defining a struct type must not retain its `__annotations__` dict"""
951+
952+
class Annotation:
953+
pass
954+
955+
ref = weakref.ref(Annotation)
956+
957+
cls = defstruct("Temp", [("x", Annotation)])
958+
del cls, Annotation
959+
gc.collect()
960+
961+
assert ref() is None
962+
963+
964+
def test_struct_definition_does_not_leak_class_body_annotations():
965+
"""A class body must not retain the annotations built for it"""
966+
967+
def define_struct_type():
968+
class Annotation:
969+
pass
970+
971+
class Example(Struct):
972+
x: Annotation
973+
974+
return weakref.ref(Annotation)
975+
976+
refs = [define_struct_type() for _ in range(3)]
977+
gc.collect()
978+
979+
assert [ref() for ref in refs] == [None, None, None]
980+
981+
982+
@pytest.mark.parametrize("spelling", ["ClassVar", "typing.ClassVar"])
983+
def test_struct_definition_does_not_leak_module_namespace(spelling):
984+
"""A string `ClassVar` annotation must not retain the module namespace"""
985+
986+
class Sentinel:
987+
pass
988+
989+
def define_in_temp_module():
990+
with temp_module(
991+
"import typing\nfrom typing import ClassVar\nimport msgspec\n"
992+
) as mod:
993+
sentinel = Sentinel()
994+
mod.__dict__["sentinel"] = sentinel
995+
exec(
996+
"from __future__ import annotations\n"
997+
"class Ex(msgspec.Struct):\n"
998+
f" a: {spelling}[int] = 1\n"
999+
" x: int = 0\n",
1000+
mod.__dict__,
1001+
)
1002+
# The leak only exists while the annotations are strings
1003+
assert mod.Ex.__annotations__["x"] == "int"
1004+
return weakref.ref(sentinel)
1005+
1006+
refs = [define_in_temp_module() for _ in range(3)]
1007+
gc.collect()
1008+
1009+
assert [ref() for ref in refs] == [None, None, None]
1010+
1011+
9491012
def test_struct_gc_not_added_if_not_needed():
9501013
"""Structs aren't tracked by GC until/unless they reference a container type"""
9511014

0 commit comments

Comments
 (0)