BUG: Fix memory leak for subclass slicing

seberg · charris · commit d98907be0793 · 2017-11-26T13:26:07.000-07:00
A slice object was created if __getslice__/__setslice__ was used by an inherited subclass (through python or directly) but not decref'd. Closes numpygh-10066
diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c
@@ -2444,7 +2444,7 @@ array_complex(PyArrayObject *self, PyObject *NPY_UNUSED(args))
 static PyObject *
 array_getslice(PyArrayObject *self, PyObject *args)
 {
-    PyObject *start, *stop, *slice;
+    PyObject *start, *stop, *slice, *result;
     if (!PyArg_ParseTuple(args, "OO:__getslice__", &start, &stop)) {
         return NULL;
     }
@@ -2455,7 +2455,9 @@ array_getslice(PyArrayObject *self, PyObject *args)
     }
 
     /* Deliberately delegate to subclasses */
-    return PyObject_GetItem((PyObject *)self, slice);
+    result = PyObject_GetItem((PyObject *)self, slice);
+    Py_DECREF(slice);
+    return result;
 }
 
 static PyObject *
@@ -2473,9 +2475,10 @@ array_setslice(PyArrayObject *self, PyObject *args)
 
     /* Deliberately delegate to subclasses */
     if (PyObject_SetItem((PyObject *)self, slice, value) < 0) {
+        Py_DECREF(slice);
         return NULL;
     }
-
+    Py_DECREF(slice);
     Py_RETURN_NONE;
 }
 
diff --git a/numpy/core/tests/test_indexing.py b/numpy/core/tests/test_indexing.py
@@ -10,7 +10,7 @@
 from itertools import product
 from numpy.testing import (
     TestCase, run_module_suite, assert_, assert_equal, assert_raises,
-    assert_array_equal, assert_warns, HAS_REFCOUNT
+    assert_array_equal, assert_warns, dec, HAS_REFCOUNT, suppress_warnings
 )
 
 
@@ -617,6 +617,54 @@ def __array_finalize__(self, old):
         assert_array_equal(new_s.old, s)
 
 class TestFancyIndexingCast(TestCase):
+    @dec.skipif(not HAS_REFCOUNT)
+    def test_slice_decref_getsetslice(self):
+        # See gh-10066, a temporary slice object should be discarted.
+        # This test is only really interesting on Python 2 since
+        # it goes through `__set/getslice__` here and can probably be
+        # removed. Use 0:7 to make sure it is never None:7.
+        class KeepIndexObject(np.ndarray):
+            def __getitem__(self, indx):
+                self.indx = indx
+                if indx == slice(0, 7):
+                    raise ValueError
+
+            def __setitem__(self, indx, val):
+                self.indx = indx
+                if indx == slice(0, 4):
+                    raise ValueError
+
+        k = np.array([1]).view(KeepIndexObject)
+        k[0:5]
+        assert_equal(k.indx, slice(0, 5))
+        assert_equal(sys.getrefcount(k.indx), 2)
+        try:
+            k[0:7]
+            raise AssertionError
+        except ValueError:
+            # The exception holds a reference to the slice so clear on Py2
+            if hasattr(sys, 'exc_clear'):
+                with suppress_warnings() as sup:
+                    sup.filter(DeprecationWarning)
+                    sys.exc_clear()
+        assert_equal(k.indx, slice(0, 7))
+        assert_equal(sys.getrefcount(k.indx), 2)
+
+        k[0:3] = 6
+        assert_equal(k.indx, slice(0, 3))
+        assert_equal(sys.getrefcount(k.indx), 2)
+        try:
+            k[0:4] = 2
+            raise AssertionError
+        except ValueError:
+            # The exception holds a reference to the slice so clear on Py2
+            if hasattr(sys, 'exc_clear'):
+                with suppress_warnings() as sup:
+                    sup.filter(DeprecationWarning)
+                    sys.exc_clear()
+        assert_equal(k.indx, slice(0, 4))
+        assert_equal(sys.getrefcount(k.indx), 2)
+
     def test_boolean_index_cast_assign(self):
         # Setup the boolean index and float arrays.
         shape = (8, 63)
@@ -1103,6 +1151,7 @@ def test_1d(self):
             for index in self.complex_indices:
                 self._check_single_index(a, index)
 
+
 class TestFloatNonIntegerArgument(TestCase):
     """
     These test that ``TypeError`` is raised when you try to use

Original file line number	Diff line number	Diff line change
`@@ -2444,7 +2444,7 @@ array_complex(PyArrayObject self, PyObject NPY_UNUSED(args))`
`2444`	`2444`	`static PyObject *`
`2445`	`2445`	`array_getslice(PyArrayObject self, PyObject args)`
`2446`	`2446`	`{`
`2447`		`- PyObject start, stop, *slice;`
	`2447`	`+ PyObject start, stop, slice, result;`
`2448`	`2448`	`if (!PyArg_ParseTuple(args, "OO:__getslice__", &start, &stop)) {`
`2449`	`2449`	`return NULL;`
`2450`	`2450`	`}`
`@@ -2455,7 +2455,9 @@ array_getslice(PyArrayObject self, PyObject args)`
`2455`	`2455`	`}`
`2456`	`2456`
`2457`	`2457`	`/* Deliberately delegate to subclasses */`
`2458`		`- return PyObject_GetItem((PyObject *)self, slice);`
	`2458`	`+ result = PyObject_GetItem((PyObject *)self, slice);`
	`2459`	`+ Py_DECREF(slice);`
	`2460`	`+ return result;`
`2459`	`2461`	`}`
`2460`	`2462`
`2461`	`2463`	`static PyObject *`
`@@ -2473,9 +2475,10 @@ array_setslice(PyArrayObject self, PyObject args)`
`2473`	`2475`
`2474`	`2476`	`/* Deliberately delegate to subclasses */`
`2475`	`2477`	`if (PyObject_SetItem((PyObject *)self, slice, value) < 0) {`
	`2478`	`+ Py_DECREF(slice);`
`2476`	`2479`	`return NULL;`
`2477`	`2480`	`}`
`2478`		`-`
	`2481`	`+ Py_DECREF(slice);`
`2479`	`2482`	`Py_RETURN_NONE;`
`2480`	`2483`	`}`
`2481`	`2484`