Skip to content
Open
49 changes: 49 additions & 0 deletions Lib/test/test_weakref.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,55 @@ class MyObj:
with self.assertRaises(TypeError):
hash(weakref.proxy(obj))

def _assert_no_proxy_refcount_leak(self, make_class, do_op, op_name):
"""Helper: verify a proxy operation doesn't leak."""
# Create dead proxy
o = make_class()
dead = weakref.proxy(o)
del o
gc.collect()

# Create live proxy
obj = make_class()
ref = weakref.ref(obj)
proxy = weakref.proxy(obj)

# run operation
try:
do_op(proxy, dead)
except ReferenceError:
pass
del proxy, obj, dead
gc.collect()

# verify
self.assertIsNone(ref(), f"Leaked object in '{op_name}' operation")

def test_proxy_unref_unary_refcount(self):
class C:
def __neg__(self): return 0
self._assert_no_proxy_refcount_leak(C,lambda p, d: operator.neg(d), "Unary")

def test_proxy_unref_binary_refcount(self):
class C:
def __add__(self, o): return NotImplemented
self._assert_no_proxy_refcount_leak(C, operator.add, "Binary")

def test_proxy_unref_ternary_refcount(self):
class C:
def __pow__(self, o, m=None): return NotImplemented
self._assert_no_proxy_refcount_leak(C, lambda p, d: pow(p, d, None), "Ternary")

def test_proxy_unref_richcompare_refcount(self):
class C:
def __eq__(self, o): return NotImplemented
self._assert_no_proxy_refcount_leak(C, lambda p, d: p == d, "Rich Compare")

def test_proxy_unref_wrapmethod_refcount(self):
class C:
def __repr__(self): return "C()"
self._assert_no_proxy_refcount_leak(C, lambda p, d: repr(d), "Wrap Method")

def test_getweakrefcount(self):
o = C()
ref1 = weakref.ref(o)
Expand Down
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need such a long description, usually just one sentence

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix reference count leak in weakref proxy operations when a dead proxy is encountered as the second argument.
116 changes: 76 additions & 40 deletions Objects/weakrefobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -530,26 +530,39 @@ proxy_check_ref(PyObject *obj)
return true;
}


/* If a parameter is a proxy, check that it is still "live" and wrap it,
* replacing the original value with the raw object. Raises ReferenceError
* if the param is a dead proxy.
/*
* Unwrap a proxy into a strong reference.
* - If `o` is a live proxy: replaces `o` with the underlying object
* (already Py_INCREF'd by _PyWeakref_GET_REF), sets *did_incref = 1.
* - If `o` is a dead proxy: sets ReferenceError, sets `o` = NULL,
* sets *did_incref = 0.
* - If `o` is not a proxy: Py_INCREF's it, sets *did_incref = 1.
* Returns 1 on success, 0 on dead proxy (caller must goto error).
*/
#define UNWRAP(o) \
if (PyWeakref_CheckProxy(o)) { \
o = _PyWeakref_GET_REF(o); \
if (!proxy_check_ref(o)) { \
return NULL; \
} \
} \
else { \
Py_INCREF(o); \
static inline int
_proxy_unwrap(PyObject **op, int *did_incref)
{
if (PyWeakref_CheckProxy(*op)) {
*op = _PyWeakref_GET_REF(*op);
if (!proxy_check_ref(*op)) {
*did_incref = 0;
return 0;
}
/* _PyWeakref_GET_REF already returned a strong ref */
}
else {
Py_INCREF(*op);
}
*did_incref = 1;
return 1;
}

#define WRAP_UNARY(method, generic) \
static PyObject * \
method(PyObject *proxy) { \
UNWRAP(proxy); \
int proxy_incref = 0; \
if (!_proxy_unwrap(&proxy, &proxy_incref)) \
return NULL; \
PyObject* res = generic(proxy); \
Py_DECREF(proxy); \
return res; \
Expand All @@ -558,12 +571,19 @@ proxy_check_ref(PyObject *obj)
#define WRAP_BINARY(method, generic) \
static PyObject * \
method(PyObject *x, PyObject *y) { \
UNWRAP(x); \
UNWRAP(y); \
PyObject* res = generic(x, y); \
Py_DECREF(x); \
Py_DECREF(y); \
return res; \
int x_incref = 0, y_incref = 0; \
if (!_proxy_unwrap(&x, &x_incref)) goto clean_up; \
if (!_proxy_unwrap(&y, &y_incref)) goto clean_up; \
{ \
PyObject* res = generic(x, y); \
Py_DECREF(x); \
Py_DECREF(y); \
return res; \
} \
clean_up: \
if (x_incref) Py_DECREF(x); \
if (y_incref) Py_DECREF(y); \
return NULL; \
}

/* Note that the third arg needs to be checked for NULL since the tp_call
Expand All @@ -572,27 +592,36 @@ proxy_check_ref(PyObject *obj)
#define WRAP_TERNARY(method, generic) \
static PyObject * \
method(PyObject *proxy, PyObject *v, PyObject *w) { \
UNWRAP(proxy); \
UNWRAP(v); \
int proxy_incref = 0, v_incref = 0, w_incref = 0; \
if (!_proxy_unwrap(&proxy, &proxy_incref)) goto clean_up; \
if (!_proxy_unwrap(&v, &v_incref)) goto clean_up; \
if (w != NULL) { \
UNWRAP(w); \
if (!_proxy_unwrap(&w, &w_incref)) goto clean_up; \
} \
PyObject* res = generic(proxy, v, w); \
Py_DECREF(proxy); \
Py_DECREF(v); \
Py_XDECREF(w); \
return res; \
{ \
PyObject* res = generic(proxy, v, w); \
Py_DECREF(proxy); \
Py_DECREF(v); \
Py_XDECREF(w); \
return res; \
} \
clean_up: \
if (proxy_incref) Py_DECREF(proxy); \
if (v_incref) Py_DECREF(v); \
if (w_incref) Py_DECREF(w); \
return NULL; \
}

#define WRAP_METHOD(method, SPECIAL) \
static PyObject * \
method(PyObject *proxy, PyObject *Py_UNUSED(ignored)) { \
UNWRAP(proxy); \
PyObject* res = PyObject_CallMethodNoArgs(proxy, &_Py_ID(SPECIAL)); \
Py_DECREF(proxy); \
return res; \
}

int proxy_incref = 0; \
if (!_proxy_unwrap(&proxy, &proxy_incref)) \
return NULL; \
PyObject* res = PyObject_CallMethodNoArgs(proxy, &_Py_ID(SPECIAL)); \
Py_DECREF(proxy); \
return res; \
}

/* direct slots */

Expand Down Expand Up @@ -635,12 +664,19 @@ proxy_setattr(PyObject *proxy, PyObject *name, PyObject *value)
static PyObject *
proxy_richcompare(PyObject *proxy, PyObject *v, int op)
{
UNWRAP(proxy);
UNWRAP(v);
PyObject* res = PyObject_RichCompare(proxy, v, op);
Py_DECREF(proxy);
Py_DECREF(v);
return res;
int proxy_incref = 0, v_incref = 0;
if (!_proxy_unwrap(&proxy, &proxy_incref)) goto clean_up;
if (!_proxy_unwrap(&v, &v_incref)) goto clean_up;
{
PyObject* res = PyObject_RichCompare(proxy, v, op);
Py_DECREF(proxy);
Py_DECREF(v);
return res;
}
clean_up:
if (proxy_incref) Py_DECREF(proxy);
if (v_incref) Py_DECREF(v);
return NULL;
}

/* number slots */
Expand Down
Loading