8000 _PyObject_TryStoreInstanceAttribute -> _PyObject_StoreInstanceAttribute · python/cpython@6d37f80 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6d37f80

Browse files
DinoVDinoV
committed
_PyObject_TryStoreInstanceAttribute -> _PyObject_StoreInstanceAttribute
Fix issue where critical section isn't released Make _PyObject_TryGetInstanceAttribute return a bool
1 parent 68fa243 commit 6d37f80

File tree

5 files changed

+41
-41
lines changed

5 files changed

+41
-41
lines changed

‎Include/internal/pycore_object.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,10 @@ extern PyObject* _PyType_GetDocFromInternalDoc(const char *, const char *);
660660
extern PyObject* _PyType_GetTextSignatureFromInternalDoc(const char *, const char *, int);
661661

662662
void _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp);
663-
extern int _PyObject_TryStoreInstanceAttribute(PyObject *obj,
663+
extern int _PyObject_StoreInstanceAttribute(PyObject *obj,
664664
PyObject *name, PyObject *value);
665-
extern int _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name,
666-
PyObject **attr);
665+
extern bool _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name,
666+
PyObject **attr);
667667

668668
#ifdef Py_GIL_DISABLED
669669
# define MANAGED_DICT_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-1)

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ extern "C" {
3232
_Py_atomic_load_ptr_relaxed(&value)
3333
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) \
3434
_Py_atomic_load_uint8_relaxed(&value)
35+
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \
36+
_Py_atomic_store_uint8_relaxed(&value, new_value)
3537
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
3638
_Py_atomic_store_ptr_relaxed(&value, new_value)
3739
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
@@ -46,6 +48,7 @@ extern "C" {
4648
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
4749
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
4850
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
51+
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
4952
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
5053
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
5154
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value

‎Objects/dictobject.c

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6629,7 +6629,7 @@ materialize_managed_dict_lock_held(PyObject *obj)
66296629
OBJECT_STAT_INC(dict_materialized_on_request);
66306630
PyDictObject *dict = make_dict_from_instance_attributes(interp, keys, values);
66316631
FT_ATOMIC_STORE_PTR_RELEASE(_PyObject_ManagedDictPointer(obj)->dict,
6632-
(PyDictObject *)dict);
6632+
(PyDictObject *)dict);
66336633
return dict;
66346634
}
66356635

@@ -6713,8 +6713,6 @@ store_instance_attr_lock_held(PyObject *obj, PyDictValues *values,
67136713
if (ix == DKIX_EMPTY) {
67146714
int res;
67156715
if (dict == NULL) {
6716-
// Make the dict but don't publish it in the object
6717-
// so that no one else will see it.
67186716
dict = materialize_managed_dict_lock_held(obj);
67196717
if (dict == NULL) {
67206718
return -1;
@@ -6795,15 +6793,14 @@ store_instance_attr_dict(PyObject *obj, PyDictObject *dict, PyObject *name, PyOb
67956793
}
67966794

67976795
int
6798-
_PyObject_TryStoreInstanceAttribute(PyObject *obj, PyObject *name, PyObject *value)
6796+
_PyObject_StoreInstanceAttribute(PyObject *obj, PyObject *name, PyObject *value)
67996797
{
68006798
PyDictValues *values = _PyObject_InlineValues(obj);
68016799
if (!FT_ATOMIC_LOAD_UINT8_RELAXED(values->valid)) {
68026800
return store_instance_attr_dict(obj, _PyObject_GetManagedDict(obj), name, value);
68036801
}
68046802

68056803
#ifdef Py_GIL_DISABLED
6806-
int res;
68076804
// We have a valid inline values, at least for now... There are two potential
68086805
// races with having the values become invalid. One is the dictionary
68096806
// being detached from the object. The other is if someone is inserting
@@ -6816,24 +6813,20 @@ _PyObject_TryStoreInstanceAttribute(PyObject *obj, PyObject *name, PyObject *val
68166813
// prevent resizing.
68176814
PyDictObject *dict = _PyObject_GetManagedDict(obj);
68186815
if (dict == NULL) {
6816+
int res;
68196817
Py_BEGIN_CRITICAL_SECTION(obj);
68206818
dict = _PyObject_GetManagedDict(obj);
68216819

68226820
if (dict == NULL) {
68236821
res = store_instance_attr_lock_held(obj, values, name, value);
68246822
}
6825-
else {
6826-
// We lost a race with the materialization of the dict, we'll
6827-
// try the insert with it...
6828-
goto with_dict;
6829-
}
68306823
Py_END_CRITICAL_SECTION();
6824+
6825+
if (dict == NULL) {
6826+
return res;
6827+
}
68316828
}
6832-
else {
6833-
with_dict:
6834-
res = store_instance_attr_dict(obj, dict, name, value);
6835-
}
6836-
return res;
6829+
return store_instance_attr_dict(obj, dict, name, value);
68376830
#else
68386831
return store_instance_attr_lock_held(obj, values, name, value);
68396832
#endif
@@ -6873,28 +6866,28 @@ _PyObject_ManagedDictValidityCheck(PyObject *obj)
68736866
// Attempts to get an instance attribute from the inline values. Returns 0 if
68746867
// the lookup from the inline values was successful or 1 if the inline values
68756868
// are no longer valid. No error is set in either case.
6876-
int
6869+
bool
68776870
_PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name, PyObject **attr)
68786871
{
68796872
assert(PyUnicode_CheckExact(name));
68806873
PyDictValues *values = _PyObject_InlineValues(obj);
68816874
if (!FT_ATOMIC_LOAD_UINT8_RELAXED(values->valid)) {
6882-
return 1;
6875+
return false;
68836876
}
68846877

68856878
PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
68866879
assert(keys != NULL);
68876880
Py_ssize_t ix = _PyDictKeys_StringLookup(keys, name);
68886881
if (ix == DKIX_EMPTY) {
68896882
*attr = NULL;
6890-
return 0;
6883+
return true;
68916884
}
68926885

68936886
#ifdef Py_GIL_DISABLED
68946887
PyObject *value = _Py_atomic_load_ptr_relaxed(&values->values[ix]);
68956888
if (value == NULL || _Py_TryIncrefCompare(&values->values[ix], value)) {
68966889
*attr = value;
6897-
return 0;
6890+
return true;
68986891
}
68996892

69006893
PyDictObject *dict = _PyObject_GetManagedDict(obj);
@@ -6916,33 +6909,34 @@ _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name, PyObject **attr
69166909
Py_END_CRITICAL_SECTION();
69176910

69186911
if (success) {
6919-
return 0;
6912+
return true;
69206913
}
69216914
}
69226915

69236916
// We have a dictionary, we'll need to lock it to prevent
69246917
// the values from being resized.
69256918
assert(dict != NULL);
6926-
int res;
6919+
6920+
bool success;
69276921
Py_BEGIN_CRITICAL_SECTION(dict);
69286922

69296923
if (dict->ma_values == values &&
69306924
FT_ATOMIC_LOAD_UINT8_RELAXED(values->valid)) {
69316925
value = _Py_atomic_load_ptr_relaxed(&values->values[ix]);
69326926
*attr = Py_XNewRef(value);
6933-
res = 0;
6927+
success = true;
69346928
} else {
69356929
// Caller needs to lookup from the dictionary
6936-
res = 1;
6930+
success = false;
< A851 /td>
69376931
}
69386932

69396933
Py_END_CRITICAL_SECTION();
69406934

6941-
return res;
6935+
return success;
69426936
#else
69436937
PyObject *value = values->values[ix];
69446938
*attr = Py_XNewRef(value);
6945-
return 0;
6939+
return true;
69466940
#endif
69476941
}
69486942

@@ -7003,14 +6997,9 @@ PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg)
70036997
static bool
70046998
set_dict_inline_values(PyObject *obj, PyObject *new_dict)
70056999
{
7006-
PyDictValues *values = _PyObject_InlineValues(obj);
7000+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
70077001

7008-
if (values->valid) {
7009-
for (Py_ssize_t i = 0; i < values->capacity; i++) {
7010-
Py_CLEAR(values->values[i]);
7011-
}
7012-
values->valid = 0;
7013-
}
7002+
PyDictValues *values = _PyObject_InlineValues(obj);
70147003

70157004
#ifdef Py_GIL_DISABLED
70167005
PyDictObject *dict = _PyObject_ManagedDictPointer(obj)->dict;
@@ -7022,6 +7011,14 @@ set_dict_inline_values(PyObject *obj, PyObject *new_dict)
70227011

70237012
Py_XINCREF(new_dict);
70247013
_PyObject_ManagedDictPointer(obj)->dict = (PyDictObject *)new_dict;
7014+
7015+
if (values->valid) {
7016+
FT_ATOMIC_STORE_UINT8_RELAXED(values->valid, 0);
7017+
for (Py_ssize_t i = 0; i < values->capacity; i++) {
7018+
Py_CLEAR(values->values[i]);
7019+
}
7020+
}
7021+
70257022
return true;
70267023
}
70277024

@@ -7032,7 +7029,7 @@ _PyObject_SetManagedDict(PyObject *obj, PyObject *new_dict)
70327029
assert(_PyObject_InlineValuesConsistencyCheck(obj));
70337030
PyTypeObject *tp = Py_TYPE(obj);
70347031
if (tp->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
7035-
PyDictObject *dict = _PyObject_ManagedDictPointer(obj)->dict;
7032+
PyDictObject *dict = _PyObject_GetManagedDict(obj);
70367033
if (dict) {
70377034
#ifdef Py_GIL_DISABLED
70387035
clear_dict:
@@ -7120,7 +7117,7 @@ PyObject_GenericGetDict(PyObject *obj, void *context)
71207117
dict = _PyObject_GetManagedDict(obj);
71217118
if (dict == NULL &&
71227119
(tp->tp_flags & Py_TPFLAGS_INLINE_VALUES) &&
7123-
_PyObject_InlineValues(obj)->valid) {
7120+
FT_ATOMIC_LOAD_UINT8_RELAXED(_PyObject_InlineValues(obj)->valid)) {
71247121
dict = _PyObject_MaterializeManagedDict(obj);
71257122
}
71267123
else if (dict == NULL) {

‎Objects/object.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,7 +1483,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
14831483
}
14841484
PyObject *dict, *attr;
14851485
if ((tp->tp_flags & Py_TPFLAGS_INLINE_VALUES) &&
1486-
!_PyObject_TryGetInstanceAttribute(obj, name, &attr)) {
1486+
_PyObject_TryGetInstanceAttribute(obj, name, &attr)) {
14871487
if (attr != NULL) {
14881488
*method = attr;
14891489
Py_XDECREF(descr);
@@ -1587,7 +1587,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name,
15871587
if (dict == NULL) {
15881588
if ((tp->tp_flags & Py_TPFLAGS_INLINE_VALUES)) {
15891589
if (PyUnicode_CheckExact(name) &&
1590-
!_PyObject_TryGetInstanceAttribute(obj, name, &res)) {
1590+
_PyObject_TryGetInstanceAttribute(obj, name, &res)) {
15911591
if (res != NULL) {
15921592
goto done;
15931593
}
@@ -1698,7 +1698,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
16981698
PyObject **dictptr;
16991699

17001700
if ((tp->tp_flags & Py_TPFLAGS_INLINE_VALUES)) {
1701-
res = _PyObject_TryStoreInstanceAttribute(obj, name, value);
1701+
res = _PyObject_StoreInstanceAttribute(obj, name, value);
17021702
goto error_check;
17031703
}
17041704

‎Objects/typeobject.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6199,7 +6199,7 @@ object_set_class(PyObject *self, PyObject *value, void *closure)
61996199

62006200
Py_BEGIN_CRITICAL_SECTION2(self, dict);
62016201

6202-
if (dict == NULL || _PyDict_DetachFromObject(dict, self)) {
6202+
if (dict == NULL || _PyDict_DetachFromObject(dict, self) < 0) {
62036203
error = true;
62046204
}
62056205

0 commit comments

Comments
 (0)
0