Skip to content

Commit 419bd3c

Browse files
committed
gh-155176: Don't track frozendicts whose contents can never be tracked by the GC
1 parent 5918085 commit 419bd3c

3 files changed

Lines changed: 76 additions & 6 deletions

File tree

Lib/test/test_dict.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,6 +1869,26 @@ def test_copy(self):
18691869
self.assertEqual(d2, frozendict(x=1, y=2))
18701870
self.assertEqual(type(d2), frozendict)
18711871

1872+
def test_gc_tracking(self):
1873+
self.assertFalse(gc.is_tracked(frozendict()))
1874+
self.assertFalse(gc.is_tracked(frozendict({1: 2})))
1875+
self.assertFalse(gc.is_tracked(frozendict.fromkeys('ab', 1)))
1876+
self.assertFalse(gc.is_tracked(frozendict({1: 2}) | {3: 4}))
1877+
1878+
self.assertTrue(gc.is_tracked(frozendict({1: [2]})))
1879+
# subclasses can create reference cycles, they are always tracked
1880+
self.assertTrue(gc.is_tracked(FrozenDict({1: 2})))
1881+
1882+
# a reference cycle through a tracked frozendict is collectable
1883+
class Obj:
1884+
pass
1885+
obj = Obj()
1886+
obj.fd = frozendict({1: obj})
1887+
ref = weakref.ref(obj)
1888+
del obj
1889+
gc.collect()
1890+
self.assertIsNone(ref())
1891+
18721892
def test_merge(self):
18731893
# test "a | b" operator
18741894
self.assertEqual(frozendict(x=1) | frozendict(y=2),
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:class:`frozendict` objects whose keys and values can never be tracked by
2+
the garbage collector are no longer tracked, like :class:`frozenset`
3+
objects.

Objects/dictobject.c

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ As a consequence of this, split keys have a maximum size of 16.
139139
static PyObject* frozendict_new(PyTypeObject *type, PyObject *args,
140140
PyObject *kwds);
141141
static PyObject* frozendict_new_untracked(PyTypeObject *type);
142+
static void frozendict_maybe_track(PyObject *op);
142143
static PyObject* dict_new(PyTypeObject *type, PyObject *args, PyObject *kwds);
143144
static PyObject* dict_new_untracked(PyTypeObject *type);
144145
static int dict_merge(PyObject *a, PyObject *b, int override, PyObject **dupkey);
@@ -3479,11 +3480,18 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
34793480
// gh-151722: If cls constructor returns a frozendict which is tracked by
34803481
// the GC, create a frozendict copy which is not tracked by the GC.
34813482
//
3483+
// gh-155176: Since frozendicts whose contents can never be tracked by the
3484+
// GC are left untracked, being untracked no longer implies that d is
3485+
// private: also make a copy if d is not uniquely referenced, otherwise
3486+
// inserting keys below would mutate an object visible elsewhere.
3487+
//
34823488
// At the function exit, return cls(fd) where fd is a frozendict.
34833489
//
34843490
// Untracking the frozendict requires tracking again the frozendict on
34853491
// error which is more complicated. It's easier to work on a copy.
3486-
if (PyFrozenDict_Check(d) && _PyObject_GC_IS_TRACKED(d)) {
3492+
if (PyFrozenDict_Check(d)
3493+
&& (_PyObject_GC_IS_TRACKED(d)
3494+
|| !_PyObject_IsUniquelyReferenced(d))) {
34873495
need_copy = 1;
34883496

34893497
PyObject *copy = frozendict_new_untracked(&PyFrozenDict_Type);
@@ -3637,7 +3645,12 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
36373645
Py_SETREF(d, copy);
36383646
}
36393647
else if (!_PyObject_GC_IS_TRACKED(d)) {
3640-
_PyObject_GC_TRACK(d);
3648+
if (PyFrozenDict_Check(d)) {
3649+
frozendict_maybe_track(d);
3650+
}
3651+
else {
3652+
_PyObject_GC_TRACK(d);
3653+
}
36413654
}
36423655
return d;
36433656
}
@@ -5202,7 +5215,12 @@ _PyDict_Or(PyObject *self, PyObject *other)
52025215
Py_DECREF(new);
52035216
return NULL;
52045217
}
5205-
_PyObject_GC_TRACK(new);
5218+
if (PyFrozenDict_Check(new)) {
5219+
frozendict_maybe_track(new);
5220+
}
5221+
else {
5222+
_PyObject_GC_TRACK(new);
5223+
}
52065224
return new;
52075225
}
52085226

@@ -5472,7 +5490,7 @@ frozendict_vectorcall(PyObject *type, PyObject * const*args,
54725490
}
54735491
}
54745492

5475-
_PyObject_GC_TRACK(self);
5493+
frozendict_maybe_track(self);
54765494
return self;
54775495
}
54785496

@@ -8502,6 +8520,35 @@ frozendict_new_untracked(PyTypeObject *type)
85028520
return d;
85038521
}
85048522

8523+
/* Track a fully built frozendict in the GC, unless it can never be part of
8524+
a reference cycle: an exact frozendict whose keys and values are all
8525+
guaranteed to never be tracked by the GC. Subclasses can create reference
8526+
cycles, so they are always tracked. */
8527+
static void
8528+
frozendict_maybe_track(PyObject *op)
8529+
{
8530+
assert(PyFrozenDict_Check(op));
8531+
assert(!_PyObject_GC_IS_TRACKED(op));
8532+
8533+
if (PyFrozenDict_CheckExact(op)) {
8534+
PyObject *key, *value;
8535+
Py_ssize_t pos = 0;
8536+
int track = 0;
8537+
while (_PyDict_Next(op, &pos, &key, &value, NULL)) {
8538+
if (_PyObject_GC_MAY_BE_TRACKED(key)
8539+
|| _PyObject_GC_MAY_BE_TRACKED(value))
8540+
{
8541+
track = 1;
8542+
break;
8543+
}
8544+
}
8545+
if (!track) {
8546+
return;
8547+
}
8548+
}
8549+
_PyObject_GC_TRACK(op);
8550+
}
8551+
85058552
static PyObject *
85068553
frozendict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
85078554
{
@@ -8520,7 +8567,7 @@ frozendict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
85208567
assert(kwds == NULL);
85218568
}
85228569

8523-
_PyObject_GC_TRACK(d);
8570+
frozendict_maybe_track(d);
85248571
return d;
85258572
}
85268573

@@ -8566,7 +8613,7 @@ frozendict_copy_impl(PyFrozenDictObject *self)
85668613

85678614
PyObject *copy = anydict_copy_untracked((PyObject*)self);
85688615
if (copy != NULL) {
8569-
_PyObject_GC_TRACK(copy);
8616+
frozendict_maybe_track(copy);
85708617
}
85718618
return copy;
85728619
}

0 commit comments

Comments
 (0)