Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Comment thread
prakashsellathurai marked this conversation as resolved.
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