Skip to content

Commit ae460d4

Browse files
DinoVDinoV
authored
pythongh-113743: Make the MRO cache thread-safe in free-threaded builds (python#113930)
Makes _PyType_Lookup thread safe, including: Thread safety of the underlying cache. Make mutation of mro and type members thread safe Also _PyType_GetMRO and _PyType_GetBases are currently returning borrowed references which aren't safe.
1 parent e74fa29 commit ae460d4

11 files changed

Lines changed: 500 additions & 80 deletions

File tree

‎Include/cpython/pyatomic.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,9 @@ _Py_atomic_store_int_release(int *obj, int value);
469469
static inline int
470470
_Py_atomic_load_int_acquire(const int *obj);
471471

472+
static inline uint32_t
473+
_Py_atomic_load_uint32_acquire(const uint32_t *obj);
474+
472475

473476
// --- _Py_atomic_fence ------------------------------------------------------
474477

‎Include/cpython/pyatomic_gcc.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,9 @@ static inline int
495495
_Py_atomic_load_int_acquire(const int *obj)
496496
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }
497497

498+
static inline uint32_t
499+
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
500+
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }
498501

499502
// --- _Py_atomic_fence ------------------------------------------------------
500503

‎Include/cpython/pyatomic_msc.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,17 @@ _Py_atomic_load_int_acquire(const int *obj)
938938
#endif
939939
}
940940

941+
static inline uint32_t
942+
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
943+
{
944+
#if defined(_M_X64) || defined(_M_IX86)
945+
return *(uint32_t volatile *)obj;
946+
#elif defined(_M_ARM64)
947+
return (int)__ldar32((uint32_t volatile *)obj);
948+
#else
949+
# error "no implementation of _Py_atomic_load_uint32_acquire"
950+
#endif
951+
}
941952

942953
// --- _Py_atomic_fence ------------------------------------------------------
943954

‎Include/cpython/pyatomic_std.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,13 @@ _Py_atomic_load_int_acquire(const int *obj)
870870
memory_order_acquire);
871871
}
872872

873+
static inline uint32_t
874+
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
875+
{
876+
_Py_USING_STD;
877+
return atomic_load_explicit((const _Atomic(uint32_t)*)obj,
878+
memory_order_acquire);
879+
}
873880

874881

875882
// --- _Py_atomic_fence ------------------------------------------------------

‎Include/internal/pycore_critical_section.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ _PyCriticalSection_SuspendAll(PyThreadState *tstate);
293293
#ifdef Py_GIL_DISABLED
294294

295295
static inline void
296-
_PyCriticalSection_AssertHeld(PyMutex *mutex) {
296+
_PyCriticalSection_AssertHeld(PyMutex *mutex)
297+
{
297298
#ifdef Py_DEBUG
298299
PyThreadState *tstate = _PyThreadState_GET();
299300
uintptr_t prev = tstate->critical_section;

‎Include/internal/pycore_lock.h‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,39 @@ PyAPI_FUNC(void) _PyRWMutex_RUnlock(_PyRWMutex *rwmutex);
251251
PyAPI_FUNC(void) _PyRWMutex_Lock(_PyRWMutex *rwmutex);
252252
PyAPI_FUNC(void) _PyRWMutex_Unlock(_PyRWMutex *rwmutex);
253253

254+
// Similar to linux seqlock: https://en.wikipedia.org/wiki/Seqlock
255+
// We use a sequence number to lock the writer, an even sequence means we're unlocked, an odd
256+
// sequence means we're locked. Readers will read the sequence before attempting to read the
257+
// underlying data and then read the sequence number again after reading the data. If the
258+
// sequence has not changed the data is valid.
259+
//
260+
// Differs a little bit in that we use CAS on sequence as the lock, instead of a seperate spin lock.
261+
// The writer can also detect that the undelering data has not changed and abandon the write
262+
// and restore the previous sequence.
263+
typedef struct {
264+
uint32_t sequence;
265+
} _PySeqLock;
266+
267+
// Lock the sequence lock for the writer
268+
PyAPI_FUNC(void) _PySeqLock_LockWrite(_PySeqLock *seqlock);
269+
270+
// Unlock the sequence lock and move to the next sequence number.
271+
PyAPI_FUNC(void) _PySeqLock_UnlockWrite(_PySeqLock *seqlock);
272+
273+
// Abandon the current update indicating that no mutations have occured
274+
// and restore the previous sequence value.
275+
PyAPI_FUNC(void) _PySeqLock_AbandonWrite(_PySeqLock *seqlock);
276+
277+
// Begin a read operation and return the current sequence number.
278+
PyAPI_FUNC(uint32_t) _PySeqLock_BeginRead(_PySeqLock *seqlock);
279+
280+
// End the read operation and confirm that the sequence number has not changed.
281+
// Returns 1 if the read was successful or 0 if the read should be re-tried.
282+
PyAPI_FUNC(uint32_t) _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous);
283+
284+
// Check if the lock was held during a fork and clear the lock. Returns 1
285+
// if the lock was held and any associated datat should be cleared.
286+
PyAPI_FUNC(uint32_t) _PySeqLock_AfterFork(_PySeqLock *seqlock);
254287

255288
#ifdef __cplusplus
256289
}

‎Include/internal/pycore_typeobject.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ extern "C" {
99
#endif
1010

1111
#include "pycore_moduleobject.h" // PyModuleObject
12+
#include "pycore_lock.h" // PyMutex
1213

1314

1415
/* state */
@@ -21,13 +22,17 @@ struct _types_runtime_state {
2122
// bpo-42745: next_version_tag remains shared by all interpreters
2223
// because of static types.
2324
unsigned int next_version_tag;
25+
PyMutex type_mutex;
2426
};
2527

2628

2729
// Type attribute lookup cache: speed up attribute and method lookups,
2830
// see _PyType_Lookup().
2931
struct type_cache_entry {
3032
unsigned int version; // initialized from type->tp_version_tag
33+
#ifdef Py_GIL_DISABLED
34+
_PySeqLock sequence;
35+
#endif
3136
PyObject *name; // reference to exactly a str or None
3237
PyObject *value; // borrowed reference or NULL
3338
};
@@ -74,7 +79,7 @@ struct types_state {
7479
extern PyStatus _PyTypes_InitTypes(PyInterpreterState *);
7580
extern void _PyTypes_FiniTypes(PyInterpreterState *);
7681
extern void _PyTypes_Fini(PyInterpreterState *);
77-
82+
extern void _PyTypes_AfterFork(void);
7883

7984
/* other API */
8085

‎Modules/_abc.c‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "pycore_object.h" // _PyType_GetSubclasses()
99
#include "pycore_runtime.h" // _Py_ID()
1010
#include "pycore_setobject.h" // _PySet_NextEntry()
11-
#include "pycore_typeobject.h" // _PyType_GetMRO()
1211
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
1312
#include "clinic/_abc.c.h"
1413

@@ -744,18 +743,12 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
744743
Py_DECREF(ok);
745744

746745
/* 4. Check if it's a direct subclass. */
747-
PyObject *mro = _PyType_GetMRO((PyTypeObject *)subclass);
748-
assert(PyTuple_Check(mro));
749-
for (pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) {
750-
PyObject *mro_item = PyTuple_GET_ITEM(mro, pos);
751-
assert(mro_item != NULL);
752-
if ((PyObject *)self == mro_item) {
753-
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) {
754-
goto end;
755-
}
756-
result = Py_True;
746+
if (PyType_IsSubtype((PyTypeObject *)subclass, (PyTypeObject *)self)) {
747+
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) {
757748
goto end;
758749
}
750+
result = Py_True;
751+
goto end;
759752
}
760753

761754
/* 5. Check if it's a subclass of a registered class (recursive). */

0 commit comments

Comments
 (0)