diff options
author | Sebastian Berg <sebastian@sipsolutions.net> | 2020-07-23 11:14:38 -0500 |
---|---|---|
committer | Sebastian Berg <sebastian@sipsolutions.net> | 2020-10-19 18:33:40 -0500 |
commit | 028a61af17b2b3d960323fc0a9595bdfd91b9a7e (patch) | |
tree | 7a7e5905d21f846a1db0115137c491388e3b2fed | |
parent | aebaa8d2dbff7af620a2fca3b5ca9fbb08d4c60c (diff) | |
download | numpy-028a61af17b2b3d960323fc0a9595bdfd91b9a7e.tar.gz |
BUG: Fix memory leak of buffer-info cache due to relaxed strides
When relaxed strides is active (and has an effect), we recalculate
the strides to export "clean" strides in the buffer interface.
(Python and probably some other exporters expect this, i.e. NumPy
has fully switched to and embraced relaxed strides, but the buffer
interface at large probably not.)
The place where "fixing" the strides occured however meant that
when the strides are fixed, the old, cached buffer-info was not
reused when it should have been reused.
This moves the "fixing" logic so that reuse will occur. It leaves
one issue open in that an array shaped e.g. `(1, 10)` is both
C- and F-contiguous. Thus, if it is exported as C-contiguous and
then as F-contiguous, and then *again* as C-contiguous, this will
work, but the last export will compare to the export as an F-contig
buffer and thus still leak memory.
Address gh-16934 (but does leave a small hole)
-rw-r--r-- | numpy/core/src/multiarray/buffer.c | 86 | ||||
-rw-r--r-- | numpy/core/tests/test_multiarray.py | 7 |
2 files changed, 53 insertions, 40 deletions
diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c index af40cdc2c..770813e70 100644 --- a/numpy/core/src/multiarray/buffer.c +++ b/numpy/core/src/multiarray/buffer.c @@ -456,7 +456,7 @@ static PyObject *_buffer_info_cache = NULL; /* Fill in the info structure */ static _buffer_info_t* -_buffer_info_new(PyObject *obj) +_buffer_info_new(PyObject *obj, npy_bool f_contiguous) { /* * Note that the buffer info is cached as PyLongObjects making them appear @@ -504,9 +504,43 @@ _buffer_info_new(PyObject *obj) info->shape = (npy_intp *)((char *)info + sizeof(_buffer_info_t)); assert((size_t)info->shape % sizeof(npy_intp) == 0); info->strides = info->shape + PyArray_NDIM(arr); - for (k = 0; k < PyArray_NDIM(arr); ++k) { - info->shape[k] = PyArray_DIMS(arr)[k]; - info->strides[k] = PyArray_STRIDES(arr)[k]; + +#if NPY_RELAXED_STRIDES_CHECKING + /* + * When NPY_RELAXED_STRIDES_CHECKING is used, some buffer users + * may expect a contiguous buffer to have well formatted strides + * also when a dimension is 1, but we do not guarantee this + * internally. Thus, recalculate strides for contiguous arrays. + * (This is unnecessary, but has no effect in the case where + * NPY_RELAXED_STRIDES CHECKING is disabled.) + */ + if (PyArray_IS_C_CONTIGUOUS(arr) && !( + f_contiguous && PyArray_IS_F_CONTIGUOUS(arr))) { + Py_ssize_t sd = PyArray_ITEMSIZE(arr); + for (k = info->ndim-1; k >= 0; --k) { + info->shape[k] = PyArray_DIMS(arr)[k]; + info->strides[k] = sd; + sd *= info->shape[k]; + } + } + else if (PyArray_IS_F_CONTIGUOUS(arr)) { + Py_ssize_t sd = PyArray_ITEMSIZE(arr); + for (k = 0; k < info->ndim; ++k) { + info->shape[k] = PyArray_DIMS(arr)[k]; + info->strides[k] = sd; + sd *= info->shape[k]; + } + } + else { +#else /* NPY_RELAXED_STRIDES_CHECKING */ + /* We can always use the arrays strides directly */ + { +#endif + + for (k = 0; k < PyArray_NDIM(arr); ++k) { + info->shape[k] = PyArray_DIMS(arr)[k]; + info->strides[k] = PyArray_STRIDES(arr)[k]; + } } } Py_INCREF(descr); @@ -565,7 +599,7 @@ _buffer_info_free(_buffer_info_t *info) /* Get buffer info from the global dictionary */ static _buffer_info_t* -_buffer_get_info(PyObject *obj) +_buffer_get_info(PyObject *obj, npy_bool f_contiguous) { PyObject *key = NULL, *item_list = NULL, *item = NULL; _buffer_info_t *info = NULL, *old_info = NULL; @@ -578,7 +612,7 @@ _buffer_get_info(PyObject *obj) } /* Compute information */ - info = _buffer_info_new(obj); + info = _buffer_info_new(obj, f_contiguous); if (info == NULL) { return NULL; } @@ -596,6 +630,13 @@ _buffer_get_info(PyObject *obj) item = PyList_GetItem(item_list, PyList_GET_SIZE(item_list) - 1); old_info = (_buffer_info_t*)PyLong_AsVoidPtr(item); + /* + * TODO: In rare cases when an array is both C- and + * F-contiguous, and buffers are exported alternating + * we may always have a cache miss even though the correct + * info had already been exported before (causing a memory + * leak). + */ if (_buffer_info_cmp(info, old_info) == 0) { _buffer_info_free(info); info = old_info; @@ -706,7 +747,7 @@ array_getbuffer(PyObject *obj, Py_buffer *view, int flags) } /* Fill in information */ - info = _buffer_get_info(obj); + info = _buffer_get_info(obj, (flags & PyBUF_F_CONTIGUOUS) == PyBUF_F_CONTIGUOUS); if (info == NULL) { goto fail; } @@ -742,35 +783,6 @@ array_getbuffer(PyObject *obj, Py_buffer *view, int flags) } if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) { view->strides = info->strides; - -#ifdef NPY_RELAXED_STRIDES_CHECKING - /* - * If NPY_RELAXED_STRIDES_CHECKING is on, the array may be - * contiguous, but it won't look that way to Python when it - * tries to determine contiguity by looking at the strides - * (since one of the elements may be -1). In that case, just - * regenerate strides from shape. - */ - if (PyArray_CHKFLAGS(self, NPY_ARRAY_C_CONTIGUOUS) && - !((flags & PyBUF_F_CONTIGUOUS) == PyBUF_F_CONTIGUOUS)) { - Py_ssize_t sd = view->itemsize; - int i; - - for (i = view->ndim-1; i >= 0; --i) { - view->strides[i] = sd; - sd *= view->shape[i]; - } - } - else if (PyArray_CHKFLAGS(self, NPY_ARRAY_F_CONTIGUOUS)) { - Py_ssize_t sd = view->itemsize; - int i; - - for (i = 0; i < view->ndim; ++i) { - view->strides[i] = sd; - sd *= view->shape[i]; - } - } -#endif } else { view->strides = NULL; @@ -800,7 +812,7 @@ void_getbuffer(PyObject *self, Py_buffer *view, int flags) } /* Fill in information */ - info = _buffer_get_info(self); + info = _buffer_get_info(self, 0); if (info == NULL) { goto fail; } diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 6f8af1757..d376fe6b7 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -7178,9 +7178,10 @@ class TestNewBufferProtocol: x3 = np.arange(dt3.itemsize, dtype=np.int8).view(dt3) self._check_roundtrip(x3) - def test_relaxed_strides(self): - # Test that relaxed strides are converted to non-relaxed - c = np.ones((1, 10, 10), dtype='i8') + @pytest.mark.valgrind_error(reason="leaks buffer info cache temporarily.") + def test_relaxed_strides(self, c=np.ones((1, 10, 10), dtype='i8')): + # Note: c defined as parameter so that it is persistent and leak + # checks will notice gh-16934 (buffer info cache leak). # Check for NPY_RELAXED_STRIDES_CHECKING: if np.ones((10, 1), order="C").flags.f_contiguous: |