Skip to content
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
25 changes: 25 additions & 0 deletions Lib/test/test_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import warnings
import weakref
from random import randrange, shuffle
import _testcapi
from test import support
from test.support import warnings_helper

Expand Down Expand Up @@ -2154,6 +2155,30 @@ def test_cuboctahedron(self):
for cubevert in edge:
self.assertIn(cubevert, g)

class TestPySet_Add(unittest.TestCase):
def test_set(self):
# Test the PySet_Add c-api for set objects
s = set()
assert _testcapi.pyset_add(s, 1) == {1}
self.assertRaises(TypeError, _testcapi.pyset_add, s, [])

def test_frozenset(self):
# Test the PySet_Add c-api for frozenset objects
assert _testcapi.pyset_add(frozenset(), 1) == frozenset([1])
frozen_set = frozenset()
self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why the second test fails, whereas the first succeed.

Copy link
Contributor Author

@eendebakpt eendebakpt Oct 28, 2025

Choose a reason for hiding this comment

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

The second test fails because the argument frozen_set is not uniquely referenced. The error is raised here:

(!PyFrozenSet_Check(anyset) || !_PyObject_IsUniquelyReferenced(anyset))) {

I will add a comment to the test


def test_frozenset_gc_tracking(self):
# see gh-140234
class TrackedHashableClass():
pass

a = TrackedHashableClass()
result_set = _testcapi.pyset_add(frozenset(), 1)
assert not gc.is_tracked(result_set)
result_set = _testcapi.pyset_add(frozenset(), a)
assert gc.is_tracked(result_set)


#==============================================================================

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 @@ -1876,7 +1876,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.
24 changes: 23 additions & 1 deletion Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2435,6 +2435,27 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args))
}



static PyObject *
// Interface to PySet_Add, returning the set
pyset_add(PyObject* self, PyObject* const* args, Py_ssize_t nargsf)
{
Py_ssize_t nargs = _PyVectorcall_NARGS(nargsf);
if (nargs != 2) {
PyErr_SetString(PyExc_ValueError, "pyset_add requires exactly two arguments");
return NULL;
}
PyObject *set = args[0];
PyObject *item = args[1];

int return_value = PySet_Add(set, item);
if (return_value < 0) {
return NULL;
}

return Py_NewRef(set);
}

// Used by `finalize_thread_hang`.
#if defined(_POSIX_THREADS) && !defined(__wasi__)
static void finalize_thread_hang_cleanup_callback(void *Py_UNUSED(arg)) {
Expand Down Expand Up @@ -2625,7 +2646,7 @@ static PyMethodDef TestMethods[] = {
{"return_null_without_error", return_null_without_error, METH_NOARGS},
{"return_result_with_error", return_result_with_error, METH_NOARGS},
{"getitem_with_error", getitem_with_error, METH_VARARGS},
{"Py_CompileString", pycompilestring, METH_O},
{"Py_CompileString", pycompilestring, METH_O},
{"raise_SIGINT_then_send_None", raise_SIGINT_then_send_None, METH_VARARGS},
{"stack_pointer", stack_pointer, METH_NOARGS},
#ifdef W_STOPCODE
Expand All @@ -2646,6 +2667,7 @@ static PyMethodDef TestMethods[] = {
{"gen_get_code", gen_get_code, METH_O, NULL},
{"get_feature_macros", get_feature_macros, METH_NOARGS, NULL},
{"test_code_api", test_code_api, METH_NOARGS, NULL},
{"pyset_add", _PyCFunction_CAST(pyset_add), METH_FASTCALL, NULL},
{"settrace_to_error", settrace_to_error, METH_O, NULL},
{"settrace_to_record", settrace_to_record, METH_O, NULL},
{"test_macros", test_macros, METH_NOARGS, NULL},
Expand Down
28 changes: 26 additions & 2 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,23 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable)
return make_new_set(type, iterable);
}

void
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment to explain the purpose of this function with a link to the GitHub issue.

_PyFrozenSet_MaybeUntrack(PyObject *op)
{
if (op == NULL || !PyFrozenSet_CheckExact(op)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not untracking frozenset subtypes?

Copy link
Member

@efimov-mikhail efimov-mikhail Oct 28, 2025

Choose a reason for hiding this comment

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

We can make a cycle using frozenset subtype:

>>> class F(frozenset):
...     pass
...     
>>> f = F([1,2,3])
>>> f.cycle = f

So, we need to track them.

Copy link
Member

Choose a reason for hiding this comment

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

Oh ok. In this case, please add a comment explaining that :-)

return;
}
// if no elements of a frozenset are tracked, 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.

return;
}
}
_PyObject_GC_UNTRACK(op);
}

static PyObject *
make_new_frozenset(PyTypeObject *type, PyObject *iterable)
{
Expand All @@ -1185,7 +1202,9 @@ 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);
_PyFrozenSet_MaybeUntrack(obj);
return obj;
}

static PyObject *
Expand Down Expand Up @@ -2710,7 +2729,9 @@ PySet_New(PyObject *iterable)
PyObject *
PyFrozenSet_New(PyObject *iterable)
{
return make_new_set(&PyFrozenSet_Type, iterable);
PyObject *result = make_new_set(&PyFrozenSet_Type, iterable);
_PyFrozenSet_MaybeUntrack(result);
return result;
}

Py_ssize_t
Expand Down Expand Up @@ -2779,6 +2800,9 @@ PySet_Add(PyObject *anyset, PyObject *key)
return -1;
}

if (PyFrozenSet_CheckExact(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) {
_PyObject_GC_TRACK(anyset);
}
int rv;
Py_BEGIN_CRITICAL_SECTION(anyset);
rv = set_add_key((PySetObject *)anyset, key);
Expand Down
Loading