Skip to content

Commit 2d4adac

Browse files
[3.15] gh-159739: Release weak proxy operands on errors (GH-159968) (#160018)
(cherry picked from commit ebce4b7) Co-authored-by: Jelle Zijlstra <jelle@openai.com>
1 parent 8411879 commit 2d4adac

3 files changed

Lines changed: 55 additions & 15 deletions

File tree

‎Lib/test/test_weakref.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,20 @@ class F(float):
605605
self.assertEqual(p + 1.0, 3.0)
606606
self.assertEqual(1.0 + p, 3.0) # this used to SEGV
607607

608+
def test_dead_proxy_operand_does_not_leak(self):
609+
dead = weakref.proxy(Object(2))
610+
gc_collect()
611+
for operation in (operator.add, operator.eq, lambda a, b: pow(a, 2, b)):
612+
with self.subTest(operation=operation):
613+
live = Object(1)
614+
ref = weakref.ref(live)
615+
proxy = weakref.proxy(live)
616+
with self.assertRaises(ReferenceError):
617+
operation(proxy, dead)
618+
del live
619+
gc_collect()
620+
self.assertIsNone(ref())
621+
608622
def test_callbacks_protected(self):
609623
# Callbacks protected from already-set exceptions?
610624
# Regression test for SF bug #478534.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a reference leak in binary, ternary, and comparison operations on weak
2+
reference proxies when another operand is a dead proxy.

‎Objects/weakrefobject.c‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -536,19 +536,26 @@ proxy_check_ref(PyObject *obj)
536536
}
537537

538538

539-
/* If a parameter is a proxy, check that it is still "live" and wrap it,
540-
* replacing the original value with the raw object. Raises ReferenceError
541-
* if the param is a dead proxy.
539+
/* Return a new reference to a live proxy's referent, or to the object itself
540+
* if it is not a proxy. Raise ReferenceError for a dead proxy.
542541
*/
542+
static PyObject *
543+
proxy_unwrap(PyObject *obj)
544+
{
545+
if (PyWeakref_CheckProxy(obj)) {
546+
PyObject *referent = _PyWeakref_GET_REF(obj);
547+
if (!proxy_check_ref(referent)) {
548+
return NULL;
549+
}
550+
return referent;
551+
}
552+
return Py_NewRef(obj);
553+
}
554+
543555
#define UNWRAP(o) \
544-
if (PyWeakref_CheckProxy(o)) { \
545-
o = _PyWeakref_GET_REF(o); \
546-
if (!proxy_check_ref(o)) { \
547-
return NULL; \
548-
} \
549-
} \
550-
else { \
551-
Py_INCREF(o); \
556+
o = proxy_unwrap(o); \
557+
if (o == NULL) { \
558+
return NULL; \
552559
}
553560

554561
#define WRAP_UNARY(method, generic) \
@@ -564,7 +571,11 @@ proxy_check_ref(PyObject *obj)
564571
static PyObject * \
565572
method(PyObject *x, PyObject *y) { \
566573
UNWRAP(x); \
567-
UNWRAP(y); \
574+
y = proxy_unwrap(y); \
575+
if (y == NULL) { \
576+
Py_DECREF(x); \
577+
return NULL; \
578+
} \
568579
PyObject* res = generic(x, y); \
569580
Py_DECREF(x); \
570581
Py_DECREF(y); \
@@ -578,9 +589,18 @@ proxy_check_ref(PyObject *obj)
578589
static PyObject * \
579590
method(PyObject *proxy, PyObject *v, PyObject *w) { \
580591
UNWRAP(proxy); \
581-
UNWRAP(v); \
592+
v = proxy_unwrap(v); \
593+
if (v == NULL) { \
594+
Py_DECREF(proxy); \
595+
return NULL; \
596+
} \
582597
if (w != NULL) { \
583-
UNWRAP(w); \
598+
w = proxy_unwrap(w); \
599+
if (w == NULL) { \
600+
Py_DECREF(proxy); \
601+
Py_DECREF(v); \
602+
return NULL; \
603+
} \
584604
} \
585605
PyObject* res = generic(proxy, v, w); \
586606
Py_DECREF(proxy); \
@@ -641,7 +661,11 @@ static PyObject *
641661
proxy_richcompare(PyObject *proxy, PyObject *v, int op)
642662
{
643663
UNWRAP(proxy);
644-
UNWRAP(v);
664+
v = proxy_unwrap(v);
665+
if (v == NULL) {
666+
Py_DECREF(proxy);
667+
return NULL;
668+
}
645669
PyObject* res = PyObject_RichCompare(proxy, v, op);
646670
Py_DECREF(proxy);
647671
Py_DECREF(v);

0 commit comments

Comments
 (0)