Skip to content

gh-113743: Make the MRO cache thread-safe in free-threaded builds #113930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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