Skip to content

Commit

Permalink
gh-112075: Make instance attributes stored in inline "dict" thread sa…
Browse files Browse the repository at this point in the history
…fe (#114742)

Make instance attributes stored in inline "dict" thread safe on free-threaded builds
  • Loading branch information
DinoV authored Apr 22, 2024
1 parent 1446024 commit 8b541c0
Show file tree
Hide file tree
Showing 13 changed files with 419 additions and 142 deletions.
1 change: 1 addition & 0 deletions Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ do { \
PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj);

PyAPI_FUNC(int) PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg);
PyAPI_FUNC(void) _PyObject_SetManagedDict(PyObject *obj, PyObject *new_dict);
PyAPI_FUNC(void) PyObject_ClearManagedDict(PyObject *obj);

#define TYPE_MAX_WATCHERS 8
Expand Down
19 changes: 8 additions & 11 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

#ifndef Py_INTERNAL_DICT_H
#define Py_INTERNAL_DICT_H
#ifdef __cplusplus
Expand All @@ -9,9 +8,10 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_identifier.h" // _Py_Identifier
#include "pycore_object.h" // PyManagedDictPointer
#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_identifier.h" // _Py_Identifier
#include "pycore_object.h" // PyManagedDictPointer
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SSIZE_ACQUIRE

// Unsafe flavor of PyDict_GetItemWithError(): no error checking
extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key);
Expand Down Expand Up @@ -249,7 +249,7 @@ _PyDict_NotifyEvent(PyInterpreterState *interp,
return DICT_NEXT_VERSION(interp) | (mp->ma_version_tag & DICT_WATCHER_AND_MODIFICATION_MASK);
}

extern PyDictObject *_PyObject_MakeDictFromInstanceAttributes(PyObject *obj);
extern PyDictObject *_PyObject_MaterializeManagedDict(PyObject *obj);

PyAPI_FUNC(PyObject *)_PyDict_FromItems(
PyObject *const *keys, Py_ssize_t keys_offset,
Expand Down Expand Up @@ -277,19 +277,16 @@ _PyDictValues_AddToInsertionOrder(PyDictValues *values, Py_ssize_t ix)
static inline size_t
shared_keys_usable_size(PyDictKeysObject *keys)
{
#ifdef Py_GIL_DISABLED
// dk_usable will decrease for each instance that is created and each
// value that is added. dk_nentries will increase for each value that
// is added. We want to always return the right value or larger.
// We therefore increase dk_nentries first and we decrease dk_usable
// second, and conversely here we read dk_usable first and dk_entries
// second (to avoid the case where we read entries before the increment
// and read usable after the decrement)
return (size_t)(_Py_atomic_load_ssize_acquire(&keys->dk_usable) +
_Py_atomic_load_ssize_acquire(&keys->dk_nentries));
#else
return (size_t)keys->dk_nentries + (size_t)keys->dk_usable;
#endif
Py_ssize_t dk_usable = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(keys->dk_usable);
Py_ssize_t dk_nentries = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(keys->dk_nentries);
return dk_nentries + dk_usable;
}

static inline size_t
Expand Down
16 changes: 12 additions & 4 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ extern "C" {
#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED()
#include "pycore_emscripten_trampoline.h" // _PyCFunction_TrampolineCall()
#include "pycore_interp.h" // PyInterpreterState.gc
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_PTR_RELAXED
#include "pycore_pystate.h" // _PyInterpreterState_GET()

/* Check if an object is consistent. For example, ensure that the reference
Expand Down Expand Up @@ -659,10 +660,10 @@ extern PyObject* _PyType_GetDocFromInternalDoc(const char *, const char *);
extern PyObject* _PyType_GetTextSignatureFromInternalDoc(const char *, const char *, int);

void _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp);
extern int _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name, PyObject *value);
PyObject * _PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name);
extern int _PyObject_StoreInstanceAttribute(PyObject *obj,
PyObject *name, PyObject *value);
extern bool _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name,
PyObject **attr);

#ifdef Py_GIL_DISABLED
# define MANAGED_DICT_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-1)
Expand All @@ -683,6 +684,13 @@ _PyObject_ManagedDictPointer(PyObject *obj)
return (PyManagedDictPointer *)((char *)obj + MANAGED_DICT_OFFSET);
}

static inline PyDictObject *
_PyObject_GetManagedDict(PyObject *obj)
{
PyManagedDictPointer *dorv = _PyObject_ManagedDictPointer(obj);
return (PyDictObject *)FT_ATOMIC_LOAD_PTR_RELAXED(dorv->dict);
}

static inline PyDictValues *
_PyObject_InlineValues(PyObject *obj)
{
Expand Down
14 changes: 14 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ extern "C" {

#ifdef Py_GIL_DISABLED
#define FT_ATOMIC_LOAD_PTR(value) _Py_atomic_load_ptr(&value)
#define FT_ATOMIC_STORE_PTR(value, new_value) _Py_atomic_store_ptr(&value, new_value)
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
#define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) \
_Py_atomic_load_ssize_acquire(&value)
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
_Py_atomic_load_ssize_relaxed(&value)
#define FT_ATOMIC_STORE_PTR(value, new_value) \
Expand All @@ -30,6 +33,12 @@ extern "C" {
_Py_atomic_load_ptr_acquire(&value)
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) \
_Py_atomic_load_uintptr_acquire(&value)
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) \
_Py_atomic_load_ptr_relaxed(&value)
#define FT_ATOMIC_LOAD_UINT8(value) \
_Py_atomic_load_uint8(&value)
#define FT_ATOMIC_STORE_UINT8(value, new_value) \
_Py_atomic_store_uint8(&value, new_value)
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
_Py_atomic_store_ptr_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
Expand All @@ -43,11 +52,16 @@ extern "C" {

#else
#define FT_ATOMIC_LOAD_PTR(value) value
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
#define FT_ATOMIC_LOAD_SSIZE(value) value
#define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT8(value) value
#define FT_ATOMIC_STORE_UINT8(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value
Expand Down
9 changes: 9 additions & 0 deletions Lib/test/test_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,15 @@ def __init__(self):
obj.foo = None # Aborted here
self.assertEqual(obj.__dict__, {"foo":None})

def test_store_attr_deleted_dict(self):
class Foo:
pass

f = Foo()
del f.__dict__
f.a = 3
self.assertEqual(f.a, 3)


if __name__ == '__main__':
unittest.main()
Loading

0 comments on commit 8b541c0

Please sign in to comment.