diff options
| -rw-r--r-- | Lib/test/test_weakref.py | 20 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/C API/2019-10-08-01-23-24.bpo-38395.MJ6Ey9.rst | 3 | ||||
| -rw-r--r-- | Objects/weakrefobject.c | 119 |
3 files changed, 111 insertions, 31 deletions
diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index e14df9e..228bc17 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -391,6 +391,26 @@ class ReferencesTestCase(TestBase): lyst = List() self.assertEqual(bool(weakref.proxy(lyst)), bool(lyst)) + def test_proxy_iter(self): + # Test fails with a debug build of the interpreter + # (see bpo-38395). + + obj = None + + class MyObj: + def __iter__(self): + nonlocal obj + del obj + return NotImplemented + + obj = MyObj() + p = weakref.proxy(obj) + with self.assertRaises(TypeError): + # "blech" in p calls MyObj.__iter__ through the proxy, + # without keeping a reference to the real object, so it + # can be killed in the middle of the call + "blech" in p + def test_getweakrefcount(self): o = C() ref1 = weakref.ref(o) diff --git a/Misc/NEWS.d/next/C API/2019-10-08-01-23-24.bpo-38395.MJ6Ey9.rst b/Misc/NEWS.d/next/C API/2019-10-08-01-23-24.bpo-38395.MJ6Ey9.rst new file mode 100644 index 0000000..4bc3060 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2019-10-08-01-23-24.bpo-38395.MJ6Ey9.rst @@ -0,0 +1,3 @@ +Fix a crash in :class:`weakref.proxy` objects due to incorrect lifetime +management when calling some associated methods that may delete the last +reference to object being referenced by the proxy. Patch by Pablo Galindo. diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index daee476..bf79e0c 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -145,11 +145,14 @@ weakref_hash(PyWeakReference *self) { if (self->hash != -1) return self->hash; - if (PyWeakref_GET_OBJECT(self) == Py_None) { + PyObject* obj = PyWeakref_GET_OBJECT(self); + if (obj == Py_None) { PyErr_SetString(PyExc_TypeError, "weak object has gone away"); return -1; } - self->hash = PyObject_Hash(PyWeakref_GET_OBJECT(self)); + Py_INCREF(obj); + self->hash = PyObject_Hash(obj); + Py_DECREF(obj); return self->hash; } @@ -159,11 +162,15 @@ weakref_repr(PyWeakReference *self) { PyObject *name, *repr; _Py_IDENTIFIER(__name__); + PyObject* obj = PyWeakref_GET_OBJECT(self); - if (PyWeakref_GET_OBJECT(self) == Py_None) + if (obj == Py_None) { return PyUnicode_FromFormat("<weakref at %p; dead>", self); + } - if (_PyObject_LookupAttrId(PyWeakref_GET_OBJECT(self), &PyId___name__, &name) < 0) { + Py_INCREF(obj); + if (_PyObject_LookupAttrId(obj, &PyId___name__, &name) < 0) { + Py_DECREF(obj); return NULL; } if (name == NULL || !PyUnicode_Check(name)) { @@ -171,16 +178,17 @@ weakref_repr(PyWeakReference *self) "<weakref at %p; to '%s' at %p>", self, Py_TYPE(PyWeakref_GET_OBJECT(self))->tp_name, - PyWeakref_GET_OBJECT(self)); + obj); } else { repr = PyUnicode_FromFormat( "<weakref at %p; to '%s' at %p (%U)>", self, Py_TYPE(PyWeakref_GET_OBJECT(self))->tp_name, - PyWeakref_GET_OBJECT(self), + obj, name); } + Py_DECREF(obj); Py_XDECREF(name); return repr; } @@ -207,8 +215,14 @@ weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op) else Py_RETURN_FALSE; } - return PyObject_RichCompare(PyWeakref_GET_OBJECT(self), - PyWeakref_GET_OBJECT(other), op); + PyObject* obj = PyWeakref_GET_OBJECT(self); + PyObject* other_obj = PyWeakref_GET_OBJECT(other); + Py_INCREF(obj); + Py_INCREF(other_obj); + PyObject* res = PyObject_RichCompare(obj, other_obj, op); + Py_DECREF(obj); + Py_DECREF(other_obj); + return res; } /* Given the head of an object's list of weak references, extract the @@ -415,18 +429,14 @@ proxy_checkref(PyWeakReference *proxy) o = PyWeakref_GET_OBJECT(o); \ } -#define UNWRAP_I(o) \ - if (PyWeakref_CheckProxy(o)) { \ - if (!proxy_checkref((PyWeakReference *)o)) \ - return -1; \ - o = PyWeakref_GET_OBJECT(o); \ - } - #define WRAP_UNARY(method, generic) \ static PyObject * \ method(PyObject *proxy) { \ UNWRAP(proxy); \ - return generic(proxy); \ + Py_INCREF(proxy); \ + PyObject* res = generic(proxy); \ + Py_DECREF(proxy); \ + return res; \ } #define WRAP_BINARY(method, generic) \ @@ -434,7 +444,12 @@ proxy_checkref(PyWeakReference *proxy) method(PyObject *x, PyObject *y) { \ UNWRAP(x); \ UNWRAP(y); \ - return generic(x, y); \ + Py_INCREF(x); \ + Py_INCREF(y); \ + PyObject* res = generic(x, y); \ + Py_DECREF(x); \ + Py_DECREF(y); \ + return res; \ } /* Note that the third arg needs to be checked for NULL since the tp_call @@ -447,7 +462,14 @@ proxy_checkref(PyWeakReference *proxy) UNWRAP(v); \ if (w != NULL) \ UNWRAP(w); \ - return generic(proxy, v, w); \ + Py_INCREF(proxy); \ + Py_INCREF(v); \ + Py_XINCREF(w); \ + PyObject* res = generic(proxy, v, w); \ + Py_DECREF(proxy); \ + Py_DECREF(v); \ + Py_XDECREF(w); \ + return res; \ } #define WRAP_METHOD(method, special) \ @@ -455,7 +477,10 @@ proxy_checkref(PyWeakReference *proxy) method(PyObject *proxy, PyObject *Py_UNUSED(ignored)) { \ _Py_IDENTIFIER(special); \ UNWRAP(proxy); \ - return _PyObject_CallMethodIdNoArgs(proxy, &PyId_##special); \ + Py_INCREF(proxy); \ + PyObject* res = _PyObject_CallMethodIdNoArgs(proxy, &PyId_##special); \ + Py_DECREF(proxy); \ + return res; \ } @@ -481,7 +506,11 @@ proxy_setattr(PyWeakReference *proxy, PyObject *name, PyObject *value) { if (!proxy_checkref(proxy)) return -1; - return PyObject_SetAttr(PyWeakref_GET_OBJECT(proxy), name, value); + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + int res = PyObject_SetAttr(obj, name, value); + Py_DECREF(obj); + return res; } static PyObject * @@ -532,9 +561,13 @@ static int proxy_bool(PyWeakReference *proxy) { PyObject *o = PyWeakref_GET_OBJECT(proxy); - if (!proxy_checkref(proxy)) + if (!proxy_checkref(proxy)) { return -1; - return PyObject_IsTrue(o); + } + Py_INCREF(o); + int res = PyObject_IsTrue(o); + Py_DECREF(o); + return res; } static void @@ -553,9 +586,13 @@ proxy_contains(PyWeakReference *proxy, PyObject *value) { if (!proxy_checkref(proxy)) return -1; - return PySequence_Contains(PyWeakref_GET_OBJECT(proxy), value); -} + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + int res = PySequence_Contains(obj, value); + Py_DECREF(obj); + return res; +} /* mapping slots */ @@ -564,7 +601,12 @@ proxy_length(PyWeakReference *proxy) { if (!proxy_checkref(proxy)) return -1; - return PyObject_Length(PyWeakref_GET_OBJECT(proxy)); + + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + Py_ssize_t res = PyObject_Length(obj); + Py_DECREF(obj); + return res; } WRAP_BINARY(proxy_getitem, PyObject_GetItem) @@ -575,10 +617,16 @@ proxy_setitem(PyWeakReference *proxy, PyObject *key, PyObject *value) if (!proxy_checkref(proxy)) return -1; - if (value == NULL) - return PyObject_DelItem(PyWeakref_GET_OBJECT(proxy), key); - else - return PyObject_SetItem(PyWeakref_GET_OBJECT(proxy), key, value); + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + int res; + if (value == NULL) { + res = PyObject_DelItem(obj, key); + } else { + res = PyObject_SetItem(obj, key, value); + } + Py_DECREF(obj); + return res; } /* iterator slots */ @@ -588,7 +636,11 @@ proxy_iter(PyWeakReference *proxy) { if (!proxy_checkref(proxy)) return NULL; - return PyObject_GetIter(PyWeakref_GET_OBJECT(proxy)); + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + PyObject* res = PyObject_GetIter(obj); + Py_DECREF(obj); + return res; } static PyObject * @@ -596,7 +648,12 @@ proxy_iternext(PyWeakReference *proxy) { if (!proxy_checkref(proxy)) return NULL; - return PyIter_Next(PyWeakref_GET_OBJECT(proxy)); + + PyObject *obj = PyWeakref_GET_OBJECT(proxy); + Py_INCREF(obj); + PyObject* res = PyIter_Next(obj); + Py_DECREF(obj); + return res; } |