Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a3292c2
Do not track frozenset objects with immutables
eendebakpt Oct 16, 2025
cd294a6
cleanup
eendebakpt Oct 16, 2025
7e28cf2
cleanup
eendebakpt Oct 16, 2025
30057a5
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Oct 16, 2025
c4deb03
fix test
eendebakpt Oct 16, 2025
607237a
📜🤖 Added by blurb_it.
blurb-it[bot] Oct 16, 2025
2735a71
Update Objects/setobject.c
eendebakpt Oct 17, 2025
c05db54
make sure PySet_Add tracks frozensets if needed
eendebakpt Oct 24, 2025
7f6bc4b
Merge branch 'frozenset_immutable_tracking' of github.com:eendebakpt/…
eendebakpt Oct 24, 2025
0b97604
review comment
eendebakpt Oct 24, 2025
948daed
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Oct 24, 2025
08e22c3
use _testcapi for testing
eendebakpt Oct 26, 2025
62afc76
whitespace
eendebakpt Oct 26, 2025
eab653e
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Oct 26, 2025
37fc61d
Apply suggestions from code review
eendebakpt Oct 26, 2025
4f8bda7
review comment
eendebakpt Oct 26, 2025
e9d42b4
Merge branch 'frozenset_immutable_tracking' of github.com:eendebakpt/…
eendebakpt Oct 26, 2025
4b39149
Apply suggestions from code review
eendebakpt Oct 28, 2025
2859802
review comments
eendebakpt Oct 28, 2025
08f43c5
review comments
eendebakpt Oct 28, 2025
d12102c
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Jan 18, 2026
5f8b1f2
Update Objects/setobject.c
eendebakpt Jan 18, 2026
ae5cc7f
review comments
eendebakpt Jan 23, 2026
786019d
Update Lib/test/test_set.py
eendebakpt Jan 23, 2026
f37f46e
Update Modules/_testcapimodule.c
eendebakpt Jan 23, 2026
afd8a5b
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Jan 23, 2026
377e6d8
Apply suggestions from code review
eendebakpt Jan 23, 2026
c62fa9a
revert to fastcall
eendebakpt Jan 24, 2026
69b728f
Merge branch 'frozenset_immutable_tracking' of github.com:eendebakpt/…
eendebakpt Jan 24, 2026
71d9eba
fix header
eendebakpt Jan 25, 2026
5c19de2
Fully write test in C
eendebakpt Jan 25, 2026
e7dd248
adjust tests
eendebakpt Jan 26, 2026
075b582
cleanup tests
eendebakpt Jan 26, 2026
3dd4c95
cleanup tests
eendebakpt Jan 26, 2026
8d864ef
rework
eendebakpt Jan 26, 2026
e262963
refactor code
eendebakpt Jan 26, 2026
6027391
cleanup
eendebakpt Jan 26, 2026
6269f68
Merge branch 'main' into frozenset_immutable_tracking
eendebakpt Jan 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Lib/test/test_capi/test_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ def test_add(self):
# CRASHES: add(instance, NULL)
# CRASHES: add(NULL, NULL)

def test_add_frozenset(self):
add = _testlimitedcapi.set_add
frozen_set = frozenset()
# test adding an element to a non-uniquely referenced frozenset throws an exception
self.assertRaises(SystemError, add, frozen_set, 1)

def test_discard(self):
discard = _testlimitedcapi.set_discard
for cls in (set, set_subclass):
Expand Down
5 changes: 4 additions & 1 deletion Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1880,7 +1880,10 @@ class S(set):
check(S(), set(), '3P')
class FS(frozenset):
__slots__ = 'a', 'b', 'c'
check(FS(), frozenset(), '3P')

class mytuple(tuple):
pass
check(FS([mytuple()]), frozenset([mytuple()]), '3P')
from collections import OrderedDict
class OD(OrderedDict):
__slots__ = 'a', 'b', 'c'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Frozenset objects with immutable elements are no longer tracked by the garbage collector.
67 changes: 67 additions & 0 deletions Modules/_testlimitedcapi/set.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,71 @@ test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj))
return NULL;
}

static PyObject *
raiseTestError(const char* test_name, const char* msg)
{
PyObject *exc = PyErr_GetRaisedException();
PyErr_Format(exc, "%s: %s", test_name, msg);
return NULL;
}

static PyObject *
test_frozenset_add_in_capi_tracking_immutable(PyObject *self, PyObject *Py_UNUSED(ignored))
{
// Test: GC tracking - frozenset with only immutable items should not be tracked
PyObject *frozenset = PyFrozenSet_New(NULL);
if (frozenset == NULL) {
return NULL;
}
if (PySet_Add(frozenset, Py_True) < 0) {
Py_DECREF(frozenset);
return NULL;
}
if (PyObject_GC_IsTracked(frozenset)) {
Py_DECREF(frozenset);
return raiseTestError("test_frozenset_add_in_capi_tracking_immutable",
"frozenset with only int should not be GC tracked");
}
Py_DECREF(frozenset);
Py_RETURN_NONE;
}

static PyObject *
test_frozenset_add_in_capi_tracking(PyObject *self, PyObject *Py_UNUSED(ignored))
{
// Test: GC tracking - frozenset with tracked object should be tracked
PyObject *frozenset = PyFrozenSet_New(NULL);
if (frozenset == NULL) {
return NULL;
}

PyObject *tracked_obj = PyErr_NewException("_testlimitedcapi.py_set_add", NULL, NULL);
if (tracked_obj == NULL) {
goto error;
}
if (!PyObject_GC_IsTracked(tracked_obj)) {
return raiseTestError("test_frozenset_add_in_capi_tracking",
"test object should be tracked");
}
if (PySet_Add(frozenset, tracked_obj) < 0) {
goto error;
}
Py_DECREF(tracked_obj);
if (!PyObject_GC_IsTracked(frozenset)) {
Py_DECREF(frozenset);
return raiseTestError("test_frozenset_add_in_capi_tracking",
"frozenset with with GC tracked object should be tracked");
}
Py_DECREF(frozenset);
Py_RETURN_NONE;

error:
Py_DECREF(frozenset);
Py_XDECREF(tracked_obj);
return NULL;
}


static PyObject *
test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_UNUSED(obj))
{
Expand Down Expand Up @@ -219,6 +284,8 @@ static PyMethodDef test_methods[] = {
{"set_clear", set_clear, METH_O},

{"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS},
{"test_frozenset_add_in_capi_tracking", test_frozenset_add_in_capi_tracking, METH_NOARGS},
{"test_frozenset_add_in_capi_tracking_immutable", test_frozenset_add_in_capi_tracking_immutable, METH_NOARGS},
{"test_set_contains_does_not_convert_unhashable_key",
test_set_contains_does_not_convert_unhashable_key, METH_NOARGS},

Expand Down
36 changes: 34 additions & 2 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,26 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable)
return make_new_set(type, iterable);
}

void
// gh-140232: check whether a frozenset can be untracked from the GC
_PyFrozenSet_MaybeUntrack(PyObject *op)
{
assert(op != NULL);
// subclasses of a frozenset can generate reference cycles, so do not untrack
if (!PyFrozenSet_CheckExact(op)) {
return;
}
// if no elements of a frozenset are tracked by the GC, we untrack the object
Py_ssize_t pos = 0;
setentry *entry;
while (set_next((PySetObject *)op, &pos, &entry)) {
if (_PyObject_GC_MAY_BE_TRACKED(entry->key)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use faster _PyType_IS_GC(Py_TYPE(entry->key)) as in maybe_tracked from Objects/tupleobject.c?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure performance matters a lot, but I would prefer to have it consistent with what is used in tupleobject.c. Unless there are objections, I will change the implementation to use the maybe_tracked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, leaving this as it is. The maybe_tracked has a comment mentioning it is a temporary workaround.

return;
}
}
_PyObject_GC_UNTRACK(op);
}

static PyObject *
make_new_frozenset(PyTypeObject *type, PyObject *iterable)
{
Expand All @@ -1379,7 +1399,11 @@ make_new_frozenset(PyTypeObject *type, PyObject *iterable)
/* frozenset(f) is idempotent */
return Py_NewRef(iterable);
}
return make_new_set(type, iterable);
PyObject *obj = make_new_set(type, iterable);
if (obj != NULL) {
_PyFrozenSet_MaybeUntrack(obj);
}
return obj;
}

static PyObject *
Expand Down Expand Up @@ -2932,7 +2956,11 @@ PySet_New(PyObject *iterable)
PyObject *
PyFrozenSet_New(PyObject *iterable)
{
return make_new_set(&PyFrozenSet_Type, iterable);
PyObject *result = make_new_set(&PyFrozenSet_Type, iterable);
if (result != 0) {
_PyFrozenSet_MaybeUntrack(result);
}
return result;
}

Py_ssize_t
Expand Down Expand Up @@ -3010,6 +3038,10 @@ PySet_Add(PyObject *anyset, PyObject *key)
// API limits the usage of `PySet_Add` to "fill in the values of brand
// new frozensets before they are exposed to other code". In this case,
// this can be done without a lock.
// since another key is added to the set, we must track the frozenset if needed
if (PyFrozenSet_CheckExact(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) {
_PyObject_GC_TRACK(anyset);
}
return set_add_key((PySetObject *)anyset, key);
}

Expand Down
Loading