traverseda
/
cpython
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Issue #1868: Eliminate subtle timing issues in thread-local objects by 

getting rid of the cached copy of thread-local attribute dictionary.
This commit is contained in:
Antoine Pitrou 2010-08-28 18:17:03 +00:00
parent 64a38c0eb5
commit 1a9a9d5433
6 changed files with 182 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Include/object.h
View File

@ -448,6 +448,14 @@ PyAPI_FUNC(int) PyCallable_Check(PyObject *);
PyAPI_FUNC(void) PyObject_ClearWeakRefs(PyObject *);
/* Same as PyObject_Generic{Get,Set}Attr, but passing the attributes
dict as the last parameter. */
PyAPI_FUNC(PyObject *)
_PyObject_GenericGetAttrWithDict(PyObject *, PyObject *, PyObject *);
PyAPI_FUNC(int)
_PyObject_GenericSetAttrWithDict(PyObject *, PyObject *,
PyObject *, PyObject *);
/* PyObject_Dir(obj) acts like Python builtins.dir(obj), returning a
list of strings. PyObject_Dir(NULL) is like builtins.dir(),

8
Lib/_threading_local.py
View File

@ -194,6 +194,10 @@ class local(_localbase):
lock.release()
def __setattr__(self, name, value):
if name == '__dict__':
raise AttributeError(
"%r object attribute '__dict__' is read-only"
% self.__class__.__name__)
lock = object.__getattribute__(self, '_local__lock')
lock.acquire()
try:
@ -203,6 +207,10 @@ class local(_localbase):
lock.release()
def __delattr__(self, name):
if name == '__dict__':
raise AttributeError(
"%r object attribute '__dict__' is read-only"
% self.__class__.__name__)
lock = object.__getattribute__(self, '_local__lock')
lock.acquire()
try:

62
Lib/test/test_threading_local.py
View File

@ -123,6 +123,67 @@ class BaseLocalTest:
self.assertRaises(TypeError, self._local, a=1)
self.assertRaises(TypeError, self._local, 1)
def _test_one_class(self, c):
self._failed = "No error message set or cleared."
obj = c()
e1 = threading.Event()
e2 = threading.Event()
def f1():
obj.x = 'foo'
obj.y = 'bar'
del obj.y
e1.set()
e2.wait()
def f2():
try:
foo = obj.x
except AttributeError:
# This is expected -- we haven't set obj.x in this thread yet!
self._failed = "" # passed
else:
self._failed = ('Incorrectly got value %r from class %r\n' %
(foo, c))
sys.stderr.write(self._failed)
t1 = threading.Thread(target=f1)
t1.start()
e1.wait()
t2 = threading.Thread(target=f2)
t2.start()
t2.join()
# The test is done; just let t1 know it can exit, and wait for it.
e2.set()
t1.join()
self.assertFalse(self._failed, self._failed)
def test_threading_local(self):
self._test_one_class(self._local)
def test_threading_local_subclass(self):
class LocalSubclass(self._local):
"""To test that subclasses behave properly."""
self._test_one_class(LocalSubclass)
def _test_dict_attribute(self, cls):
obj = cls()
obj.x = 5
self.assertEqual(obj.__dict__, {'x': 5})
with self.assertRaises(AttributeError):
obj.__dict__ = {}
with self.assertRaises(AttributeError):
del obj.__dict__
def test_dict_attribute(self):
self._test_dict_attribute(self._local)
def test_dict_attribute_subclass(self):
class LocalSubclass(self._local):
"""To test that subclasses behave properly."""
self._test_dict_attribute(LocalSubclass)
class ThreadLocalTest(unittest.TestCase, BaseLocalTest):
_local = _thread._local
@ -140,7 +201,6 @@ class ThreadLocalTest(unittest.TestCase, BaseLocalTest):
gc.collect()
self.assertIs(wr(), None)
class PyThreadingLocalTest(unittest.TestCase, BaseLocalTest):
_local = _threading_local.local

3
Misc/NEWS
View File

@ -132,6 +132,9 @@ Extensions
Library
-------
- Issue #1868: Eliminate subtle timing issues in thread-local objects by
getting rid of the cached copy of thread-local attribute dictionary.
- Issue #1512791: In setframerate() in the wave module, non-integral
frame rates are rounded to the nearest integer.

81
Modules/_threadmodule.c
View File

@ -15,6 +15,7 @@
static PyObject *ThreadError;
static long nb_threads = 0;
static PyObject *str_dict;
/* Lock objects */
@ -586,8 +587,6 @@ typedef struct {
PyObject *key;
PyObject *args;
PyObject *kw;
/* The current thread's local dict (necessary for tp_dictoffset) */
PyObject *dict;
PyObject *weakreflist; /* List of weak references to self */
/* A {localdummy weakref -> localdict} dict */
PyObject *dummies;
@ -599,9 +598,9 @@ typedef struct {
static PyObject *_ldict(localobject *self);
static PyObject *_localdummy_destroyed(PyObject *meth_self, PyObject *dummyweakref);
/* Create and register the dummy for the current thread, as well as the
corresponding local dict */
static int
/* Create and register the dummy for the current thread.
Returns a borrowed reference of the corresponding local dict */
static PyObject *
_local_create_dummy(localobject *self)
{
PyObject *tdict, *ldict = NULL, *wr = NULL;
@ -637,15 +636,14 @@ _local_create_dummy(localobject *self)
goto err;
Py_CLEAR(dummy);
Py_CLEAR(self->dict);
self->dict = ldict;
return 0;
Py_DECREF(ldict);
return ldict;
err:
Py_XDECREF(ldict);
Py_XDECREF(wr);
Py_XDECREF(dummy);
return -1;
return NULL;
}
static PyObject *
@ -691,7 +689,7 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
if (self->wr_callback == NULL)
goto err;
if (_local_create_dummy(self) < 0)
if (_local_create_dummy(self) == NULL)
goto err;
return (PyObject *)self;
@ -707,7 +705,6 @@ local_traverse(localobject *self, visitproc visit, void *arg)
Py_VISIT(self->args);
Py_VISIT(self->kw);
Py_VISIT(self->dummies);
Py_VISIT(self->dict);
return 0;
}
@ -718,7 +715,6 @@ local_clear(localobject *self)
Py_CLEAR(self->args);
Py_CLEAR(self->kw);
Py_CLEAR(self->dummies);
Py_CLEAR(self->dict);
Py_CLEAR(self->wr_callback);
/* Remove all strong references to dummies from the thread states */
if (self->key
@ -764,9 +760,9 @@ _ldict(localobject *self)
dummy = PyDict_GetItem(tdict, self->key);
if (dummy == NULL) {
if (_local_create_dummy(self) < 0)
ldict = _local_create_dummy(self);
if (ldict == NULL)
return NULL;
ldict = self->dict;
if (Py_TYPE(self)->tp_init != PyBaseObject_Type.tp_init &&
Py_TYPE(self)->tp_init((PyObject*)self,
@ -783,14 +779,6 @@ _ldict(localobject *self)
ldict = ((localdummyobject *) dummy)->localdict;
}
/* The call to tp_init above may have caused another thread to run.
Install our ldict again. */
if (self->dict != ldict) {
Py_INCREF(ldict);
Py_CLEAR(self->dict);
self->dict = ldict;
}
return ldict;
}
@ -798,29 +786,25 @@ static int
local_setattro(localobject *self, PyObject *name, PyObject *v)
{
PyObject *ldict;
int r;
ldict = _ldict(self);
if (ldict == NULL)
return -1;
return PyObject_GenericSetAttr((PyObject *)self, name, v);
}
r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
if (r == 1) {
PyErr_Format(PyExc_AttributeError,
"'%.50s' object attribute '%U' is read-only",
Py_TYPE(self)->tp_name, name);
return -1;
}
if (r == -1)
return -1;
static PyObject *
local_getdict(localobject *self, void *closure)
{
PyObject *ldict;
ldict = _ldict(self);
Py_XINCREF(ldict);
return ldict;
return _PyObject_GenericSetAttrWithDict((PyObject *)self, name, v, ldict);
}
static PyGetSetDef local_getset[] = {
{"__dict__", (getter)local_getdict, (setter)NULL,
"Local-data dictionary", NULL},
{NULL} /* Sentinel */
};
static PyObject *local_getattro(localobject *, PyObject *);
static PyTypeObject localtype = {
@ -854,12 +838,12 @@ static PyTypeObject localtype = {
/* tp_iternext */ 0,
/* tp_methods */ 0,
/* tp_members */ 0,
/* tp_getset */ local_getset,
/* tp_getset */ 0,
/* tp_base */ 0,
/* tp_dict */ 0, /* internal use */
/* tp_descr_get */ 0,
/* tp_descr_set */ 0,
/* tp_dictoffset */ offsetof(localobject, dict),
/* tp_dictoffset */ 0,
/* tp_init */ 0,
/* tp_alloc */ 0,
/* tp_new */ local_new,
@ -871,20 +855,29 @@ static PyObject *
local_getattro(localobject *self, PyObject *name)
{
PyObject *ldict, *value;
int r;
ldict = _ldict(self);
if (ldict == NULL)
return NULL;
r = PyObject_RichCompareBool(name, str_dict, Py_EQ);
if (r == 1) {
Py_INCREF(ldict);
return ldict;
}
if (r == -1)
return NULL;
if (Py_TYPE(self) != &localtype)
/* use generic lookup for subtypes */
return PyObject_GenericGetAttr((PyObject *)self, name);
return _PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict);
/* Optimization: just look in dict ourselves */
value = PyDict_GetItem(ldict, name);
if (value == NULL)
/* Fall back on generic to get __class__ and __dict__ */
return PyObject_GenericGetAttr((PyObject *)self, name);
return _PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict);
Py_INCREF(value);
return value;
@ -909,8 +902,6 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
PyObject *ldict;
ldict = PyDict_GetItem(self->dummies, dummyweakref);
if (ldict != NULL) {
if (ldict == self->dict)
Py_CLEAR(self->dict);
PyDict_DelItem(self->dummies, dummyweakref);
}
if (PyErr_Occurred())
@ -1278,6 +1269,10 @@ PyInit__thread(void)
nb_threads = 0;
str_dict = PyUnicode_InternFromString("__dict__");
if (str_dict == NULL)
return NULL;
/* Initialize the C thread library */
PyThread_init_thread();
return m;

111
Objects/object.c
View File

@ -954,7 +954,7 @@ _PyObject_NextNotImplemented(PyObject *self)
/* Generic GetAttr functions - put these in your tp_[gs]etattro slot */
PyObject *
PyObject_GenericGetAttr(PyObject *obj, PyObject *name)
_PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *dict)
{
PyTypeObject *tp = Py_TYPE(obj);
PyObject *descr = NULL;
@ -990,37 +990,38 @@ PyObject_GenericGetAttr(PyObject *obj, PyObject *name)
}
}
/* Inline _PyObject_GetDictPtr */
dictoffset = tp->tp_dictoffset;
if (dictoffset != 0) {
PyObject *dict;
if (dictoffset < 0) {
Py_ssize_t tsize;
size_t size;
if (dict == NULL) {
/* Inline _PyObject_GetDictPtr */
dictoffset = tp->tp_dictoffset;
if (dictoffset != 0) {
if (dictoffset < 0) {
Py_ssize_t tsize;
size_t size;
tsize = ((PyVarObject *)obj)->ob_size;
if (tsize < 0)
tsize = -tsize;
size = _PyObject_VAR_SIZE(tp, tsize);
tsize = ((PyVarObject *)obj)->ob_size;
if (tsize < 0)
tsize = -tsize;
size = _PyObject_VAR_SIZE(tp, tsize);
dictoffset += (long)size;
assert(dictoffset > 0);
assert(dictoffset % SIZEOF_VOID_P == 0);
}
dictptr = (PyObject **) ((char *)obj + dictoffset);
dict = *dictptr;
if (dict != NULL) {
Py_INCREF(dict);
res = PyDict_GetItem(dict, name);
if (res != NULL) {
Py_INCREF(res);
Py_XDECREF(descr);
Py_DECREF(dict);
goto done;
dictoffset += (long)size;
assert(dictoffset > 0);
assert(dictoffset % SIZEOF_VOID_P == 0);
}
Py_DECREF(dict);
dictptr = (PyObject **) ((char *)obj + dictoffset);
dict = *dictptr;
}
}
if (dict != NULL) {
Py_INCREF(dict);
res = PyDict_GetItem(dict, name);
if (res != NULL) {
Py_INCREF(res);
Py_XDECREF(descr);
Py_DECREF(dict);
goto done;
}
Py_DECREF(dict);
}
if (f != NULL) {
res = f(descr, obj, (PyObject *)Py_TYPE(obj));
@ -1042,8 +1043,15 @@ PyObject_GenericGetAttr(PyObject *obj, PyObject *name)
return res;
}
PyObject *
PyObject_GenericGetAttr(PyObject *obj, PyObject *name)
{
return _PyObject_GenericGetAttrWithDict(obj, name, NULL);
}
int
PyObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *value)
_PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
PyObject *value, PyObject *dict)
{
PyTypeObject *tp = Py_TYPE(obj);
PyObject *descr;
@ -1075,27 +1083,29 @@ PyObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *value)
}
}
dictptr = _PyObject_GetDictPtr(obj);
if (dictptr != NULL) {
PyObject *dict = *dictptr;
if (dict == NULL && value != NULL) {
dict = PyDict_New();
if (dict == NULL)
goto done;
*dictptr = dict;
}
if (dict != NULL) {
Py_INCREF(dict);
if (value == NULL)
res = PyDict_DelItem(dict, name);
else
res = PyDict_SetItem(dict, name, value);
if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError))
PyErr_SetObject(PyExc_AttributeError, name);
Py_DECREF(dict);
goto done;
if (dict == NULL) {
dictptr = _PyObject_GetDictPtr(obj);
if (dictptr != NULL) {
dict = *dictptr;
if (dict == NULL && value != NULL) {
dict = PyDict_New();
if (dict == NULL)
goto done;
*dictptr = dict;
}
}
}
if (dict != NULL) {
Py_INCREF(dict);
if (value == NULL)
res = PyDict_DelItem(dict, name);
else
res = PyDict_SetItem(dict, name, value);
if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError))
PyErr_SetObject(PyExc_AttributeError, name);
Py_DECREF(dict);
goto done;
}
if (f != NULL) {
res = f(descr, obj, value);
@ -1117,6 +1127,13 @@ PyObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *value)
return res;
}
int
PyObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *value)
{
return _PyObject_GenericSetAttrWithDict(obj, name, value, NULL);
}
/* Test a value used as condition, e.g., in a for or if statement.
Return -1 if an error occurred */