Skip to content

Commit 4ad5626

Browse files
committed
bpo-38395: Fix ownership in weakref.proxy methods
The implementation of weakref.proxy's methods call back into the Python API using a borrowed references of the weakly referenced object (acquired via PyWeakref_GET_OBJECT). This API call may delete the last reference to the object (either directly or via GC), leaving a dangling pointer, which can be subsequently dereferenced. To fix this, claim temporarily ownership of the referenced object when calling the appropiate method. Some functions because at the moment they do not need to access the borrowed referent, but to protect against future changes to these functions, ownership needs to be fixed in all potentially affected methids.
1 parent 36e33c3 commit 4ad5626

File tree

3 files changed

+58
-10
lines changed

3 files changed

+58
-10
lines changed

Lib/test/test_weakref.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,23 @@ class List(list): pass
391391
lyst = List()
392392
self.assertEqual(bool(weakref.proxy(lyst)), bool(lyst))
393393

394+
def test_proxy_iter(self):
395+
# Test fails with a debug build of the interpreter
396+
# (see bpo-38395).
397+
398+
obj = None
399+
400+
class MyObj:
401+
def __iter__(self):
402+
nonlocal obj
403+
del obj
404+
return NotImplemented
405+
406+
obj = MyObj()
407+
p = weakref.proxy(obj)
408+
with self.assertRaises(TypeError):
409+
"blech" in p
410+
394411
def test_getweakrefcount(self):
395412
o = C()
396413
ref1 = weakref.ref(o)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :class:`weakref.proxy` objects due to incorrect lifetime
2+
management when calling some associated methods that may delete the last
3+
reference to object being referenced by the proxy. Patch by Pablo Galindo.

Objects/weakrefobject.c

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,11 @@ proxy_setattr(PyWeakReference *proxy, PyObject *name, PyObject *value)
481481
{
482482
if (!proxy_checkref(proxy))
483483
return -1;
484-
return PyObject_SetAttr(PyWeakref_GET_OBJECT(proxy), name, value);
484+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
485+
Py_INCREF(obj);
486+
int res = PyObject_SetAttr(obj, name, value);
487+
Py_DECREF(obj);
488+
return res;
485489
}
486490

487491
static PyObject *
@@ -553,9 +557,13 @@ proxy_contains(PyWeakReference *proxy, PyObject *value)
553557
{
554558
if (!proxy_checkref(proxy))
555559
return -1;
556-
return PySequence_Contains(PyWeakref_GET_OBJECT(proxy), value);
557-
}
558560

561+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
562+
Py_INCREF(obj);
563+
int res = PySequence_Contains(obj, value);
564+
Py_DECREF(obj);
565+
return res;
566+
}
559567

560568
/* mapping slots */
561569

@@ -564,21 +572,32 @@ proxy_length(PyWeakReference *proxy)
564572
{
565573
if (!proxy_checkref(proxy))
566574
return -1;
567-
return PyObject_Length(PyWeakref_GET_OBJECT(proxy));
575+
576+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
577+
Py_INCREF(obj);
578+
Py_ssize_t res = PyObject_Length(obj);
579+
Py_DECREF(obj);
580+
return res;
568581
}
569582

570583
WRAP_BINARY(proxy_getitem, PyObject_GetItem)
571584

572585
static int
573586
proxy_setitem(PyWeakReference *proxy, PyObject *key, PyObject *value)
574587
{
588+
int res = 0;
575589
if (!proxy_checkref(proxy))
576590
return -1;
577591

578-
if (value == NULL)
579-
return PyObject_DelItem(PyWeakref_GET_OBJECT(proxy), key);
580-
else
581-
return PyObject_SetItem(PyWeakref_GET_OBJECT(proxy), key, value);
592+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
593+
Py_INCREF(obj);
594+
if (value == NULL) {
595+
res = PyObject_DelItem(obj, key);
596+
} else {
597+
res = PyObject_SetItem(obj, key, value);
598+
}
599+
Py_DECREF(obj);
600+
return res;
582601
}
583602

584603
/* iterator slots */
@@ -588,15 +607,24 @@ proxy_iter(PyWeakReference *proxy)
588607
{
589608
if (!proxy_checkref(proxy))
590609
return NULL;
591-
return PyObject_GetIter(PyWeakref_GET_OBJECT(proxy));
610+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
611+
Py_INCREF(obj);
612+
PyObject* res = PyObject_GetIter(obj);
613+
Py_DECREF(obj);
614+
return res;
592615
}
593616

594617
static PyObject *
595618
proxy_iternext(PyWeakReference *proxy)
596619
{
597620
if (!proxy_checkref(proxy))
598621
return NULL;
599-
return PyIter_Next(PyWeakref_GET_OBJECT(proxy));
622+
623+
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
624+
Py_INCREF(obj);
625+
PyObject* res = PyIter_Next(obj);
626+
Py_DECREF(obj);
627+
return res;
600628
}
601629

602630

0 commit comments

Comments
 (0)