8000 Always lock object on writes to managed values · python/cpython@f896695 · GitHub
[go: up one dir, main page]
Skip to content

Commit f896695

Browse files
DinoVDinoV
committed
Always lock object on writes to managed values
1 parent c694324 commit f896695

File tree

8 files changed

+116
-97
lines changed

8 files changed

+116
-97
lines changed

‎Include/internal/pycore_dict.h

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ _PyDict_NotifyEvent(PyInterpreterState *interp,
245245
return DICT_NEXT_VERSION(interp) | (mp->ma_version_tag & DICT_WATCHER_AND_MODIFICATION_MASK);
246246
}
247247

248-
extern PyObject *_PyObject_MakeDictFromInstanceAttributes(PyObject *obj, PyDictValues *values);
248+
extern PyObject *_PyObject_MaterializeManagedDict(PyObject *obj);
249249
extern bool _PyObject_MakeInstanceAttributesFromDict(PyObject *obj, PyDictOrValues *dorv);
250250
extern PyObject *_PyDict_FromItems(
251251
PyObject *const *keys, Py_ssize_t keys_offset,
@@ -264,6 +264,37 @@ _PyDictValues_AddToInsertionOrder(PyDictValues *values, Py_ssize_t ix)
264264
*size_ptr = size;
265265
}
266266

267+
#ifdef Py_GIL_DISABLED
268+
269+
static inline bool
270+
_PyDictOrValues_TryStoreValue(PyObject *obj, Py_ssize_t index, PyObject *value)
271+
{
272+
bool success;
273+
Py_BEGIN_CRITICAL_SECTION(obj);
274+
275+
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
276+
if (!_PyDictOrValues_IsValues(*dorv_ptr)) {
277+
success = false;
278+
goto exit;
279+
}
280+
281+
PyDictValues *values = _PyDictOrValues_GetValues(*dorv_ptr);
282+
PyObject *old_value = _Py_atomic_load_ptr_relaxed(&values->values[index]);
283+
_Py_atomic_store_ptr_relaxed(&values->values[index], value);
284+
if (old_value == NULL) {
285+
_PyDictValues_AddToInsertionOrder(values, index);
286+
}
287+
else {
288+
Py_DECREF(old_value);
289+
}
290+
success = true;
291+
exit:
292+
Py_END_CRITICAL_SECTION();
293+
return success;
294+
}
295+
296+
#endif
297+
267298
#ifdef __cplusplus
268299
}
269300
#endif

‎Include/internal/pycore_object.h

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ extern "C" {
1414
#include "pycore_interp.h" // PyInterpreterState.gc
1515
#include "pycore_pystate.h" // _PyInterpreterState_GET()
1616
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION
17+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_PTR_RELAXED
1718

1819
/* Check if an object is consistent. For example, ensure that the reference
1920
counter is greater than or equal to 1, and ensure that ob_type is not NULL.
@@ -653,7 +654,7 @@ _PyObject_DictOrValuesPointer(PyObject *obj)
653654
static inline int
654655
_PyDictOrValues_IsValues(PyDictOrValues dor 9E88 v)
655656
{
656-
return ((uintptr_t)dorv.values) & 1;
657+
return ((uintptr_t)FT_ATOMIC_LOAD_PTR_RELAXED(dorv.values)) & 1;
657658
}
658659

659660
// Should only be used when we know the object isn't racing with other
@@ -662,7 +663,7 @@ static inline PyDictValues *
662663
_PyDictOrValues_GetValues(PyDictOrValues dorv)
663664
{
664665
assert(_PyDictOrValues_IsValues(dorv));
665-
return (PyDictValues *)(dorv.values + 1);
666+
return (PyDictValues *)(((char *)FT_ATOMIC_LOAD_PTR_RELAXED(dorv.values)) + 1);
666667
}
667668

668669
static inline PyDictValues *
@@ -698,31 +699,19 @@ static inline PyObject *
698699
_PyDictOrValues_GetDict(PyDictOrValues dorv)
699700
{
700701
assert(!_PyDictOrValues_IsValues(dorv));
701-
#ifdef Py_GIL_DISABLED
702-
return _Py_atomic_load_ptr_relaxed(&dorv.dict);
703-
#else
704-
return dorv.dict;
705-
#endif
702+
return FT_ATOMIC_LOAD_PTR_RELAXED(dorv.dict);
706703
}
707704

708705
static inline void
709706
_PyDictOrValues_SetValues(PyDictOrValues *ptr, PyDictValues *values)
710707
{
711-
#ifdef Py_GIL_DISABLED
712-
_Py_atomic_store_ptr_relaxed(&ptr->values, (char *)values - 1);
713-
#else
714-
ptr->values = ((char *)values) - 1;
715-
#endif
708+
FT_ATOMIC_STORE_PTR_RELAXED(ptr->values, (char *)values - 1);
716709
}
717710

718711
static inline void
719712
_PyDictOrValues_SetDict(PyDictOrValues *ptr, PyObject *dict)
720713
{
721-
#ifdef Py_GIL_DISABLED
722-
_Py_atomic_store_ptr_relaxed(&ptr->dict, dict);
723-
#else
724-
ptr->dict = dict;
725-
#endif
714+
FT_ATOMIC_STORE_PTR_RELAXED(ptr->dict, dict);
726715
}
727716

728717
extern PyObject ** _PyObject_ComputedDictPointer(PyObject *);

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ extern "C" {
2323
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
2424
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
2525
_Py_atomic_load_ssize_relaxed(&value)
26+
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) \
27+
_Py_atomic_load_ptr_relaxed(&value)
2628
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
2729
_Py_atomic_store_ptr_relaxed(&value, new_value)
2830
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
@@ -32,6 +34,7 @@ extern "C" {
3234
#else
3335
#define FT_ATOMIC_LOAD_SSIZE(value) value
3436
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
37+
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
3538
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
3639
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
3740
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value

‎Objects/dictobject.c

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6415,24 +6415,48 @@ make_dict_from_instance_attributes(PyInterpreterState *interp,
64156415
}
64166416

64176417
PyObject *
6418-
_PyObject_MakeDictFromInstanceAttributes(PyObject *obj, PyDictValues *values)
6418+
_PyObject_MaterializeManagedDict(PyObject *obj)
64196419
{
64206420
PyInterpreterState *interp = _PyInterpreterState_GET();
6421-
PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
6422-
OBJECT_STAT_INC(dict_materialized_on_request);
6423-
return make_dict_from_instance_attributes(interp, keys, values);
6424-
}
6421+
PyTypeObject *tp = Py_TYPE(obj);
6422+
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
6423+
PyObject *dict;
6424+
6425+
assert(_PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT));
6426+
6427+
Py_BEGIN_CRITICAL_SECTION(obj);
64256428

6426-
static bool
6427-
has_unique_reference(PyObject *op)
6428-
{
64296429
#ifdef Py_GIL_DISABLED
6430-
return (_Py_IsOwnedByCurrentThread(op) &&
6431-
op->ob_ref_local == 1 &&
6432-
_Py_atomic_load_ssize_relaxed(&op->ob_ref_shared) == 0);
6430+
// Check again that we didn't race with some other thread materializing dict
6431+
if (!_PyDictOrValues_IsValues(*dorv_ptr)) {
6432+
dict = _PyDictOrValues_GetDict(*dorv_ptr);
6433+
goto got_dict;
6434+
}
64336435
#else
6434-
return Py_REFCNT(op) == 1;
6436+
assert(_PyDictOrValues_IsValues(*dorv_ptr));
6437+
#endif
6438+
6439+
OBJECT_STAT_INC(dict_materialized_on_request);
6440+
dict = make_dict_from_instance_attrib F987 utes(
6441+
interp, CACHED_KEYS(tp), _PyDictOrValues_GetValues(*dorv_ptr));
6442+
6443+
if (dict != NULL) {
6444+
#ifdef Py_GIL_DISABLED
6445+
if (_PyObject_GC_IS_SHARED(obj)) {
6446+
// The values were accessed from multiple threads and need to be
6447+
// freed via QSBR, now the dict needs to be shared as it owns the
6448+
// values.
6449+
_PyObject_GC_SET_SHARED(dict);
6450+
}
64356451
#endif
6452+
_PyDictOrValues_SetDict(dorv_ptr, dict);
6453+
}
6454+
6455+
#ifdef Py_GIL_DISABLED
6456+
got_dict:
6457+
#endif
6458+
Py_END_CRITICAL_SECTION();
6459+
return dict;
64366460
}
64376461

64386462
// Return true if the dict was dematerialized, false otherwise.
@@ -6456,7 +6480,7 @@ _PyObject_MakeInstanceAttributesFromDict(PyObject *obj, PyDictOrValues *dorv)
64566480
}
64576481
assert(_PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_HEAPTYPE));
64586482
if (dict->ma_keys != CACHED_KEYS(Py_TYPE(obj)) ||
6459-
!has_unique_reference((PyObject *)dict))
6483+
Py_REFCNT(dict) != 1)
64606484
{
64616485
return false;
64626486
}
@@ -6693,24 +6717,8 @@ PyObject_GenericGetDict(PyObject *obj, void *context)
66936717
PyTypeObject *tp = Py_TYPE(obj);
66946718
if (_PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT)) {
66956719
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
6696-
PyDictValues *values;
6697-
if ((values = _PyDictOrValues_TryGetValues(obj)) != NULL) {
6698-
OBJECT_STAT_INC(dict_materialized_on_request);
6699-
Py_BEGIN_CRITICAL_SECTION(obj);
6700-
dict = make_dict_from_instance_attributes(
6701-
interp, CACHED_KEYS(tp), values);
6702-
if (dict != NULL) {
6703-
#ifdef Py_GIL_DISABLED
6704-
if (_PyObject_GC_IS_SHARED(obj)) {
6705-
// The values were accessed from multiple threads and need to be
6706-
// freed via QSBR, now the dict needs to be shared as it owns the
6707-
// values.
6708-
_PyObject_GC_SET_SHARED(dict);
6709-
}
6710-
#endif
6711-
_PyDictOrValues_SetDict(dorv_ptr, dict);
6712-
}
6713-
Py_END_CRITICAL_SECTION();
6720+
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
6721+
dict = _PyObject_MaterializeManagedDict(obj);
67146722
}
67156723
else {
67166724
dict = _PyDictOrValues_GetDict(*dorv_ptr);

‎Objects/object.c

Lines changed: 21 additions & 39 deletions
< 10000 /tr>
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include "pycore_ceval.h" // _Py_EnterRecursiveCallTstate()
88
#include "pycore_context.h" // _PyContextTokenMissing_Type
99
#include "pycore_descrobject.h" // _PyMethodWrapper_Type
10-
#include "pycore_dict.h" // _PyObject_MakeDictFromInstanceAttributes()
10+
#include "pycore_dict.h" // _PyObject_MaterializeManagedDict()
1111
#include "pycore_floatobject.h" // _PyFloat_DebugMallocStats()
1212
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION()
1313
#include "pycore_hashtable.h" // _Py_hashtable_new()
@@ -1398,34 +1398,9 @@ _PyObject_GetDictPtr(PyObject *obj)
13981398
return _PyObject_ComputedDictPointer(obj);
13991399
}
14001400
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
1401-
PyDictValues *values;
1402-
if ((values = _PyDictOrValues_TryGetValues(obj)) != NULL) {
1403-
PyObject *dict;
1404-
Py_BEGIN_CRITICAL_SECTION(obj);
1405-
#ifdef Py_GIL_DISABLED
1406-
if ((values = _PyDictOrValues_TryGetValues(obj)) == NULL) {
1407-
dict = _PyDictOrValues_GetDict(*dorv_ptr);
1408-
goto done;
1409-
}
1410-
#endif
1411-
1412-
dict = _PyObject_MakeDictFromInstanceAttributes(obj, values);
1413-
if (dict == NULL) {
1414-
goto done;
1415-
}
1416-
1417-
#ifdef Py_GIL_DISABLED
1418-
if (_PyObject_GC_IS_SHARED(obj)) {
1419-
// The values were accessed from multiple threads and need to be
1420-
// freed via QSBR, now the dict needs to be shared as it owns the
1421-
// values.
1422-
_PyObject_GC_SET_SHARED(dict);
1423-
}
1424-
#endif
1425-
dorv_ptr->dict = dict;
1401+
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
1402+
PyObject *dict = _PyObject_MaterializeManagedDict(obj);
14261403

1427-
done:
1428-
Py_END_CRITICAL_SECTION();
14291404
if (dict == NULL) {
14301405
PyErr_Clear();
14311406
return NULL;
@@ -1618,12 +1593,11 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name,
16181593
}
16191594
}
16201595
else {
1621-
dict = _PyObject_MakeDictFromInstanceAttributes(obj, values);
1596+
dict = _PyObject_MaterializeManagedDict(obj);
16221597
if (dict == NULL) {
16231598
res = NULL;
16241599
goto done;
16251600
}
1626-
dorv_ptr->dict = dict;
16271601
}
16281602
}
16291603
else {
@@ -1725,23 +1699,31 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
17251699
PyObject **dictptr;
17261700
if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {
17271701
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
1728-
PyDictValues *values;
1729-
if ((values = _PyDictOrValues_TryGetValues(obj)) != NULL) {
1702+
bool stored_inline = false;
1703+
1704+
Py_BEGIN_CRITICAL_SECTION(obj);
1705+
1706+
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
17301707
res = _PyObject_StoreInstanceAttribute(
1731-
obj, values, name, value);
1732-
goto error_check;
1708+
obj, _PyDictOrValues_GetValues(*dorv_ptr), name, value);
1709+
stored_inline = true;
1710+
goto managed_dict_done;
17331711
}
17341712
dictptr = &dorv_ptr->dict;
17351713
if (*dictptr == NULL) {
17361714
if (_PyObject_InitInlineValues(obj, tp) < 0) {
17371715
goto done;
17381716
}
1739-
if ((values = _PyDictOrValues_TryGetValues(obj)) != NULL) {
1740-
res = _PyObject_StoreInstanceAttribute(
1741-
obj, values, name, value);
1742-
goto error_check;
1743-
}
1717+
res = _PyObject_StoreInstanceAttribute(
1718+
obj, _PyDictOrValues_GetValues(*dorv_ptr), name, value);
1719+
stored_inline = true;
1720+
goto managed_dict_done;
17441721
}
1722+
managed_dict_done:
1723+
Py_END_CRITICAL_SECTION();
1724+
1725+
if (stored_inline)
1726+
goto error_check;
17451727
}
17461728
else {
17471729
dictptr = _PyObject_ComputedDictPointer(obj);

‎Python/bytecodes.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,10 +2094,11 @@ dummy_func(
20942094

20952095
op(_STORE_ATTR_INSTANCE_VALUE, (index/1, value, owner --)) {
20962096
STAT_INC(STORE_ATTR, hit);
2097+
#if Py_GIL_DISABLED
2098+
bool success = _PyDictOrValues_TryStoreValue(owner, index, value);
2099+
DEOPT_IF(!success);
2100+
#else
20972101
PyDictValues *values = _PyDictOrValues_TryGetValues(owner);
2098-
#ifdef Py_GIL_DISABLED
2099-
DEOPT_IF(values == NULL);
2100-
#endif
21012102
PyObject *old_value = values->values[index];
21022103
values->values[index] = value;
21032104
if (old_value == NULL) {
@@ -2106,6 +2107,7 @@ dummy_func(
21062107
else {
21072108
Py_DECREF(old_value);
21082109
}
2110+
#endif
21092111
Py_DECREF(owner);
21102112
}
21112113

‎Python/executor_cases.c.h

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
0