Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
26 changes: 26 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,31 @@ 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()
self.assertEqual(_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
self.assertEqual(_testcapi.pyset_add(frozenset(), 1), frozenset([1]))
frozen_set = frozenset()
# if the argument to PySet_Add is a frozenset that is not uniquely references an error is generated
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)
self.assertFalse(gc.is_tracked(result_set))
result_set = _testcapi.pyset_add(frozenset(), a)
self.assertTrue(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.
23 changes: 22 additions & 1 deletion Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2435,6 +2435,26 @@ 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 +2645,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 +2666,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
35 changes: 33 additions & 2 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,26 @@ 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.

// 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.

return;
}
}
_PyObject_GC_UNTRACK(op);
}

static PyObject *
make_new_frozenset(PyTypeObject *type, PyObject *iterable)
{
Expand All @@ -1185,7 +1205,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 @@ -2710,7 +2734,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 @@ -2779,6 +2807,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