diff options
| -rw-r--r-- | doc/release/upcoming_changes/17582.new_feature.rst | 2 | ||||
| -rw-r--r-- | doc/source/reference/c-api/array.rst | 7 | ||||
| -rw-r--r-- | doc/source/reference/c-api/data_memory.rst | 37 | ||||
| -rw-r--r-- | doc/source/reference/global_state.rst | 10 | ||||
| -rw-r--r-- | numpy/core/src/multiarray/arrayobject.c | 11 | ||||
| -rw-r--r-- | numpy/core/tests/test_mem_policy.py | 49 |
6 files changed, 105 insertions, 11 deletions
diff --git a/doc/release/upcoming_changes/17582.new_feature.rst b/doc/release/upcoming_changes/17582.new_feature.rst index be5997dda..c2426330c 100644 --- a/doc/release/upcoming_changes/17582.new_feature.rst +++ b/doc/release/upcoming_changes/17582.new_feature.rst @@ -3,6 +3,8 @@ NEP 49 configurable allocators As detailed in `NEP 49`_, the function used for allocation of the data segment of a ndarray can be changed. The policy can be set globally or in a context. For more information see the NEP and the :ref:`data_memory` reference docs. +Also add a ``NUMPY_WARN_IF_NO_MEM_POLICY`` override to warn on dangerous use +of transfering ownership by setting ``NPY_ARRAY_OWNDATA``. .. _`NEP 49`: https://numpy.org/neps/nep-0049.html diff --git a/doc/source/reference/c-api/array.rst b/doc/source/reference/c-api/array.rst index 6a135fd71..232690486 100644 --- a/doc/source/reference/c-api/array.rst +++ b/doc/source/reference/c-api/array.rst @@ -325,8 +325,7 @@ From scratch should be increased after the pointer is passed in, and the base member of the returned ndarray should point to the Python object that owns the data. This will ensure that the provided memory is not - freed while the returned array is in existence. To free memory as soon - as the ndarray is deallocated, set the OWNDATA flag on the returned ndarray. + freed while the returned array is in existence. .. c:function:: PyObject* PyArray_SimpleNewFromDescr( \ int nd, npy_int const* dims, PyArray_Descr* descr) @@ -1463,7 +1462,9 @@ of the constant names is deprecated in 1.7. .. c:macro:: NPY_ARRAY_OWNDATA - The data area is owned by this array. + The data area is owned by this array. Should never be set manually, instead + create a ``PyObject`` wrapping the data and set the array's base to that + object. For an example, see the test in ``test_mem_policy``. .. c:macro:: NPY_ARRAY_ALIGNED diff --git a/doc/source/reference/c-api/data_memory.rst b/doc/source/reference/c-api/data_memory.rst index c17f98a2c..11a37adc4 100644 --- a/doc/source/reference/c-api/data_memory.rst +++ b/doc/source/reference/c-api/data_memory.rst @@ -119,3 +119,40 @@ For an example of setting up and using the PyDataMem_Handler, see the test in operations that might cause new allocation events (such as the creation/destruction numpy objects, or creating/destroying Python objects which might cause a gc) + +What happens when deallocating if there is no policy set +-------------------------------------------------------- + +A rare but useful technique is to allocate a buffer outside NumPy, use +:c:func:`PyArray_NewFromDescr` to wrap the buffer in a ``ndarray``, then switch +the ``OWNDATA`` flag to true. When the ``ndarray`` is released, the +appropriate function from the ``ndarray``'s ``PyDataMem_Handler`` should be +called to free the buffer. But the ``PyDataMem_Handler`` field was never set, +it will be ``NULL``. For backward compatibility, NumPy will call ``free()`` to +release the buffer. If ``NUMPY_WARN_IF_NO_MEM_POLICY`` is set to ``1``, a +warning will be emitted. The current default is not to emit a warning, this may +change in a future version of NumPy. + +A better technique would be to use a ``PyCapsule`` as a base object: + +.. code-block:: c + + /* define a PyCapsule_Destructor, using the correct deallocator for buff */ + void free_wrap(void *capsule){ + void * obj = PyCapsule_GetPointer(capsule, PyCapsule_GetName(capsule)); + free(obj); + }; + + /* then inside the function that creates arr from buff */ + ... + arr = PyArray_NewFromDescr(... buf, ...); + if (arr == NULL) { + return NULL; + } + capsule = PyCapsule_New(buf, "my_wrapped_buffer", + (PyCapsule_Destructor)&free_wrap); + if (PyArray_SetBaseObject(arr, capsule) == -1) { + Py_DECREF(arr); + return NULL; + } + ... diff --git a/doc/source/reference/global_state.rst b/doc/source/reference/global_state.rst index f18481235..20874ceaa 100644 --- a/doc/source/reference/global_state.rst +++ b/doc/source/reference/global_state.rst @@ -84,3 +84,13 @@ contiguous in memory. Most users will have no reason to change these; for details see the :ref:`memory layout <memory-layout>` documentation. + +Warn if no memory allocation policy when deallocating data +---------------------------------------------------------- + +Some users might pass ownership of the data pointer to the ``ndarray`` by +setting the ``OWNDATA`` flag. If they do this without setting (manually) a +memory allocation policy, the default will be to call ``free``. If +``NUMPY_WARN_IF_NO_MEM_POLICY`` is set to ``"1"``, a ``RuntimeWarning`` will +be emitted. A better alternative is to use a ``PyCapsule`` with a deallocator +and set the ``ndarray.base``. diff --git a/numpy/core/src/multiarray/arrayobject.c b/numpy/core/src/multiarray/arrayobject.c index c3615f95f..237fb9b72 100644 --- a/numpy/core/src/multiarray/arrayobject.c +++ b/numpy/core/src/multiarray/arrayobject.c @@ -502,10 +502,13 @@ array_dealloc(PyArrayObject *self) nbytes = fa->descr->elsize ? fa->descr->elsize : 1; } if (fa->mem_handler == NULL) { - char const * msg = "Trying to dealloc data, but a memory policy " - "is not set. If you take ownership of the data, you must " - "also set a memory policy."; - WARN_IN_DEALLOC(PyExc_RuntimeWarning, msg); + char *env = getenv("NUMPY_WARN_IF_NO_MEM_POLICY"); + if ((env == NULL) || (strncmp(env, "1", 1) == 0)) { + char const * msg = "Trying to dealloc data, but a memory policy " + "is not set. If you take ownership of the data, you must " + "set a base owning the data (e.g. a PyCapsule)."; + WARN_IN_DEALLOC(PyExc_RuntimeWarning, msg); + } // Guess at malloc/free ??? free(fa->data); } else { diff --git a/numpy/core/tests/test_mem_policy.py b/numpy/core/tests/test_mem_policy.py index a2ff98ceb..4ec9b3b11 100644 --- a/numpy/core/tests/test_mem_policy.py +++ b/numpy/core/tests/test_mem_policy.py @@ -1,5 +1,6 @@ import asyncio import gc +import os import pytest import numpy as np import threading @@ -61,6 +62,29 @@ def get_module(tmp_path): // PyArray_BASE(PyArrayObject *)args) = NULL; Py_RETURN_NONE; """), + ("get_array_with_base", "METH_NOARGS", """ + char *buf = (char *)malloc(20); + npy_intp dims[1]; + dims[0] = 20; + PyArray_Descr *descr = PyArray_DescrNewFromType(NPY_UINT8); + PyObject *arr = PyArray_NewFromDescr(&PyArray_Type, descr, 1, dims, + NULL, buf, + NPY_ARRAY_WRITEABLE, NULL); + if (arr == NULL) return NULL; + PyObject *obj = PyCapsule_New(buf, "buf capsule", + (PyCapsule_Destructor)&warn_on_free); + if (obj == NULL) { + Py_DECREF(arr); + return NULL; + } + if (PyArray_SetBaseObject((PyArrayObject *)arr, obj) < 0) { + Py_DECREF(arr); + Py_DECREF(obj); + return NULL; + } + return arr; + + """), ] prologue = ''' #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION @@ -163,6 +187,12 @@ def get_module(tmp_path): shift_free /* free */ } }; + void warn_on_free(void *capsule) { + PyErr_WarnEx(PyExc_UserWarning, "in warn_on_free", 1); + void * obj = PyCapsule_GetPointer(capsule, + PyCapsule_GetName(capsule)); + free(obj); + }; ''' more_init = "import_array();" try: @@ -331,10 +361,21 @@ def test_switch_owner(get_module): a = get_module.get_array() assert np.core.multiarray.get_handler_name(a) is None get_module.set_own(a) - with warnings.catch_warnings(): - warnings.filterwarnings('always') - # The policy should be NULL, so we have to assume we can call "free" + oldval = os.environ.get('NUMPY_WARN_IF_NO_MEM_POLICY', None) + os.environ['NUMPY_WARN_IF_NO_MEM_POLICY'] = "1" + try: + # The policy should be NULL, so we have to assume we can call + # "free" with assert_warns(RuntimeWarning) as w: del a gc.collect() - print(w) + finally: + if oldval is None: + os.environ.pop('NUMPY_WARN_IF_NO_MEM_POLICY') + else: + os.environ['NUMPY_WARN_IF_NO_MEM_POLICY'] = oldval + + a = get_module.get_array_with_base() + with pytest.warns(UserWarning, match='warn_on_free'): + del a + gc.collect() |