Skip to content

Commit

Permalink
pythongh-113743: Make the MRO cache thread-safe in free-threaded buil…
Browse files Browse the repository at this point in the history
…ds (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.
  • Loading branch information
DinoV authored and pull[bot] committed Mar 4, 2024
1 parent fdd6a16 commit d03c748
Show file tree
Hide file tree
Showing 11 changed files with 500 additions and 80 deletions.
3 changes: 3 additions & 0 deletions Include/cpython/pyatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ _Py_atomic_store_int_release(int *obj, int value);
static inline int
_Py_atomic_load_int_acquire(const int *obj);

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj);


// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
3 changes: 3 additions & 0 deletions Include/cpython/pyatomic_gcc.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,9 @@ static inline int
_Py_atomic_load_int_acquire(const int *obj)
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }

// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
11 changes: 11 additions & 0 deletions Include/cpython/pyatomic_msc.h
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,17 @@ _Py_atomic_load_int_acquire(const int *obj)
#endif
}

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{
#if defined(_M_X64) || defined(_M_IX86)
return *(uint32_t volatile *)obj;
#elif defined(_M_ARM64)
return (int)__ldar32((uint32_t volatile *)obj);
#else
# error "no implementation of _Py_atomic_load_uint32_acquire"
#endif
}

// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
7 changes: 7 additions & 0 deletions Include/cpython/pyatomic_std.h
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,13 @@ _Py_atomic_load_int_acquire(const int *obj)
memory_order_acquire);
}

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{
_Py_USING_STD;
return atomic_load_explicit((const _Atomic(uint32_t)*)obj,
memory_order_acquire);
}


// --- _Py_atomic_fence ------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_critical_section.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ _PyCriticalSection_SuspendAll(PyThreadState *tstate);
#ifdef Py_GIL_DISABLED

static inline void
_PyCriticalSection_AssertHeld(PyMutex *mutex) {
_PyCriticalSection_AssertHeld(PyMutex *mutex)
{
#ifdef Py_DEBUG
PyThreadState *tstate = _PyThreadState_GET();
uintptr_t prev = tstate->critical_section;
Expand Down
33 changes: 33 additions & 0 deletions Include/internal/pycore_lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,39 @@ PyAPI_FUNC(void) _PyRWMutex_RUnlock(_PyRWMutex *rwmutex);
PyAPI_FUNC(void) _PyRWMutex_Lock(_PyRWMutex *rwmutex);
PyAPI_FUNC(void) _PyRWMutex_Unlock(_PyRWMutex *rwmutex);

// Similar to linux seqlock: https://en.wikipedia.org/wiki/Seqlock
// We use a sequence number to lock the writer, an even sequence means we're unlocked, an odd
// sequence means we're locked. Readers will read the sequence before attempting to read the
// underlying data and then read the sequence number again after reading the data. If the
// sequence has not changed the data is valid.
//
// Differs a little bit in that we use CAS on sequence as the lock, instead of a seperate spin lock.
// The writer can also detect that the undelering data has not changed and abandon the write
// and restore the previous sequence.
typedef struct {
uint32_t sequence;
} _PySeqLock;

// Lock the sequence lock for the writer
PyAPI_FUNC(void) _PySeqLock_LockWrite(_PySeqLock *seqlock);

// Unlock the sequence lock and move to the next sequence number.
PyAPI_FUNC(void) _PySeqLock_UnlockWrite(_PySeqLock *seqlock);

// Abandon the current update indicating that no mutations have occured
// and restore the previous sequence value.
PyAPI_FUNC(void) _PySeqLock_AbandonWrite(_PySeqLock *seqlock);

// Begin a read operation and return the current sequence number.
PyAPI_FUNC(uint32_t) _PySeqLock_BeginRead(_PySeqLock *seqlock);

// End the read operation and confirm that the sequence number has not changed.
// Returns 1 if the read was successful or 0 if the read should be re-tried.
PyAPI_FUNC(uint32_t) _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous);

// Check if the lock was held during a fork and clear the lock. Returns 1
// if the lock was held and any associated datat should be cleared.
PyAPI_FUNC(uint32_t) _PySeqLock_AfterFork(_PySeqLock *seqlock);

#ifdef __cplusplus
}
Expand Down
7 changes: 6 additions & 1 deletion Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern "C" {
#endif

#include "pycore_moduleobject.h" // PyModuleObject
#include "pycore_lock.h" // PyMutex


/* state */
Expand All @@ -21,13 +22,17 @@ struct _types_runtime_state {
// bpo-42745: next_version_tag remains shared by all interpreters
// because of static types.
unsigned int next_version_tag;
PyMutex type_mutex;
};


// Type attribute lookup cache: speed up attribute and method lookups,
// see _PyType_Lookup().
struct type_cache_entry {
unsigned int version; // initialized from type->tp_version_tag
#ifdef Py_GIL_DISABLED
_PySeqLock sequence;
#endif
PyObject *name; // reference to exactly a str or None
PyObject *value; // borrowed reference or NULL
};
Expand Down Expand Up @@ -74,7 +79,7 @@ struct types_state {
extern PyStatus _PyTypes_InitTypes(PyInterpreterState *);
extern void _PyTypes_FiniTypes(PyInterpreterState *);
extern void _PyTypes_Fini(PyInterpreterState *);

extern void _PyTypes_AfterFork(void);

/* other API */

Expand Down
15 changes: 4 additions & 11 deletions Modules/_abc.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "pycore_object.h" // _PyType_GetSubclasses()
#include "pycore_runtime.h" // _Py_ID()
#include "pycore_setobject.h" // _PySet_NextEntry()
#include "pycore_typeobject.h" // _PyType_GetMRO()
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
#include "clinic/_abc.c.h"

Expand Down Expand Up @@ -744,18 +743,12 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
Py_DECREF(ok);

/* 4. Check if it's a direct subclass. */
PyObject *mro = _PyType_GetMRO((PyTypeObject *)subclass);
assert(PyTuple_Check(mro));
for (pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) {
PyObject *mro_item = PyTuple_GET_ITEM(mro, pos);
assert(mro_item != NULL);
if ((PyObject *)self == mro_item) {
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) {
goto end;
}
result = Py_True;
if (PyType_IsSubtype((PyTypeObject *)subclass, (PyTypeObject *)self)) {
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) {
goto end;
}
result = Py_True;
goto end;
}

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

0 comments on commit d03c748

Please sign in to comment.