[Python-checkins] r84344 - in python/branches/py3k: Include/object.h Lib/_threading_local.py Lib/test/test_threading_local.py Misc/NEWS Modules/_threadmodule.c Objects/object.c
antoine.pitrou
python-checkins at python.org
Sat Aug 28 20:17:05 CEST 2010
Author: antoine.pitrou
Date: Sat Aug 28 20:17:03 2010
New Revision: 84344
Log:
Issue #1868: Eliminate subtle timing issues in thread-local objects by
getting rid of the cached copy of thread-local attribute dictionary.
Modified:
python/branches/py3k/Include/object.h
python/branches/py3k/Lib/_threading_local.py
python/branches/py3k/Lib/test/test_threading_local.py
python/branches/py3k/Misc/NEWS
python/branches/py3k/Modules/_threadmodule.c
python/branches/py3k/Objects/object.c
Modified: python/branches/py3k/Include/object.h
==============================================================================
--- python/branches/py3k/Include/object.h (original)
+++ python/branches/py3k/Include/object.h Sat Aug 28 20:17:03 2010
@@ -448,6 +448,14 @@
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(),
Modified: python/branches/py3k/Lib/_threading_local.py
==============================================================================
--- python/branches/py3k/Lib/_threading_local.py (original)
+++ python/branches/py3k/Lib/_threading_local.py Sat Aug 28 20:17:03 2010
@@ -194,6 +194,10 @@
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 @@
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:
Modified: python/branches/py3k/Lib/test/test_threading_local.py
==============================================================================
--- python/branches/py3k/Lib/test/test_threading_local.py (original)
+++ python/branches/py3k/Lib/test/test_threading_local.py Sat Aug 28 20:17:03 2010
@@ -123,6 +123,67 @@
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 @@
gc.collect()
self.assertIs(wr(), None)
-
class PyThreadingLocalTest(unittest.TestCase, BaseLocalTest):
_local = _threading_local.local
Modified: python/branches/py3k/Misc/NEWS
==============================================================================
--- python/branches/py3k/Misc/NEWS (original)
+++ python/branches/py3k/Misc/NEWS Sat Aug 28 20:17:03 2010
@@ -132,6 +132,9 @@
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.
Modified: python/branches/py3k/Modules/_threadmodule.c
==============================================================================
--- python/branches/py3k/Modules/_threadmodule.c (original)
+++ python/branches/py3k/Modules/_threadmodule.c Sat Aug 28 20:17:03 2010
@@ -15,6 +15,7 @@
static PyObject *ThreadError;
static long nb_threads = 0;
+static PyObject *str_dict;
/* Lock objects */
@@ -586,8 +587,6 @@
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 @@
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 @@
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 @@
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 @@
Py_VISIT(self->args);
Py_VISIT(self->kw);
Py_VISIT(self->dummies);
- Py_VISIT(self->dict);
return 0;
}
@@ -718,7 +715,6 @@
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 @@
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 = ((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 @@
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 @@
/* 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 @@
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 @@
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 @@
nb_threads = 0;
+ str_dict = PyUnicode_InternFromString("__dict__");
+ if (str_dict == NULL)
+ return NULL;
+
/* Initialize the C thread library */
PyThread_init_thread();
return m;
Modified: python/branches/py3k/Objects/object.c
==============================================================================
--- python/branches/py3k/Objects/object.c (original)
+++ python/branches/py3k/Objects/object.c Sat Aug 28 20:17:03 2010
@@ -954,7 +954,7 @@
/* 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,36 +990,37 @@
}
}
- /* Inline _PyObject_GetDictPtr */
- dictoffset = tp->tp_dictoffset;
- if (dictoffset != 0) {
- PyObject *dict;
- 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);
-
- 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;
+ 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);
+
+ 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;
}
+ Py_DECREF(dict);
}
if (f != NULL) {
@@ -1042,8 +1043,15 @@
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 @@
}
}
- 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 @@
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 */
More information about the Python-checkins
mailing list