+-----------------------------------+-----------------------------------+
| :c:func:`PyImport_AddModule` | :c:func:`PyImport_AddModuleRef` |
+-----------------------------------+-----------------------------------+
+| :c:func:`PyCell_GET` | :c:func:`PyCell_Get` |
++-----------------------------------+-----------------------------------+
Not all APIs that return borrowed references are problematic. For
example, :c:func:`PyTuple_GetItem` is safe because tuples are immutable.
#include "pystats.h"
#include "pyatomic.h"
#include "lock.h"
+#include "critical_section.h"
#include "object.h"
#include "refcount.h"
#include "objimpl.h"
#include "import.h"
#include "abstract.h"
#include "bltinmodule.h"
-#include "critical_section.h"
#include "cpython/pyctype.h"
#include "pystrtod.h"
#include "pystrcmp.h"
PyAPI_FUNC(int) PyCell_Set(PyObject *, PyObject *);
static inline PyObject* PyCell_GET(PyObject *op) {
+ PyObject *res;
PyCellObject *cell;
assert(PyCell_Check(op));
cell = _Py_CAST(PyCellObject*, op);
- return cell->ob_ref;
+ Py_BEGIN_CRITICAL_SECTION(cell);
+ res = cell->ob_ref;
+ Py_END_CRITICAL_SECTION();
+ return res;
}
#define PyCell_GET(op) PyCell_GET(_PyObject_CAST(op))
PyCellObject *cell;
assert(PyCell_Check(op));
cell = _Py_CAST(PyCellObject*, op);
+ Py_BEGIN_CRITICAL_SECTION(cell);
cell->ob_ref = value;
+ Py_END_CRITICAL_SECTION();
}
#define PyCell_SET(op, value) PyCell_SET(_PyObject_CAST(op), (value))
--- /dev/null
+# It's most useful to run these tests with ThreadSanitizer enabled.
+import sys
+import functools
+import threading
+import time
+import unittest
+
+from test.support import threading_helper
+
+
+class TestBase(unittest.TestCase):
+ pass
+
+
+def do_race(func1, func2):
+ """Run func1() and func2() repeatedly in separate threads."""
+ n = 1000
+
+ barrier = threading.Barrier(2)
+
+ def repeat(func):
+ barrier.wait()
+ for _i in range(n):
+ func()
+
+ threads = [
+ threading.Thread(target=functools.partial(repeat, func1)),
+ threading.Thread(target=functools.partial(repeat, func2)),
+ ]
+ for thread in threads:
+ thread.start()
+ for thread in threads:
+ thread.join()
+
+
+@threading_helper.requires_working_threading()
+class TestRaces(TestBase):
+ def test_racing_cell_set(self):
+ """Test cell object gettr/settr properties."""
+
+ def nested_func():
+ x = 0
+
+ def inner():
+ nonlocal x
+ x += 1
+
+ # This doesn't race because LOAD_DEREF and STORE_DEREF on the
+ # cell object use critical sections.
+ do_race(nested_func, nested_func)
+
+ def nested_func2():
+ x = 0
+
+ def inner():
+ y = x
+ frame = sys._getframe(1)
+ frame.f_locals["x"] = 2
+
+ return inner
+
+ def mutate_func2():
+ inner = nested_func2()
+ cell = inner.__closure__[0]
+ old_value = cell.cell_contents
+ cell.cell_contents = 1000
+ time.sleep(0)
+ cell.cell_contents = old_value
+ time.sleep(0)
+
+ # This revealed a race with cell_set_contents() since it was missing
+ # the critical section.
+ do_race(nested_func2, mutate_func2)
+
+ def test_racing_cell_cmp_repr(self):
+ """Test cell object compare and repr methods."""
+
+ def nested_func():
+ x = 0
+ y = 0
+
+ def inner():
+ return x + y
+
+ return inner.__closure__
+
+ cell_a, cell_b = nested_func()
+
+ def mutate():
+ cell_a.cell_contents += 1
+
+ def access():
+ cell_a == cell_b
+ s = repr(cell_a)
+
+ # cell_richcompare() and cell_repr used to have data races
+ do_race(mutate, access)
+
+ def test_racing_load_super_attr(self):
+ """Test (un)specialization of LOAD_SUPER_ATTR opcode."""
+
+ class C:
+ def __init__(self):
+ try:
+ super().__init__
+ super().__init__()
+ except RuntimeError:
+ pass # happens if __class__ is replaced with non-type
+
+ def access():
+ C()
+
+ def mutate():
+ # Swap out the super() global with a different one
+ real_super = super
+ globals()["super"] = lambda s=1: s
+ time.sleep(0)
+ globals()["super"] = real_super
+ time.sleep(0)
+ # Swap out the __class__ closure value with a non-type
+ cell = C.__init__.__closure__[0]
+ real_class = cell.cell_contents
+ cell.cell_contents = 99
+ time.sleep(0)
+ cell.cell_contents = real_class
+
+ # The initial PR adding specialized opcodes for LOAD_SUPER_ATTR
+ # had some races (one with the super() global changing and one
+ # with the cell binding being changed).
+ do_race(access, mutate)
+
+
+if __name__ == "__main__":
+ unittest.main()
PyObject_GC_Del(op);
}
+static PyObject *
+cell_compare_impl(PyObject *a, PyObject *b, int op)
+{
+ if (a != NULL && b != NULL) {
+ return PyObject_RichCompare(a, b, op);
+ }
+ else {
+ Py_RETURN_RICHCOMPARE(b == NULL, a == NULL, op);
+ }
+}
+
static PyObject *
cell_richcompare(PyObject *a, PyObject *b, int op)
{
if (!PyCell_Check(a) || !PyCell_Check(b)) {
Py_RETURN_NOTIMPLEMENTED;
}
+ PyObject *a_ref = PyCell_GetRef((PyCellObject *)a);
+ PyObject *b_ref = PyCell_GetRef((PyCellObject *)b);
/* compare cells by contents; empty cells come before anything else */
- a = ((PyCellObject *)a)->ob_ref;
- b = ((PyCellObject *)b)->ob_ref;
- if (a != NULL && b != NULL)
- return PyObject_RichCompare(a, b, op);
+ PyObject *res = cell_compare_impl(a_ref, b_ref, op);
- Py_RETURN_RICHCOMPARE(b == NULL, a == NULL, op);
+ Py_XDECREF(a_ref);
+ Py_XDECREF(b_ref);
+ return res;
}
static PyObject *
cell_repr(PyObject *self)
{
- PyCellObject *op = _PyCell_CAST(self);
- if (op->ob_ref == NULL) {
- return PyUnicode_FromFormat("<cell at %p: empty>", op);
+ PyObject *ref = PyCell_GetRef((PyCellObject *)self);
+ if (ref == NULL) {
+ return PyUnicode_FromFormat("<cell at %p: empty>", self);
}
-
- return PyUnicode_FromFormat("<cell at %p: %.80s object at %p>",
- op, Py_TYPE(op->ob_ref)->tp_name,
- op->ob_ref);
+ PyObject *res = PyUnicode_FromFormat("<cell at %p: %.80s object at %p>",
+ self, Py_TYPE(ref)->tp_name, ref);
+ Py_DECREF(ref);
+ return res;
}
static int
cell_get_contents(PyObject *self, void *closure)
{
PyCellObject *op = _PyCell_CAST(self);
- if (op->ob_ref == NULL) {
+ PyObject *res = PyCell_GetRef(op);
+ if (res == NULL) {
PyErr_SetString(PyExc_ValueError, "Cell is empty");
return NULL;
}
- return Py_NewRef(op->ob_ref);
+ return res;
}
static int
#include "pycore_moduleobject.h" // _PyModule_GetDict()
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
+#include "pycore_cell.h" // PyCell_GetRef() PyCell_SetTakeRef()
#include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches
}
}
if (cell != NULL) {
- PyObject *oldvalue_o = PyCell_GET(cell);
- if (value != oldvalue_o) {
- PyCell_SET(cell, Py_XNewRef(value));
- Py_XDECREF(oldvalue_o);
- }
+ Py_XINCREF(value);
+ PyCell_SetTakeRef((PyCellObject *)cell, value);
} else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {
PyStackRef_XCLOSE(fast[i]);
fast[i] = PyStackRef_FromPyObjectNew(value);
if (kind & CO_FAST_FREE) {
// The cell was set by COPY_FREE_VARS.
assert(value != NULL && PyCell_Check(value));
- value = PyCell_GET(value);
+ value = PyCell_GetRef((PyCellObject *)value);
}
else if (kind & CO_FAST_CELL) {
if (value != NULL) {
if (PyCell_Check(value)) {
assert(!_PyFrame_IsIncomplete(frame));
- value = PyCell_GET(value);
+ value = PyCell_GetRef((PyCellObject *)value);
+ }
+ else {
+ // (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL),
+ // with the initial value set when the frame was created...
+ // (unlikely) ...or it was set via the f_locals proxy.
+ Py_INCREF(value);
}
- // (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL),
- // with the initial value set when the frame was created...
- // (unlikely) ...or it was set via the f_locals proxy.
}
}
+ else {
+ Py_XINCREF(value);
+ }
}
*pvalue = value;
return 1;
continue;
}
- PyObject *value; // borrowed reference
+ PyObject *value;
if (!frame_get_var(frame, co, i, &value)) {
break;
}
if (value == NULL) {
break;
}
- return Py_NewRef(value);
+ return value;
}
PyErr_Format(PyExc_NameError, "variable %R does not exist", name);
#include "pycore_typeobject.h" // struct type_cache
#include "pycore_unionobject.h" // _Py_union_type_or
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
+#include "pycore_cell.h" // PyCell_GetRef()
#include "opcode.h" // MAKE_CELL
#include <stddef.h> // ptrdiff_t
assert(_PyFrame_GetCode(cframe)->co_nlocalsplus > 0);
PyObject *firstarg = PyStackRef_AsPyObjectBorrow(_PyFrame_GetLocalsArray(cframe)[0]);
+ if (firstarg == NULL) {
+ PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted");
+ return -1;
+ }
// The first argument might be a cell.
- if (firstarg != NULL && (_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL)) {
- // "firstarg" is a cell here unless (very unlikely) super()
- // was called from the C-API before the first MAKE_CELL op.
- if (_PyInterpreterFrame_LASTI(cframe) >= 0) {
- // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need
- // to use _PyOpcode_Deopt here:
- assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL ||
- _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS);
- assert(PyCell_Check(firstarg));
- firstarg = PyCell_GET(firstarg);
+ // "firstarg" is a cell here unless (very unlikely) super()
+ // was called from the C-API before the first MAKE_CELL op.
+ if ((_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL) &&
+ (_PyInterpreterFrame_LASTI(cframe) >= 0)) {
+ // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need
+ // to use _PyOpcode_Deopt here:
+ assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL ||
+ _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS);
+ assert(PyCell_Check(firstarg));
+ firstarg = PyCell_GetRef((PyCellObject *)firstarg);
+ if (firstarg == NULL) {
+ PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted");
+ return -1;
}
}
- if (firstarg == NULL) {
- PyErr_SetString(PyExc_RuntimeError,
- "super(): arg[0] deleted");
- return -1;
+ else {
+ Py_INCREF(firstarg);
}
// Look for __class__ in the free vars.
if (cell == NULL || !PyCell_Check(cell)) {
PyErr_SetString(PyExc_RuntimeError,
"super(): bad __class__ cell");
+ Py_DECREF(firstarg);
return -1;
}
- type = (PyTypeObject *) PyCell_GET(cell);
+ type = (PyTypeObject *) PyCell_GetRef((PyCellObject *)cell);
if (type == NULL) {
PyErr_SetString(PyExc_RuntimeError,
"super(): empty __class__ cell");
+ Py_DECREF(firstarg);
return -1;
}
if (!PyType_Check(type)) {
PyErr_Format(PyExc_RuntimeError,
"super(): __class__ is not a type (%s)",
Py_TYPE(type)->tp_name);
+ Py_DECREF(type);
+ Py_DECREF(firstarg);
return -1;
}
break;
if (type == NULL) {
PyErr_SetString(PyExc_RuntimeError,
"super(): __class__ cell not found");
+ Py_DECREF(firstarg);
return -1;
}
return -1;
}
}
+ else {
+ Py_INCREF(type);
+ Py_XINCREF(obj);
+ }
- if (obj == Py_None)
+ if (obj == Py_None) {
+ Py_DECREF(obj);
obj = NULL;
+ }
if (obj != NULL) {
obj_type = supercheck(type, obj);
- if (obj_type == NULL)
+ if (obj_type == NULL) {
+ Py_DECREF(type);
+ Py_DECREF(obj);
return -1;
- Py_INCREF(obj);
+ }
}
- Py_XSETREF(su->type, (PyTypeObject*)Py_NewRef(type));
+ Py_XSETREF(su->type, (PyTypeObject*)type);
Py_XSETREF(su->obj, obj);
Py_XSETREF(su->obj_type, obj_type);
return 0;
#include "pycore_pythonrun.h" // _Py_SourceAsString()
#include "pycore_sysmodule.h" // _PySys_GetAttr()
#include "pycore_tuple.h" // _PyTuple_FromArray()
+#include "pycore_cell.h" // PyCell_GetRef()
#include "clinic/bltinmodule.c.h"
PyObject *margs[3] = {name, bases, ns};
cls = PyObject_VectorcallDict(meta, margs, 3, mkw);
if (cls != NULL && PyType_Check(cls) && PyCell_Check(cell)) {
- PyObject *cell_cls = PyCell_GET(cell);
+ PyObject *cell_cls = PyCell_GetRef((PyCellObject *)cell);
if (cell_cls != cls) {
if (cell_cls == NULL) {
const char *msg =
"__class__ set to %.200R defining %.200R as %.200R";
PyErr_Format(PyExc_TypeError, msg, cell_cls, name, cls);
}
+ Py_XDECREF(cell_cls);
Py_SETREF(cls, NULL);
goto error;
}
+ else {
+ Py_DECREF(cell_cls);
+ }
}
}
error:
projects / thirdparty / Python / cpython.git / commitdiff
