Skip to content

Commit 27d7a35

Browse files
authored
[mypyc] Fix dict iteration memory safety on free-threaded builds (#21617)
`PyDict_Next()` returns a borrowed reference, so we need to add critical sections around the calls to ensure memory safety. I used coding agent assist.
1 parent 0da2bbd commit 27d7a35

1 file changed

Lines changed: 38 additions & 10 deletions

File tree

mypyc/lib-rt/dict_ops.c

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,24 @@ tuple_T3CIO CPyDict_NextKey(PyObject *dict_or_iter, CPyTagged offset) {
346346
PyObject *dummy;
347347

348348
if (PyDict_CheckExact(dict_or_iter)) {
349-
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &ret.f2, &dummy);
349+
PyObject *key;
350+
// PyDict_Next() returns a borrowed reference. On free-threaded builds,
351+
// hold the dict lock until we have converted it to a strong reference.
352+
Py_BEGIN_CRITICAL_SECTION(dict_or_iter);
353+
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &key, &dummy);
354+
if (ret.f0) {
355+
ret.f2 = Py_NewRef(key);
356+
}
357+
Py_END_CRITICAL_SECTION();
358+
350359
if (ret.f0) {
351360
ret.f1 = CPyTagged_FromSsize_t(py_offset);
352361
} else {
353362
// Set key to None, so mypyc can manage refcounts.
354363
ret.f1 = 0;
355364
ret.f2 = Py_None;
365+
Py_INCREF(ret.f2);
356366
}
357-
// PyDict_Next() returns borrowed references.
358-
Py_INCREF(ret.f2);
359367
} else {
360368
// offset is dummy in this case, just use the old value.
361369
ret.f1 = offset;
@@ -370,16 +378,24 @@ tuple_T3CIO CPyDict_NextValue(PyObject *dict_or_iter, CPyTagged offset) {
370378
PyObject *dummy;
371379

372380
if (PyDict_CheckExact(dict_or_iter)) {
373-
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &dummy, &ret.f2);
381+
PyObject *value;
382+
// PyDict_Next() returns a borrowed reference. On free-threaded builds,
383+
// hold the dict lock until we have converted it to a strong reference.
384+
Py_BEGIN_CRITICAL_SECTION(dict_or_iter);
385+
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &dummy, &value);
386+
if (ret.f0) {
387+
ret.f2 = Py_NewRef(value);
388+
}
389+
Py_END_CRITICAL_SECTION();
390+
374391
if (ret.f0) {
375392
ret.f1 = CPyTagged_FromSsize_t(py_offset);
376393
} else {
377394
// Set value to None, so mypyc can manage refcounts.
378395
ret.f1 = 0;
379396
ret.f2 = Py_None;
397+
Py_INCREF(ret.f2);
380398
}
381-
// PyDict_Next() returns borrowed references.
382-
Py_INCREF(ret.f2);
383399
} else {
384400
// offset is dummy in this case, just use the old value.
385401
ret.f1 = offset;
@@ -393,14 +409,27 @@ tuple_T4CIOO CPyDict_NextItem(PyObject *dict_or_iter, CPyTagged offset) {
393409
Py_ssize_t py_offset = CPyTagged_AsSsize_t(offset);
394410

395411
if (PyDict_CheckExact(dict_or_iter)) {
396-
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &ret.f2, &ret.f3);
412+
PyObject *key;
413+
PyObject *value;
414+
// PyDict_Next() returns borrowed references. On free-threaded builds,
415+
// hold the dict lock until we have converted them to strong references.
416+
Py_BEGIN_CRITICAL_SECTION(dict_or_iter);
417+
ret.f0 = PyDict_Next(dict_or_iter, &py_offset, &key, &value);
418+
if (ret.f0) {
419+
ret.f2 = Py_NewRef(key);
420+
ret.f3 = Py_NewRef(value);
421+
}
422+
Py_END_CRITICAL_SECTION();
423+
397424
if (ret.f0) {
398425
ret.f1 = CPyTagged_FromSsize_t(py_offset);
399426
} else {
400427
// Set key and value to None, so mypyc can manage refcounts.
401428
ret.f1 = 0;
402429
ret.f2 = Py_None;
403430
ret.f3 = Py_None;
431+
Py_INCREF(ret.f2);
432+
Py_INCREF(ret.f3);
404433
}
405434
} else {
406435
ret.f1 = offset;
@@ -413,6 +442,8 @@ tuple_T4CIOO CPyDict_NextItem(PyObject *dict_or_iter, CPyTagged offset) {
413442
ret.f0 = 0;
414443
ret.f2 = Py_None;
415444
ret.f3 = Py_None;
445+
Py_INCREF(ret.f2);
446+
Py_INCREF(ret.f3);
416447
} else {
417448
ret.f0 = 1;
418449
ret.f2 = PyTuple_GET_ITEM(item, 0);
@@ -423,9 +454,6 @@ tuple_T4CIOO CPyDict_NextItem(PyObject *dict_or_iter, CPyTagged offset) {
423454
return ret;
424455
}
425456
}
426-
// PyDict_Next() returns borrowed references.
427-
Py_INCREF(ret.f2);
428-
Py_INCREF(ret.f3);
429457
return ret;
430458
}
431459

0 commit comments

Comments
 (0)