Skip to content

Commit

Permalink
Add thread safety around inline dict matieralization
Browse files Browse the repository at this point in the history
  • Loading branch information
@DinoV
DinoV committed Feb 29, 2024
1 parent 3058999 commit 99fe3f4
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 93 deletions.
2 changes: 1 addition & 1 deletion Include/internal/pycore_dict.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ _PyDict_NotifyEvent(PyInterpreterState *interp,
return DICT_NEXT_VERSION(interp) | (mp->ma_version_tag & DICT_WATCHER_AND_MODIFICATION_MASK);
}

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

extern PyObject *_PyDict_FromItems(
PyObject *const *keys, Py_ssize_t keys_offset,
Expand Down
15 changes: 15 additions & 0 deletions Include/internal/pycore_object.h
View file
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 @@ -645,6 +646,13 @@ _PyObject_DictOrValuesPointer(PyObject *obj)
return (PyDictOrValues *)((char *)obj + MANAGED_DICT_OFFSET);
}

static inline PyDictOrValues
_PyObject_DictOrValues(PyObject *obj)
{
PyDictOrValues *dorv = _PyObject_DictOrValuesPointer(obj);
return *(PyDictOrValues *)FT_ATOMIC_LOAD_PTR_RELAXED(dorv);
}

static inline PyDictValues *
_PyObject_InlineValues(PyObject *obj)
{
Expand All @@ -659,6 +667,13 @@ _PyDictOrValues_GetDict(PyDictOrValues dorv)
return dorv.dict;
}

static inline void
_PyDictOrValues_SetDict(PyObject *obj, PyObject *dict)
{
PyDictOrValues *ptr = _PyObject_DictOrValuesPointer(obj);
FT_ATOMIC_STORE_PTR_RELEASE(ptr->dict, dict);
}

extern int _PyObject_InlineValuesConsistencyCheck(PyObject *obj);

extern PyObject ** _PyObject_ComputedDictPointer(PyObject *);
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_opcode_metadata.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ extern "C" {
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
_Py_atomic_load_ssize_relaxed(&value)
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) \
_Py_atomic_load_ptr_relaxed(&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 @@ -32,6 +34,7 @@ extern "C" {
#else
#define FT_ATOMIC_LOAD_SSIZE(value) value
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) 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_SSIZE_RELAXED(value, new_value) value = new_value
Expand Down
4 changes: 2 additions & 2 deletions Include/internal/pycore_uop_metadata.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 46 additions & 27 deletions Objects/dictobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6625,20 +6625,36 @@ make_dict_from_instance_attributes(PyInterpreterState *interp,
}

PyObject *
_PyObject_MakeDictFromInstanceAttributes(PyObject *obj)
_PyObject_MaterializeManagedDict(PyObject *obj)
{
PyObject *dict;
Py_BEGIN_CRITICAL_SECTION(obj);

dict = _PyObject_DictOrValuesPointer(obj)->dict;
if (dict != NULL) {
// We raced with another thread creating the dict
goto exit;
}

OBJECT_STAT_INC(dict_materialized_on_request);

PyDictValues *values = _PyObject_InlineValues(obj);
PyInterpreterState *interp = _PyInterpreterState_GET();
PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
OBJECT_STAT_INC(dict_materialized_on_request);
return make_dict_from_instance_attributes(interp, keys, values);
dict = make_dict_from_instance_attributes(interp, keys, values);

_PyDictOrValues_SetDict(obj, dict);

exit:
Py_END_CRITICAL_SECTION();
return dict;
}

int
_PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name, PyObject *value)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
assert(keys != NULL);
assert(values != NULL);
Expand All @@ -6664,15 +6680,14 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
#endif
UNLOCK_KEYS(keys);
}

PyObject *dict = _PyObject_DictOrValuesPointer(obj)->dict;
if (ix == DKIX_EMPTY) {
if (dict == NULL) {
dict = make_dict_from_instance_attributes(
interp, keys, values);
dict = _PyObject_MaterializeManagedDict(obj);
if (dict == NULL) {
return -1;
}
_PyObject_DictOrValuesPointer(obj)->dict = dict;
}
if (value == NULL) {
return PyDict_DelItem(dict, name);
Expand All @@ -6682,7 +6697,7 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
}
}
PyObject *old_value = values->values[ix];
values->values[ix] = Py_XNewRef(value);
FT_ATOMIC_STORE_PTR_RELEASE(values->values[ix], Py_XNewRef(value));
if (old_value == NULL) {
if (value == NULL) {
PyErr_Format(PyExc_AttributeError,
Expand Down Expand Up @@ -6774,11 +6789,10 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj)
}
return 1;
}
dict = _PyObject_DictOrValuesPointer(obj)->dict;
dict = _PyObject_DictOrValues(obj).dict;
}
else if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
PyDictOrValues* dorv_ptr = _PyObject_DictOrValuesPointer(obj);
dict = dorv_ptr->dict;
dict = _PyObject_DictOrValues(obj).dict;
}
else {
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
Expand Down Expand Up @@ -6869,23 +6883,30 @@ PyObject_ClearManagedDict(PyObject *obj)
int
_PyDict_DetachFromObject(PyObject *dict, PyObject *obj)
{
int res = 0;
Py_BEGIN_CRITICAL_SECTION(dict);

assert(_PyObject_InlineValuesConsistencyCheck(obj));
PyDictObject *mp = (PyDictObject *)dict;
if (mp->ma_values == NULL || mp->ma_values != _PyObject_InlineValues(obj)) {
return 0;
goto exit;
}
assert(mp->ma_values->embedded);
assert(mp->ma_values->valid);
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_INLINE_VALUES);
mp->ma_values = copy_values(mp->ma_values);
if (mp->ma_values == NULL) {
return -1;
res = -1;
goto exit;
}
_PyObject_InlineValues(obj)->valid = 0;
assert(_PyObject_InlineValuesConsistencyCheck(obj));
ASSERT_CONSISTENT(dict);
return 0;

exit:
Py_END_CRITICAL_SECTION2();
return res;

}

PyObject *
Expand All @@ -6895,25 +6916,23 @@ PyObject_GenericGetDict(PyObject *obj, void *context)
PyInterpreterState *interp = _PyInterpreterState_GET();
PyTypeObject *tp = Py_TYPE(obj);
if (_PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT)) {
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
dict = _PyDictOrValues_GetDict(*dorv_ptr);
dict = _PyObject_DictOrValues(obj).dict;
if (dict == NULL && tp->tp_flags & Py_TPFLAGS_INLINE_VALUES && _PyObject_InlineValues(obj)->valid) {
PyDictValues *values = _PyObject_InlineValues(obj);
OBJECT_STAT_INC(dict_materialized_on_request);
dict = make_dict_from_instance_attributes(
interp, CACHED_KEYS(tp), values);
if (dict != NULL) {
dorv_ptr->dict = dict;
}
dict = _PyObject_MaterializeManagedDict(obj);
}
else {
dict = _PyDictOrValues_GetDict(*dorv_ptr);
else if (dict == NULL) {
Py_BEGIN_CRITICAL_SECTION(obj);

// Check again that we're not racing with someone else creating the dict
dict = _PyObject_DictOrValues(obj).dict;
if (dict == NULL) {
dictkeys_incref(CACHED_KEYS(tp));
OBJECT_STAT_INC(dict_materialized_on_request);
dictkeys_incref(CACHED_KEYS(tp));
dict = new_dict_with_shared_keys(interp, CACHED_KEYS(tp));
dorv_ptr->dict = dict;
_PyDictOrValues_SetDict(obj, dict);
}

Py_END_CRITICAL_SECTION();
}
}
else {
Expand Down Expand Up @@ -7121,7 +7140,7 @@ _PyObject_InlineValuesConsistencyCheck(PyObject *obj)
return 1;
}
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictObject *dict = (PyDictObject *)_PyObject_DictOrValuesPointer(obj)->dict;
PyDictObject *dict = (PyDictObject *)_PyObject_DictOrValues(obj).dict;
if (dict == NULL) {
return 1;
}
Expand Down
76 changes: 41 additions & 35 deletions Objects/object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,29 @@
/* Generic object operations; and implementation of None */

#include "Python.h"
#include "pycore_brc.h" // _Py_brc_queue_object()
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _Py_EnterRecursiveCallTstate()
#include "pycore_context.h" // _PyContextTokenMissing_Type
#include "pycore_descrobject.h" // _PyMethodWrapper_Type
#include "pycore_dict.h" // _PyObject_MakeDictFromInstanceAttributes()
#include "pycore_floatobject.h" // _PyFloat_DebugMallocStats()
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION()
#include "pycore_hashtable.h" // _Py_hashtable_new()
#include "pycore_memoryobject.h" // _PyManagedBuffer_Type
#include "pycore_namespace.h" // _PyNamespace_Type
#include "pycore_object.h" // PyAPI_DATA() _Py_SwappedOp definition
#include "pycore_optimizer.h" // _PyUOpExecutor_Type, _PyUOpOptimizer_Type, ...
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "pycore_pymem.h" // _PyMem_IsPtrFreed()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_symtable.h" // PySTEntry_Type
#include "pycore_typeobject.h" // _PyBufferWrapper_Type
#include "pycore_typevarobject.h" // _PyTypeAlias_Type, _Py_initialize_generic
#include "pycore_unionobject.h" // _PyUnion_Type

#include "interpreteridobject.h" // _PyInterpreterID_Type
#include "pycore_brc.h" // _Py_brc_queue_object()
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _Py_EnterRecursiveCallTstate()
#include "pycore_context.h" // _PyContextTokenMissing_Type
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION, Py_END_CRITICAL_SECTION
#include "pycore_descrobject.h" // _PyMethodWrapper_Type
#include "pycore_dict.h" // _PyObject_MaterializeManagedDict()
#include "pycore_floatobject.h" // _PyFloat_DebugMallocStats()
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION()
#include "pycore_hashtable.h" // _Py_hashtable_new()
#include "pycore_memoryobject.h" // _PyManagedBuffer_Type
#include "pycore_namespace.h" // _PyNamespace_Type
#include "pycore_object.h" // PyAPI_DATA() _Py_SwappedOp definition
#include "pycore_optimizer.h" // _PyUOpExecutor_Type, _PyUOpOptimizer_Type, ...
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "pycore_pymem.h" // _PyMem_IsPtrFreed()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_symtable.h" // PySTEntry_Type
#include "pycore_typeobject.h" // _PyBufferWrapper_Type
#include "pycore_typevarobject.h" // _PyTypeAlias_Type, _Py_initialize_generic
#include "pycore_unionobject.h" // _PyUnion_Type

#include "interpreteridobject.h" // _PyInterpreterID_Type

#ifdef Py_LIMITED_API
// Prevent recursive call _Py_IncRef() <=> Py_INCREF()
Expand Down Expand Up @@ -1397,16 +1398,15 @@ _PyObject_GetDictPtr(PyObject *obj)
if ((Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
return _PyObject_ComputedDictPointer(obj);
}
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
if (dorv_ptr->dict == NULL && Py_TYPE(obj)->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
PyObject *dict = _PyObject_MakeDictFromInstanceAttributes(obj);
PyDictOrValues dorv = _PyObject_DictOrValues(obj);
if (dorv.dict == NULL && Py_TYPE(obj)->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
PyObject *dict = _PyObject_MaterializeManagedDict(obj);
if (dict == NULL) {
PyErr_Clear();
return NULL;
}
dorv_ptr->dict = dict;
}
return &dorv_ptr->dict;
return &_PyObject_DictOrValuesPointer(obj)->dict;
}

PyObject *
Expand Down Expand Up @@ -1486,8 +1486,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
dict = NULL;
}
else if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {
PyDictOrValues* dorv_ptr = _PyObject_DictOrValuesPointer(obj);
dict = dorv_ptr->dict;
dict = _PyObject_DictOrValues(obj).dict;
}
else {
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
Expand Down Expand Up @@ -1589,17 +1588,15 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name,
}
}
else {
dict = _PyObject_MakeDictFromInstanceAttributes(obj);
dict = _PyObject_MaterializeManagedDict(obj);
if (dict == NULL) {
res = NULL;
goto done;
}
_PyObject_DictOrValuesPointer(obj)->dict = dict;
}
}
else if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {
PyDictOrValues* dorv_ptr = _PyObject_DictOrValuesPointer(obj);
dict = _PyDictOrValues_GetDict(*dorv_ptr);
dict = _PyObject_DictOrValues(obj).dict;
}
else {
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
Expand Down Expand Up @@ -1694,12 +1691,21 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,

if (dict == NULL) {
PyObject **dictptr;
bool stored_inline = false;

Py_BEGIN_CRITICAL_SECTION(obj);
if ((tp->tp_flags & Py_TPFLAGS_INLINE_VALUES) && _PyObject_InlineValues(obj)->valid) {
res = _PyObject_StoreInstanceAttribute(
obj, _PyObject_InlineValues(obj), name, value);
stored_inline = true;
}
Py_END_CRITICAL_SECTION();

if (stored_inline) {
goto error_check;
}
else if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {

if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
dictptr = &dorv_ptr->dict;
}
Expand Down Expand Up @@ -1773,7 +1779,7 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context)
PyObject **dictptr = _PyObject_GetDictPtr(obj);
if (dictptr == NULL) {
if (_PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_INLINE_VALUES) &&
_PyObject_DictOrValuesPointer(obj)->dict == NULL
_PyObject_DictOrValues(obj).dict == NULL
) {
/* Was unable to convert to dict */
PyErr_NoMemory();
Expand Down
5 changes: 2 additions & 3 deletions Objects/typeobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6085,13 +6085,12 @@ object_set_class(PyObject *self, PyObject *value, void *closure)
/* Changing the class will change the implicit dict keys,
* so we must materialize the dictionary first. */
if (oldto->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
PyObject *dict = _PyObject_DictOrValuesPointer(self)->dict;
PyObject *dict = _PyObject_DictOrValues(self).dict;
if (dict == NULL) {
dict = _PyObject_MakeDictFromInstanceAttributes(self);
dict = _PyObject_MaterializeManagedDict(self);
if (dict == NULL) {
return -1;
}
_PyObject_DictOrValuesPointer(self)->dict = dict;
}
if (_PyDict_DetachFromObject(dict, self)) {
return -1;
Expand Down
Loading

0 comments on commit 99fe3f4

Please sign in to comment.