Author: Devin Jeanpierre <[email protected]>
Branch:
Changeset: r83922:4c00f63f3fa2
Date: 2016-04-25 13:02 -0700
http://bitbucket.org/pypy/pypy/changeset/4c00f63f3fa2/
Log: Correctly leak replaced object in PyList_SET_ITEM.
diff --git a/pypy/module/cpyext/include/listobject.h
b/pypy/module/cpyext/include/listobject.h
--- a/pypy/module/cpyext/include/listobject.h
+++ b/pypy/module/cpyext/include/listobject.h
@@ -1,2 +1,1 @@
-#define PyList_GET_ITEM PyList_GetItem
#define PyList_SET_ITEM PyList_SetItem
diff --git a/pypy/module/cpyext/listobject.py b/pypy/module/cpyext/listobject.py
--- a/pypy/module/cpyext/listobject.py
+++ b/pypy/module/cpyext/listobject.py
@@ -3,7 +3,7 @@
from pypy.module.cpyext.api import (cpython_api, CANNOT_FAIL, Py_ssize_t,
build_type_checkers)
from pypy.module.cpyext.pyerrors import PyErr_BadInternalCall
-from pypy.module.cpyext.pyobject import Py_DecRef, PyObject
+from pypy.module.cpyext.pyobject import Py_DecRef, PyObject, make_ref
from pypy.objspace.std.listobject import W_ListObject
from pypy.interpreter.error import OperationError
@@ -21,6 +21,24 @@
"""
return space.newlist([None] * len)
+@cpython_api([PyObject, Py_ssize_t, PyObject], rffi.INT_real,
error=CANNOT_FAIL)
+def PyList_SET_ITEM(space, w_list, index, w_item):
+ """Set the item at index index in list to item. Return 0 on success
+ or -1 on failure.
+
+ This function "steals" a reference to item, and, unlike PyList_SetItem(),
+ does not discard a reference to any item that it being replaced; any
+ reference in list at position i will be leaked.
+ """
+ assert isinstance(w_list, W_ListObject)
+ assert 0 <= index < w_list.length
+ Py_DecRef(space, w_item)
+ # Deliberately leak, so that it can be safely decref'd.
+ make_ref(space, w_list.getitem(index))
+ w_list.setitem(index, w_item)
+ return 0
+
+
@cpython_api([PyObject, Py_ssize_t, PyObject], rffi.INT_real, error=-1)
def PyList_SetItem(space, w_list, index, w_item):
"""Set the item at index index in list to item. Return 0 on success
diff --git a/pypy/module/cpyext/test/test_listobject.py
b/pypy/module/cpyext/test/test_listobject.py
--- a/pypy/module/cpyext/test/test_listobject.py
+++ b/pypy/module/cpyext/test/test_listobject.py
@@ -136,3 +136,28 @@
l = [1, 2, 3]
module.setlistitem(l,0)
assert l == [None, 2, 3]
+
+ def test_set_item_macro(self):
+ """PyList_SET_ITEM leaks a reference to the target."""
+ module = self.import_extension('foo', [
+ ("test_refcount_diff_after_setitem", "METH_NOARGS",
+ """
+ PyObject* o = PyList_New(0);
+ PyObject* o2 = PyList_New(0);
+
+ PyList_Append(o, o2); // does not steal o2
+
+ Py_ssize_t refcount = Py_REFCNT(o2);
+
+ // Steal a reference to o2, but leak the old reference to o2.
+ // The net result should be no change in refcount.
+ PyList_SET_ITEM(o, 0, o2);
+
+ Py_ssize_t new_refcount = Py_REFCNT(o2);
+
+ Py_CLEAR(o);
+ Py_DECREF(o2); // append incref'd.
+ // Py_CLEAR(o2); // naive implementation would fail here.
+ return PyLong_FromSsize_t(new_refcount - refcount);
+ """)])
+ assert module.test_refcount_diff_after_setitem() == 0
_______________________________________________
pypy-commit mailing list
[email protected]
https://mail.python.org/mailman/listinfo/pypy-commit
