Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

gh-120858: PyDict_Next should not lock the dict (#120859) 

PyDict_Next no longer locks the dictionary in the free-threaded build. Locking
around individual PyDict_Next calls is not sufficient because the function
returns borrowed references and because it allows concurrent modifications
during the iteraiton loop.

The internal locking also interferes with correct external synchronization
because it may suspend outer critical sections created by the caller.
This commit is contained in:
Sam Gross 2024-06-24 14:15:15 -04:00 committed by GitHub
parent dee63cb359
commit 375b723d58
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 34 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
Doc/c-api/dict.rst
View File

@ -290,6 +290,17 @@ Dictionary Objects
Py_DECREF(o);
}
The function is not thread-safe in the :term:`free-threaded <free threading>`
build without external synchronization. You can use
:c:macro:`Py_BEGIN_CRITICAL_SECTION` to lock the dictionary while iterating
over it::
Py_BEGIN_CRITICAL_SECTION(self->dict);
while (PyDict_Next(self->dict, &pos, &key, &value)) {
...
}
Py_END_CRITICAL_SECTION();
.. c:function:: int PyDict_Merge(PyObject *a, PyObject *b, int override)

20
Doc/howto/free-threading-extensions.rst
View File

@ -114,6 +114,24 @@ Containers like :c:struct:`PyListObject`,
in the free-threaded build. For example, the :c:func:`PyList_Append` will
lock the list before appending an item.
.. _PyDict_Next:
``PyDict_Next``
'''''''''''''''
A notable exception is :c:func:`PyDict_Next`, which does not lock the
dictionary. You should use :c:macro:`Py_BEGIN_CRITICAL_SECTION` to protect
the dictionary while iterating over it if the dictionary may be concurrently
modified::
Py_BEGIN_CRITICAL_SECTION(dict);
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(dict, &pos, &key, &value)) {
...
}
Py_END_CRITICAL_SECTION();
Borrowed References
===================
@ -141,7 +159,7 @@ that return :term:`strong references <strong reference>`.
+-----------------------------------+-----------------------------------+
| :c:func:`PyDict_SetDefault` | :c:func:`PyDict_SetDefaultRef` |
+-----------------------------------+-----------------------------------+
| :c:func:`PyDict_Next` | no direct replacement |
| :c:func:`PyDict_Next` | none (see :ref:`PyDict_Next`) |
+-----------------------------------+-----------------------------------+
| :c:func:`PyWeakref_GetObject` | :c:func:`PyWeakref_GetRef` |
+-----------------------------------+-----------------------------------+

3
Misc/NEWS.d/next/C API/2024-06-21-16-41-21.gh-issue-120858.Z5_-Mn.rst Normal file
View File

@ -0,0 +1,3 @@
:c:func:`PyDict_Next` no longer locks the dictionary in the free-threaded
build. The locking needs to be done by the caller around the entire iteration
loop.

8
Objects/dictobject.c
View File

@ -2808,8 +2808,6 @@ _PyDict_Next(PyObject *op, Py_ssize_t *ppos, PyObject **pkey,
if (!PyDict_Check(op))
return 0;
ASSERT_DICT_LOCKED(op);
mp = (PyDictObject *)op;
i = *ppos;
if (_PyDict_HasSplitTable(mp)) {
@ -2882,11 +2880,7 @@ _PyDict_Next(PyObject *op, Py_ssize_t *ppos, PyObject **pkey,
int
PyDict_Next(PyObject *op, Py_ssize_t *ppos, PyObject **pkey, PyObject **pvalue)
{
int res;
Py_BEGIN_CRITICAL_SECTION(op);
res = _PyDict_Next(op, ppos, pkey, pvalue, NULL);
Py_END_CRITICAL_SECTION();
return res;
return _PyDict_Next(op, ppos, pkey, pvalue, NULL);
}