Skip to content

Commit 255b321

Browse files
vstinnercorona10
andcommitted
gh-151722: Do not track the dict in the GC in _PyDict_FromKeys()
dict_merge() no longer requires the dictionary to be tracked by the GC. Co-authored-by: Donghee Na <donghee.na@python.org>
1 parent ad38cf8 commit 255b321

2 files changed

Lines changed: 57 additions & 30 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:meth:`!frozendict.fromkeys` now creates a :class:`frozendict` which is not
2+
tracked by the garbage collector, and only tracks it once the dictionary is
3+
fully initialized. Patch by Donghee Na and Victor Stinner.

Objects/dictobject.c

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ static PyObject* frozendict_new(PyTypeObject *type, PyObject *args,
140140
PyObject *kwds);
141141
static PyObject* frozendict_new_untracked(PyTypeObject *type);
142142
static PyObject* dict_new(PyTypeObject *type, PyObject *args, PyObject *kwds);
143+
static PyObject* dict_new_untracked(PyTypeObject *type);
143144
static int dict_merge(PyObject *a, PyObject *b, int override, PyObject **dupkey);
144145
static int dict_contains(PyObject *op, PyObject *key);
145146
static int dict_merge_from_seq2(PyObject *d, PyObject *seq2, int override);
@@ -3414,40 +3415,49 @@ dict_set_fromkeys(PyDictObject *mp, PyObject *iterable, PyObject *value)
34143415
PyObject *
34153416
_PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
34163417
{
3417-
PyObject *it; /* iter(iterable) */
3418+
PyObject *it = NULL; /* iter(iterable) */
34183419
PyObject *key;
34193420
PyObject *d;
34203421
int status;
34213422

3422-
d = _PyObject_CallNoArgs(cls);
3423+
PyTypeObject *cls_type = _PyType_CAST(cls);
3424+
if (PyObject_IsSubclass(cls, (PyObject*)&PyFrozenDict_Type)
3425+
&& cls_type->tp_new == frozendict_new)
3426+
{
3427+
// gh-151722: Create a frozendict copy which is not tracked by the GC.
3428+
d = frozendict_new_untracked(cls_type);
3429+
}
3430+
else {
3431+
d = _PyObject_CallNoArgs(cls);
3432+
}
34233433
if (d == NULL) {
34243434
return NULL;
34253435
}
34263436

3427-
// If cls is a dict or frozendict subclass with overridden constructor,
3428-
// copy the frozendict.
3429-
PyTypeObject *cls_type = _PyType_CAST(cls);
3430-
if (PyFrozenDict_Check(d) && cls_type->tp_new != frozendict_new) {
3437+
// gh-151722: If cls constructor returns a frozendict which is tracked by
3438+
// the GC, create a frozendict copy which is not tracked by the GC.
3439+
if (PyFrozenDict_Check(d) && _PyObject_GC_IS_TRACKED(d)) {
34313440
// Subclass-friendly copy
34323441
PyObject *copy;
34333442
if (PyObject_IsSubclass(cls, (PyObject*)&PyFrozenDict_Type)) {
3434-
copy = frozendict_new(cls_type, NULL, NULL);
3443+
copy = frozendict_new_untracked(cls_type);
34353444
}
34363445
else {
3437-
copy = dict_new(cls_type, NULL, NULL);
3446+
copy = dict_new_untracked(cls_type);
34383447
}
34393448
if (copy == NULL) {
3440-
Py_DECREF(d);
3441-
return NULL;
3449+
goto Fail;
34423450
}
34433451
if (dict_merge(copy, d, 1, NULL) < 0) {
3444-
Py_DECREF(d);
34453452
Py_DECREF(copy);
3446-
return NULL;
3453+
goto Fail;
34473454
}
34483455
Py_SETREF(d, copy);
34493456
}
34503457
assert(!PyFrozenDict_Check(d) || can_modify_dict((PyDictObject*)d));
3458+
if (PyFrozenDict_Check(d)) {
3459+
assert(!_PyObject_GC_IS_TRACKED(d));
3460+
}
34513461

34523462
if (PyDict_CheckExact(d)) {
34533463
if (PyDict_CheckExact(iterable)) {
@@ -3456,23 +3466,23 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
34563466
Py_BEGIN_CRITICAL_SECTION2(d, iterable);
34573467
d = (PyObject *)dict_dict_fromkeys(mp, iterable, value);
34583468
Py_END_CRITICAL_SECTION2();
3459-
return d;
3469+
goto Done;
34603470
}
34613471
else if (PyFrozenDict_CheckExact(iterable)) {
34623472
PyDictObject *mp = (PyDictObject *)d;
34633473

34643474
Py_BEGIN_CRITICAL_SECTION(d);
34653475
d = (PyObject *)dict_dict_fromkeys(mp, iterable, value);
34663476
Py_END_CRITICAL_SECTION();
3467-
return d;
3477+
goto Done;
34683478
}
34693479
else if (PyAnySet_CheckExact(iterable)) {
34703480
PyDictObject *mp = (PyDictObject *)d;
34713481

34723482
Py_BEGIN_CRITICAL_SECTION2(d, iterable);
34733483
d = (PyObject *)dict_set_fromkeys(mp, iterable, value);
34743484
Py_END_CRITICAL_SECTION2();
3475-
return d;
3485+
goto Done;
34763486
}
34773487
}
34783488
else if (PyFrozenDict_CheckExact(d)) {
@@ -3482,27 +3492,26 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)
34823492
Py_BEGIN_CRITICAL_SECTION(iterable);
34833493
d = (PyObject *)dict_dict_fromkeys(mp, iterable, value);
34843494
Py_END_CRITICAL_SECTION();
3485-
return d;
3495+
goto Done;
34863496
}
34873497
else if (PyFrozenDict_CheckExact(iterable)) {
34883498
PyDictObject *mp = (PyDictObject *)d;
34893499
d = (PyObject *)dict_dict_fromkeys(mp, iterable, value);
3490-
return d;
3500+
goto Done;
34913501
}
34923502
else if (PyAnySet_CheckExact(iterable)) {
34933503
PyDictObject *mp = (PyDictObject *)d;
34943504

34953505
Py_BEGIN_CRITICAL_SECTION(iterable);
34963506
d = (PyObject *)dict_set_fromkeys(mp, iterable, value);
34973507
Py_END_CRITICAL_SECTION();
3498-
return d;
3508+
goto Done;
34993509
}
35003510
}
35013511

35023512
it = PyObject_GetIter(iterable);
35033513
if (it == NULL){
3504-
Py_DECREF(d);
3505-
return NULL;
3514+
goto Fail;
35063515
}
35073516

35083517
if (PyDict_CheckExact(d)) {
@@ -3541,12 +3550,19 @@ dict_iter_exit:;
35413550
if (PyErr_Occurred())
35423551
goto Fail;
35433552
Py_DECREF(it);
3544-
return d;
3553+
goto Done;
35453554

35463555
Fail:
3547-
Py_DECREF(it);
3556+
Py_XDECREF(it);
35483557
Py_DECREF(d);
35493558
return NULL;
3559+
3560+
Done:
3561+
// d can be NULL
3562+
if (d != NULL && !_PyObject_GC_IS_TRACKED(d)) {
3563+
_PyObject_GC_TRACK(d);
3564+
}
3565+
return d;
35503566
}
35513567

35523568
/* Methods */
@@ -4147,9 +4163,6 @@ dict_dict_merge(PyDictObject *mp, PyDictObject *other, int override, PyObject **
41474163
set_keys(mp, keys);
41484164
STORE_USED(mp, other->ma_used);
41494165
ASSERT_CONSISTENT(mp);
4150-
if (PyDict_Check(mp)) {
4151-
assert(_PyObject_GC_IS_TRACKED(mp));
4152-
}
41534166
return 0;
41544167
}
41554168
}
@@ -4316,7 +4329,12 @@ dict_merge_api(PyObject *a, PyObject *b, int override, PyObject **dupkey)
43164329
}
43174330
return -1;
43184331
}
4319-
return dict_merge(a, b, override, dupkey);
4332+
4333+
int res = dict_merge(a, b, override, dupkey);
4334+
if (PyDict_Check(a)) {
4335+
assert(_PyObject_GC_IS_TRACKED(a));
4336+
}
4337+
return res;
43204338
}
43214339

43224340
int
@@ -4475,10 +4493,16 @@ copy_lock_held(PyObject *o, int as_frozendict)
44754493
}
44764494
if (copy == NULL)
44774495
return NULL;
4478-
if (dict_merge(copy, o, 1, NULL) == 0)
4479-
return copy;
4480-
Py_DECREF(copy);
4481-
return NULL;
4496+
if (dict_merge(copy, o, 1, NULL) < 0) {
4497+
Py_DECREF(copy);
4498+
return NULL;
4499+
4500+
}
4501+
4502+
if (PyDict_Check(copy)) {
4503+
assert(_PyObject_GC_IS_TRACKED(copy));
4504+
}
4505+
return copy;
44824506
}
44834507

44844508
PyObject *

0 commit comments

Comments
 (0)