Merge pull request #12572 from eric-wieser/fix-9647

BUG: Make `arr.ctypes.data` hold onto a reference to the underlying array

5 files changed, 133 insertions, 26 deletions

diff --git a/numpy/core/_add_newdocs.py b/numpy/core/_add_newdocs.py
index 80fa1466e..513415e09 100644
--- a/numpy/core/_add_newdocs.py
+++ b/numpy/core/_add_newdocs.py
@@ -2042,14 +2042,6 @@ add_newdoc('numpy.core.multiarray', 'ndarray', ('ctypes',
 
     .. automethod:: numpy.core._internal._ctypes.strides_as
 
-    Be careful using the ctypes attribute - especially on temporary
-    arrays or arrays constructed on the fly. For example, calling
-    ``(a+b).ctypes.data_as(ctypes.c_void_p)`` returns a pointer to memory
-    that is invalid because the array created as (a+b) is deallocated
-    before the next Python statement. You can avoid this problem using
-    either ``c=a+b`` or ``ct=(a+b).ctypes``. In the latter case, ct will
-    hold a reference to the array until ct is deleted or re-assigned.
-
     If the ctypes module is not available, then the ctypes attribute
     of array objects still returns something useful, but ctypes objects
     are not returned and errors may be raised instead. In particular,
diff --git a/numpy/core/_internal.py b/numpy/core/_internal.py
index 59da60253..27a3deeda 100644
--- a/numpy/core/_internal.py
+++ b/numpy/core/_internal.py
@@ -238,19 +238,68 @@ _getintp_ctype.cache = None
 
 class _missing_ctypes(object):
     def cast(self, num, obj):
-        return num
+        return num.value
+
+    class c_void_p(object):
+        def __init__(self, ptr):
+            self.value = ptr
+
+
+class _unsafe_first_element_pointer(object):
+    """
+    Helper to allow viewing an array as a ctypes pointer to the first element
+
+    This avoids:
+      * dealing with strides
+      * `.view` rejecting object-containing arrays
+      * `memoryview` not supporting overlapping fields
+    """
+    def __init__(self, arr):
+        self.base = arr
+
+    @property
+    def __array_interface__(self):
+        i = dict(
+            shape=(),
+            typestr='|V0',
+            data=(self.base.__array_interface__['data'][0], False),
+            strides=(),
+            version=3,
+        )
+        return i
+
+
+def _get_void_ptr(arr):
+    """
+    Get a `ctypes.c_void_p` to arr.data, that keeps a reference to the array
+    """
+    import numpy as np
+    # convert to a 0d array that has a data pointer referrign to the start
+    # of arr. This holds a reference to arr.
+    simple_arr = np.asarray(_unsafe_first_element_pointer(arr))
+
+    # create a `char[0]` using the same memory.
+    c_arr = (ctypes.c_char * 0).from_buffer(simple_arr)
+
+    # finally cast to void*
+    return ctypes.cast(ctypes.pointer(c_arr), ctypes.c_void_p)
 
-    def c_void_p(self, num):
-        return num
 
 class _ctypes(object):
     def __init__(self, array, ptr=None):
+        self._arr = array
+
         if ctypes:
             self._ctypes = ctypes
+            # get a void pointer to the buffer, which keeps the array alive
+            self._data = _get_void_ptr(array)
+            assert self._data.value == ptr
         else:
+            # fake a pointer-like object that holds onto the reference
             self._ctypes = _missing_ctypes()
-        self._arr = array
-        self._data = ptr
+            self._data = self._ctypes.c_void_p(ptr)
+            self._data._objects = array
+
         if self._arr.ndim == 0:
             self._zerod = True
         else:
@@ -263,6 +312,8 @@ class _ctypes(object):
         ``self.data_as(ctypes.c_void_p)``. Perhaps you want to use the data as a
         pointer to a ctypes array of floating-point data:
         ``self.data_as(ctypes.POINTER(ctypes.c_double))``.
+
+        The returned pointer will keep a reference to the array.
         """
         return self._ctypes.cast(self._data, obj)
 
@@ -284,7 +335,8 @@ class _ctypes(object):
             return None
         return (obj*self._arr.ndim)(*self._arr.strides)
 
-    def get_data(self):
+    @property
+    def data(self):
         """
         A pointer to the memory area of the array as a Python integer.
         This memory area may contain data that is not aligned, or not in correct
@@ -293,10 +345,16 @@ class _ctypes(object):
         attribute to arbitrary C-code to avoid trouble that can include Python
         crashing. User Beware! The value of this attribute is exactly the same
         as ``self._array_interface_['data'][0]``.
+
+        Note that unlike `data_as`, a reference will not be kept to the array:
+        code like ``ctypes.c_void_p((a + b).ctypes.data)`` will result in a
+        pointer to a deallocated array, and should be spelt
+        ``(a + b).ctypes.data_as(ctypes.c_void_p)``
         """
-        return self._data
+        return self._data.value
 
-    def get_shape(self):
+    @property
+    def shape(self):
         """
         (c_intp*self.ndim): A ctypes array of length self.ndim where
         the basetype is the C-integer corresponding to ``dtype('p')`` on this
@@ -307,7 +365,8 @@ class _ctypes(object):
         """
         return self.shape_as(_getintp_ctype())
 
-    def get_strides(self):
+    @property
+    def strides(self):
         """
         (c_intp*self.ndim): A ctypes array of length self.ndim where
         the basetype is the same as for the shape attribute. This ctypes array
@@ -317,13 +376,20 @@ class _ctypes(object):
         """
         return self.strides_as(_getintp_ctype())
 
-    def get_as_parameter(self):
-        return self._ctypes.c_void_p(self._data)
+    @property
+    def _as_parameter_(self):
+        """
+        Overrides the ctypes semi-magic method
+
+        Enables `c_func(some_array.ctypes)`
+        """
+        return self._data
 
-    data = property(get_data)
-    shape = property(get_shape)
-    strides = property(get_strides)
-    _as_parameter_ = property(get_as_parameter, None, doc="_as parameter_")
+    # kept for compatibility
+    get_data = data.fget
+    get_shape = shape.fget
+    get_strides = strides.fget
+    get_as_parameter = _as_parameter_.fget
 
 
 def _newnames(datatype, order):
diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c
index 23a8dcea2..f77e414da 100644
--- a/numpy/core/src/multiarray/ctors.c
+++ b/numpy/core/src/multiarray/ctors.c
@@ -2024,7 +2024,7 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
         newtype = oldtype;
         Py_INCREF(oldtype);
     }
-    if (PyDataType_ISUNSIZED(newtype)) {
+    else if (PyDataType_ISUNSIZED(newtype)) {
         PyArray_DESCR_REPLACE(newtype);
         if (newtype == NULL) {
             return NULL;
diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
index 32d49df02..951c01c6d 100644
--- a/numpy/core/tests/test_multiarray.py
+++ b/numpy/core/tests/test_multiarray.py
@@ -7626,6 +7626,55 @@ class TestCTypes(object):
         finally:
             _internal.ctypes = ctypes
 
+    def _make_readonly(x):
+        x.flags.writeable = False
+        return x
+
+    @pytest.mark.parametrize('arr', [
+        np.array([1, 2, 3]),
+        np.array([['one', 'two'], ['three', 'four']]),
+        np.array((1, 2), dtype='i4,i4'),
+        np.zeros((2,), dtype=
+            np.dtype(dict(
+                formats=['<i4', '<i4'],
+                names=['a', 'b'],
+                offsets=[0, 2],
+                itemsize=6
+            ))
+        ),
+        np.array([None], dtype=object),
+        np.array([]),
+        np.empty((0, 0)),
+        _make_readonly(np.array([1, 2, 3])),
+    ], ids=[
+        '1d',
+        '2d',
+        'structured',
+        'overlapping',
+        'object',
+        'empty',
+        'empty-2d',
+        'readonly'
+    ])
+    def test_ctypes_data_as_holds_reference(self, arr):
+        # gh-9647
+        # create a copy to ensure that pytest does not mess with the refcounts
+        arr = arr.copy()
+
+        arr_ref = weakref.ref(arr)
+
+        ctypes_ptr = arr.ctypes.data_as(ctypes.c_void_p)
+
+        # `ctypes_ptr` should hold onto `arr`
+        del arr
+        gc.collect()
+        assert_(arr_ref() is not None, "ctypes pointer did not hold onto a reference")
+
+        # but when the `ctypes_ptr` object dies, so should `arr`
+        del ctypes_ptr
+        gc.collect()
+        assert_(arr_ref() is None, "unknowable whether ctypes pointer holds a reference")
+
 
 class TestWritebackIfCopy(object):
     # all these tests use the WRITEBACKIFCOPY mechanism
diff --git a/numpy/ma/tests/test_core.py b/numpy/ma/tests/test_core.py
index 2775b11ec..e0dbf1b1a 100644
--- a/numpy/ma/tests/test_core.py
+++ b/numpy/ma/tests/test_core.py
@@ -2401,9 +2401,9 @@ class TestMaskedArrayInPlaceArithmetics(object):
         assert_equal(xm, y + 1)
 
         (x, _, xm) = self.floatdata
-        id1 = x.data.ctypes._data
+        id1 = x.data.ctypes.data
         x += 1.
-        assert_(id1 == x.data.ctypes._data)
+        assert_(id1 == x.data.ctypes.data)
         assert_equal(x, y + 1.)
 
     def test_inplace_addition_array(self):