10000 gh-112075: Add critical sections for most dict APIs (#114508) · python/cpython@92abb01 · GitHub
[go: up one dir, main page]
Skip to content

Commit 92abb01

Browse files
DinoVDinoV
authored
gh-112075: Add critical sections for most dict APIs (#114508)
Starts adding thread safety to dict objects. Use @critical_section for APIs which are exposed via argument clinic and don't directly correlate with a public C API which needs to acquire the lock Use a _lock_held suffix for keeping changes to complicated functions simple and just wrapping them with a critical section Acquire and release the lock in an existing function where it won't be overly disruptive to the existing logic
1 parent b6228b5 commit 92abb01

File tree

6 files changed

+782
-284
lines changed

6 files changed

+782
-284
lines changed

‎Include/internal/pycore_critical_section.h

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,37 @@ extern "C" {
104104
# define Py_END_CRITICAL_SECTION2() \
105105
_PyCriticalSection2_End(&_cs2); \
106106
}
107+
108+
// Asserts that the mutex is locked. The mutex must be held by the
109+
// top-most critical section otherwise there's the possibility
110+
// that the mutex would be swalled out in some code paths.
111+
#define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex) \
112+
_PyCriticalSection_AssertHeld(mutex)
113+
114+
// Asserts that the mutex for the given object is locked. The mutex must
115+
// be held by the top-most critical section otherwise there's the
116+
// possibility that the mutex would be swalled out in some code paths.
117+
#ifdef Py_DEBUG
118+
119+
#define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op) \
120+
if (Py_REFCNT(op) != 1) { \
121+
_Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&_PyObject_CAST(op)->ob_mutex); \
122+
}
123+
124+
#else /* Py_DEBUG */
125+
126+
#define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
127+
128+
#endif /* Py_DEBUG */
129+
107130
#else /* !Py_GIL_DISABLED */
108131
// The critical section APIs are no-ops with the GIL.
109132
# define Py_BEGIN_CRITICAL_SECTION(op)
110133
# define Py_END_CRITICAL_SECTION()
111134
# define Py_BEGIN_CRITICAL_SECTION2(a, b)
112135
# define Py_END_CRITICAL_SECTION2()
136+
# define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex)
137+
# define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
113138
#endif /* !Py_GIL_DISABLED */
114139

115140
typedef struct {
@@ -236,6 +261,27 @@ _PyCriticalSection2_End(_PyCriticalSection2 *c)
236261
PyAPI_FUNC(void)
237262
_PyCriticalSection_SuspendAll(PyThreadState *tstate);
238263

264+
#ifdef Py_GIL_DISABLED
265+
266+
static inline void
267+
_PyCriticalSection_AssertHeld(PyMutex *mutex) {
268+
#ifdef Py_DEBUG
269+
PyThreadState *tstate = _PyThreadState_GET();
270+
uintptr_t prev = tstate->critical_section;
271+
if (prev & _Py_CRITICAL_SECTION_TWO_MUTEXES) {
272+
_PyCriticalSection2 *cs = (_PyCriticalSection2 *)(prev & ~_Py_CRITICAL_SECTION_MASK);
273+
assert(cs != NULL && (cs->base.mutex == mutex || cs->mutex2 == mutex));
274+
}
275+
else {
276+
_PyCriticalSection *cs = (_PyCriticalSection *)(tstate->critical_section & ~_Py_CRITICAL_SECTION_MASK);
277+
assert(cs != NULL && cs->mutex == mutex);
278+
}
279+
280+
#endif
281+
}
282+
283+
#endif
284+
239285
#ifdef __cplusplus
240286
}
241287
#endif

‎Modules/_sre/sre.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ static const char copyright[] =
3939
" SRE 2.2.2 Copyright (c) 1997-2002 by Secret Labs AB ";
4040

4141
#include "Python.h"
42-
#include "pycore_dict.h" // _PyDict_Next()
43-
#include "pycore_long.h" // _PyLong_GetZero()
44-
#include "pycore_moduleobject.h" // _PyModule_GetState()
42+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION
43+
#include "pycore_dict.h" // _PyDict_Next()
44+
#include "pycore_long.h" // _PyLong_GetZero()
45+
#include "pycore_moduleobject.h" // _PyModule_GetState()
4546

46-
#include "sre.h" // SRE_CODE
47+
#include "sre.h" // SRE_CODE
4748

48-
#include <ctype.h> // tolower(), toupper(), isalnum()
49+
#include <ctype.h> // tolower(), toupper(), isalnum()
4950

5051
#define SRE_CODE_BITS (8 * sizeof(SRE_CODE))
5152

@@ -2349,26 +2350,28 @@ _sre_SRE_Match_groupdict_impl(MatchObject *self, PyObject *default_value)
23492350
if (!result || !self->pattern->groupindex)
23502351
return result;
23512352

2353+
Py_BEGIN_CRITICAL_SECTION(self->pattern->groupindex);
23522354
while (_PyDict_Next(self->pattern->groupindex, &pos, &key, &value, &hash)) {
23532355
int status;
23542356
Py_INCREF(key);
23552357
value = match_getslice(self, key, default_value);
23562358
if (!value) {
23572359
Py_DECREF(key);
2358-
goto failed;
2360+
Py_CLEAR(result);
2361+
goto exit;
23592362
}
23602363
status = _PyDict_SetItem_KnownHash(result, key, value, hash);
23612364
Py_DECREF(value);
23622365
Py_DECREF(key);
2363-
if (status < 0)
2364-
goto failed;
2366+
if (status < 0) {
2367+
Py_CLEAR(result);
2368+
goto exit;
2369+
}
23652370
}
2371+
exit:
2372+
Py_END_CRITICAL_SECTION();
23662373

23672374
return result;
2368-
2369-
failed:
2370-
Py_DECREF(result);
2371-
return NULL;
23722375
}
23732376

23742377
/*[clinic input]

‎Objects/clinic/dictobject.c.h

Lines changed: 28 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
0