Skip to content

Commit

Permalink
ceval: fix some thread-safety issues
Browse files Browse the repository at this point in the history
  • Loading branch information
@colesbury
colesbury committed Apr 23, 2023
1 parent 2ae5ee5 commit 9f9b3d0
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 108 deletions.
9 changes: 9 additions & 0 deletions Include/internal/pycore_list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern "C" {
#endif

#include "listobject.h" // _PyList_CAST()
#include <stddef.h> // offsetof()


/* runtime lifecycle */
Expand Down Expand Up @@ -59,6 +60,14 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem)
return _PyList_AppendTakeRefListResize(self, newitem);
}

static inline Py_ssize_t
_PyList_Capacity(PyObject **ob_item)
{
char *mem = (char *)ob_item;
_PyListArray *arr = (_PyListArray *)(mem - offsetof(_PyListArray, ob_item));
return arr->allocated;
}

// Repeat the bytes of a buffer in place
static inline void
_Py_memory_repeat(char* dest, Py_ssize_t len_dest, Py_ssize_t len_src)
Expand Down
10 changes: 10 additions & 0 deletions Objects/cellobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ PyCell_New(PyObject *obj)
op = (PyCellObject *)PyObject_GC_New(PyCellObject, &PyCell_Type);
if (op == NULL)
return NULL;

if (obj) {
_PyObject_SetMaybeWeakref(obj);
}
op->ob_ref = Py_XNewRef(obj);

_PyObject_GC_TRACK(op);
Expand Down Expand Up @@ -67,6 +71,9 @@ PyCell_Set(PyObject *op, PyObject *value)
return -1;
}
PyObject *old_value = PyCell_GET(op);
if (value) {
_PyObject_SetMaybeWeakref(value);
}
PyCell_SET(op, Py_XNewRef(value));
Py_XDECREF(old_value);
return 0;
Expand Down Expand Up @@ -139,6 +146,9 @@ cell_get_contents(PyCellObject *op, void *closure)
static int
cell_set_contents(PyCellObject *op, PyObject *obj, void *Py_UNUSED(ignored))
{
if (obj) {
_PyObject_SetMaybeWeakref(obj);
}
Py_XSETREF(op->ob_ref, Py_XNewRef(obj));
return 0;
}
Expand Down
9 changes: 1 addition & 8 deletions Objects/listobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,13 +301,6 @@ valid_index(Py_ssize_t i, Py_ssize_t limit)
return (size_t) i < (size_t) limit;
}

static Py_ssize_t
list_capacity(PyObject **ob_item)
{
_PyListArray *arr = list_array(ob_item);
return _Py_atomic_load_ssize_relaxed(&arr->allocated);
}

Py_NO_INLINE static PyObject *
list_item_locked(PyListObject *self, Py_ssize_t idx, PyObject *dead)
{
Expand Down Expand Up @@ -349,7 +342,7 @@ list_fetch_item(PyListObject *self, Py_ssize_t idx)
if (ob_item == NULL) {
return NULL;
}
if (!valid_index(idx, list_capacity(ob_item))) {
if (!valid_index(idx, _PyList_Capacity(ob_item))) {
return NULL;
}
PyObject *item = _Py_atomic_load_ptr(&ob_item[idx]);
Expand Down
3 changes: 2 additions & 1 deletion Objects/longobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ get_small_int(sdigit ival)
{
assert(IS_SMALL_INT(ival));
PyObject *v = (PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS + ival];
assert(_PyObject_IS_IMMORTAL(v));
return v;
}

Expand Down Expand Up @@ -277,7 +278,7 @@ _PyLong_AssignValue(PyObject **target, Py_ssize_t value)
return 0;
}
else if (old != NULL && PyLong_CheckExact(old) &&
Py_REFCNT(old) == 1 && Py_SIZE(old) == 1 &&
_PyObject_HasLocalRefcnt(old, 1) && Py_SIZE(old) == 1 &&
(size_t)value <= PyLong_MASK)
{
// Mutate in place if there are no other references the old
Expand Down
106 changes: 57 additions & 49 deletions Python/bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "pycore_call.h" // _PyObject_FastCallDictTstate()
#include "pycore_ceval.h" // _PyEval_SignalAsyncExc()
#include "pycore_code.h"
#include "pycore_critical_section.h"
#include "pycore_function.h"
#include "pycore_intrinsics.h"
#include "pycore_long.h" // _PyLong_GetZero()
Expand Down Expand Up @@ -65,6 +66,7 @@ do { \

/* Flow control macros */
#define DEOPT_IF(cond, instname) ((void)0)
#define DEOPT_UNLOCK_IF(cond, instname) ((void)0)
#define ERROR_IF(cond, labelname) ((void)0)
#define JUMPBY(offset) ((void)0)
#define GO_TO_INSTRUCTION(instname) ((void)0)
Expand Down Expand Up @@ -422,7 +424,7 @@ dummy_func(
assert(cframe.use_tracing == 0);
DEOPT_IF(!PyDict_CheckExact(dict), BINARY_SUBSCR);
STAT_INC(BINARY_SUBSCR, hit);
res = PyDict_GetItemWithError(dict, sub);
res = PyDict_FetchItemWithError(dict, sub);
if (res == NULL) {
if (!_PyErr_Occurred(tstate)) {
_PyErr_SetKeyError(sub);
Expand All @@ -431,7 +433,6 @@ dummy_func(
Py_DECREF(sub);
ERROR_IF(true, error);
}
Py_INCREF(res); // Do this before DECREF'ing dict, sub
DECREF_INPUTS();
}

Expand Down Expand Up @@ -1223,15 +1224,14 @@ dummy_func(
}

inst(DELETE_DEREF, (--)) {
PyObject *cell = GETLOCAL(oparg);
PyObject *oldobj = PyCell_GET(cell);
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
PyObject *oldobj = _Py_atomic_exchange_ptr(&cell->ob_ref, NULL);
// Can't use ERROR_IF here.
// Fortunately we don't need its superpower.
if (oldobj == NULL) {
format_exc_unbound(tstate, frame->f_code, oparg);
goto error;
}
PyCell_SET(cell, NULL);
Py_DECREF(oldobj);
}

Expand Down Expand Up @@ -1270,19 +1270,18 @@ dummy_func(
}

inst(LOAD_DEREF, ( -- value)) {
PyObject *cell = GETLOCAL(oparg);
value = PyCell_GET(cell);
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
value = _Py_XFetchRef(&cell->ob_ref);
if (value == NULL) {
format_exc_unbound(tstate, frame->f_code, oparg);
ERROR_IF(true, error);
}
Py_INCREF(value);
}

inst(STORE_DEREF, (v --)) {
PyObject *cell = GETLOCAL(oparg);
PyObject *oldobj = PyCell_GET(cell);
PyCell_SET(cell, v);
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
_PyObject_SetMaybeWeakref(v);
PyObject *oldobj = _Py_atomic_exchange_ptr(&cell->ob_ref, v);
Py_XDECREF(oldobj);
}

Expand Down Expand Up @@ -1566,12 +1565,12 @@ dummy_func(
DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
assert(tp->tp_dictoffset < 0);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
PyDictOrValues dorv = _PyObject_DictOrValues(owner);
DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
res = _PyDictOrValues_GetValues(dorv)->values[cache->index];
PyDictValues *dv = _PyDictOrValues_GetValues(dorv);
res = _Py_TryXFetchRef(&dv->values[cache->index]);
DEOPT_IF(res == NULL, LOAD_ATTR);
STAT_INC(LOAD_ATTR, hit);
Py_INCREF(res);
SET_TOP(NULL);
STACK_GROW((oparg & 1));
SET_TOP(res);
Expand All @@ -1588,15 +1587,14 @@ dummy_func(
DEOPT_IF(!PyModule_CheckExact(owner), LOAD_ATTR);
PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict;
assert(dict != NULL);
DEOPT_IF(dict->ma_keys->dk_version != read_u32(cache->version),
LOAD_ATTR);
assert(dict->ma_keys->dk_kind == DICT_KEYS_UNICODE);
assert(cache->index < dict->ma_keys->dk_nentries);
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + cache->index;
res = ep->me_value;
PyDictKeysObject *keys = _Py_atomic_load_ptr_relaxed(&dict->ma_keys);
DEOPT_IF(keys->dk_version != read_u32(cache->version), LOAD_ATTR);
assert(keys->dk_kind == DICT_KEYS_UNICODE);
assert(cache->index < keys->dk_nentries);
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(keys) + cache->index;
res = _Py_TryXFetchRef(&ep->me_value);
DEOPT_IF(res == NULL, LOAD_ATTR);
STAT_INC(LOAD_ATTR, hit);
Py_INCREF(res);
SET_TOP(NULL);
STACK_GROW((oparg & 1));
SET_TOP(res);
Expand All @@ -1615,7 +1613,7 @@ dummy_func(
assert(type_version != 0);
DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
PyDictOrValues dorv = _PyObject_DictOrValues(owner);
DEOPT_IF(_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv);
DEOPT_IF(dict == NULL, LOAD_ATTR);
Expand Down Expand Up @@ -1758,18 +1756,17 @@ dummy_func(
assert(type_version != 0);
DEOPT_IF(tp->tp_version_tag != type_version, STORE_ATTR);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
DEOPT_IF(!_PyDictOrValues_IsValues(dorv), STORE_ATTR);
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(owner);
PyDictValues *values = _PyDictValues_Lock(dorv_ptr);
DEOPT_IF(values == NULL, STORE_ATTR);
STAT_INC(STORE_ATTR, hit);
PyDictValues *values = _PyDictOrValues_GetValues(dorv);
PyObject *old_value = values->values[index];
values->values[index] = value;
_Py_atomic_store_ptr_release(&values->values[index], value);
if (old_value == NULL) {
_PyDictValues_AddToInsertionOrder(values, index);
}
else {
Py_DECREF(old_value);
}
_PyDictValues_Unlock(dorv_ptr);
Py_XDECREF(old_value);
Py_DECREF(owner);
}

Expand All @@ -1779,30 +1776,31 @@ dummy_func(
assert(type_version != 0);
DEOPT_IF(tp->tp_version_tag != type_version, STORE_ATTR);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
PyDictOrValues dorv = _PyObject_DictOrValues(owner);
DEOPT_IF(_PyDictOrValues_IsValues(dorv), STORE_ATTR);
PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv);
DEOPT_IF(dict == NULL, STORE_ATTR);
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(names, oparg);
DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, STORE_ATTR);
Py_BEGIN_CRITICAL_SECTION(&dict->ma_mutex);
DEOPT_UNLOCK_IF(hint >= (size_t)dict->ma_keys->dk_nentries, STORE_ATTR);
PyObject *old_value;
uint64_t new_version;
if (DK_IS_UNICODE(dict->ma_keys)) {
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name, STORE_ATTR);
DEOPT_UNLOCK_IF(ep->me_key != name, STORE_ATTR);
old_value = ep->me_value;
DEOPT_IF(old_value == NULL, STORE_ATTR);
DEOPT_UNLOCK_IF(old_value == NULL, STORE_ATTR);
new_version = _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, dict, name, value);
ep->me_value = value;
_Py_atomic_store_ptr_relaxed(&ep->me_value, value);
}
else {
PyDictKeyEntry *ep = DK_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name, STORE_ATTR);
DEOPT_UNLOCK_IF(ep->me_key != name, STORE_ATTR);
old_value = ep->me_value;
DEOPT_IF(old_value == NULL, STORE_ATTR);
DEOPT_UNLOCK_IF(old_value == NULL, STORE_ATTR);
new_version = _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, dict, name, value);
ep->me_value = value;
_Py_atomic_store_ptr_relaxed(&ep->me_value, value);
}
Py_DECREF(old_value);
STAT_INC(STORE_ATTR, hit);
Expand All @@ -1812,6 +1810,7 @@ dummy_func(
}
/* PEP 509 */
dict->ma_version_tag = new_version;
Py_END_CRITICAL_SECTION;
Py_DECREF(owner);
}

Expand Down Expand Up @@ -2287,15 +2286,19 @@ dummy_func(
DEOPT_IF(Py_TYPE(it) != &PyListIter_Type, FOR_ITER);
STAT_INC(FOR_ITER, hit);
PyListObject *seq = it->it_seq;
if (it->it_index >= 0) {
if (it->it_index < PyList_GET_SIZE(seq)) {
PyObject *next = PyList_GET_ITEM(seq, it->it_index++);
PUSH(Py_NewRef(next));
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER);
goto end_for_iter_list; // End of this instruction
}
it->it_index = -1;
Py_ssize_t index = _Py_atomic_load_ssize_relaxed(&it->it_index);
Py_ssize_t size = _Py_atomic_load_ssize_relaxed(&((PyVarObject *)seq)->ob_size);
if ((size_t)index < (size_t)size) {
PyObject **ob_item = _Py_atomic_load_ptr_relaxed(&seq->ob_item);
DEOPT_IF(index >= _PyList_Capacity(ob_item), FOR_ITER);
PyObject *next = _Py_TryXFetchRef(&ob_item[index]);
DEOPT_IF(next == NULL, FOR_ITER);
_Py_atomic_store_ssize_relaxed(&it->it_index, index + 1);
PUSH(next);
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER);
goto end_for_iter_list; // End of this instruction
}
_Py_atomic_store_ssize_relaxed(&it->it_index, -1);
STACK_SHRINK(1);
Py_DECREF(it);
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1);
Expand Down Expand Up @@ -2491,7 +2494,7 @@ dummy_func(
assert(type_version != 0);
DEOPT_IF(self_cls->tp_version_tag != type_version, LOAD_ATTR);
assert(self_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(self);
PyDictOrValues dorv = _PyObject_DictOrValues(self);
DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
PyHeapTypeObject *self_heap_type = (PyHeapTypeObject *)self_cls;
DEOPT_IF(self_heap_type->ht_cached_keys->dk_version !=
Expand Down Expand Up @@ -2976,11 +2979,16 @@ dummy_func(
PyObject *callable = PEEK(3);
PyInterpreterState *interp = _PyInterpreterState_GET();
DEOPT_IF(callable != interp->callable_cache.list_append, CALL);
PyObject *list = SECOND();
DEOPT_IF(!PyList_Check(list), CALL);
PyObject *self = SECOND();
DEOPT_IF(!PyList_Check(self), CALL);
STAT_INC(CALL, hit);
PyObject *arg = POP();
if (_PyList_AppendTakeRef((PyListObject *)list, arg) < 0) {
PyListObject *list = (PyListObject *)self;
int err;
Py_BEGIN_CRITICAL_SECTION(&list->mutex);
err = _PyList_AppendTakeRef(list, arg);
Py_END_CRITICAL_SECTION;
if (err < 0) {
goto error;
}
STACK_SHRINK(2);
Expand Down
12 changes: 11 additions & 1 deletion Python/ceval.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "pycore_call.h" // _PyObject_FastCallDictTstate()
#include "pycore_ceval.h" // _PyEval_SignalAsyncExc()
#include "pycore_code.h"
#include "pycore_critical_section.h"
#include "pycore_function.h"
#include "pycore_intrinsics.h"
#include "pycore_long.h" // _PyLong_GetZero()
Expand Down Expand Up @@ -48,7 +49,7 @@
# error "ceval.c must be build with Py_BUILD_CORE define for best performance"
#endif

#if !defined(Py_DEBUG) && !defined(Py_TRACE_REFS)
#if !defined(Py_DEBUG) && !defined(Py_TRACE_REFS) && !defined(_Py_THREAD_SANITIZER)
// GH-89279: The MSVC compiler does not inline these static inline functions
// in PGO build in _PyEval_EvalFrameDefault(), because this function is over
// the limit of PGO, and that limit cannot be configured.
Expand Down Expand Up @@ -888,6 +889,15 @@ GETITEM(PyObject *v, Py_ssize_t i) {
GO_TO_INSTRUCTION(INSTNAME); \
}

#define DEOPT_UNLOCK_IF(COND, INSTNAME) \
if ((COND)) { \
/* This is only a single jump on release builds! */ \
UPDATE_MISS_STATS((INSTNAME)); \
assert(_PyOpcode_Deopt[opcode] == (INSTNAME)); \
_Py_critical_section_end(&_cs); \
GO_TO_INSTRUCTION(INSTNAME); \
}


#define GLOBALS() frame->f_globals
#define BUILTINS() frame->f_builtins
Expand Down

0 comments on commit 9f9b3d0

Please sign in to comment.