From 3f3217680434abf7140b2fc1bc6d6d9a4e5d4b13 Mon Sep 17 00:00:00 2001 From: "J.A. de Jong - Redu-Sone B.V., ASCEE V.O.F" Date: Thu, 3 Jun 2021 21:35:04 +0200 Subject: [PATCH] Bugfix for transferring malloc'ed data to a pyarray under windows with the capsule --- lasp/c/lasp_pyarray.h | 110 ++++++++++++++++++++++++------------------ lasp/c/lasp_python.h | 2 +- 2 files changed, 64 insertions(+), 48 deletions(-) diff --git a/lasp/c/lasp_pyarray.h b/lasp/c/lasp_pyarray.h index cbe8792..b0a3b7d 100644 --- a/lasp/c/lasp_pyarray.h +++ b/lasp/c/lasp_pyarray.h @@ -22,65 +22,81 @@ * Function passed to Python to use for cleanup of * foreignly obtained data. **/ +#define LASP_CAPSULE_NAME "pyarray_data_destructor" static inline void capsule_cleanup(PyObject *capsule) { - void *memory = PyCapsule_GetPointer(capsule, NULL); - free(memory); + void *memory = PyCapsule_GetPointer(capsule, LASP_CAPSULE_NAME); + free(memory); } #endif + static inline PyObject *data_to_ndarray(void *data, int n_rows, int n_cols, - int typenum, bool transfer_ownership, - bool F_contiguous) { + int typenum, bool transfer_ownership, + bool F_contiguous) { - /* fprintf(stderr, "Enter data_to_ndarray\n"); */ - assert(data); - import_array(); + /* fprintf(stderr, "Enter data_to_ndarray\n"); */ + assert(data); + import_array(); + PyArray_Descr *descr = PyArray_DescrFromType(typenum); + if(!descr) return NULL; - npy_intp dims[2]; - if(F_contiguous){ - dims[0] = n_cols; - dims[1] = n_rows; + npy_intp dims[2] = {n_rows, n_cols}; + npy_intp strides[2]; - } else { - dims[0] = n_rows; - dims[1] = n_cols; + int flags = 0; + if(F_contiguous){ + flags |= NPY_ARRAY_FARRAY; + strides[0] = descr->elsize; + strides[1] = descr->elsize*n_rows; + } else { + strides[0] = descr->elsize*n_rows; + strides[1] = descr->elsize; + } + + /* assert(n_rows > 0); */ + /* assert(n_cols > 0); */ + + PyArrayObject *arr = + (PyArrayObject *)PyArray_NewFromDescr( + &PyArray_Type, + descr, // Description + 2, // nd + dims, // dimensions + strides, // strides + data, // Data pointer + flags, // Flags + NULL // obj + ); + + if (!arr) { + fprintf(stderr, "arr = 0!"); + return NULL; + } + + if (transfer_ownership == true) { + #ifdef MS_WIN64 + // The default destructor of Python cannot free the data, as it is allocated + // with malloc. Therefore, with this code, we tell Numpy/Python to use + // the capsule_cleanup constructor. See: + // https://stackoverflow.com/questions/54269956/crash-of-jupyter-due-to-the-use-of-pyarray-enableflags/54278170#54278170 + // Note that in general it was disadvised to build all C code with MinGW on + // Windows. We do it anyway, see if we find any problems on the way. + PyObject *capsule = PyCapsule_New(data, LASP_CAPSULE_NAME, capsule_cleanup); + int res = PyArray_SetBaseObject(arr, capsule); + if(res != 0) { + fprintf(stderr, "Failed to set base object of array!"); + return NULL; } - /* assert(n_rows > 0); */ - /* assert(n_cols > 0); */ + #endif + /* fprintf(stderr, "============Ownership transfer================\n"); */ + PyArray_ENABLEFLAGS(arr, NPY_OWNDATA); + } + /* fprintf(stderr, "Exit data_to_ndarray\n"); */ - PyArrayObject *arr = - (PyArrayObject *)PyArray_SimpleNewFromData(2, dims, typenum, data); - - if (!arr) { - return NULL; - } - if (F_contiguous) { - arr = (PyArrayObject*) PyArray_Transpose(arr, NULL); - } - - if (transfer_ownership == true) { - #ifdef MS_WIN64 - // The default destructor of Python cannot free the data, as it is allocated - // with malloc. Therefore, with this code, we tell Numpy/Python to use - // the capsule_cleanup constructor. See: - // https://stackoverflow.com/questions/54269956/crash-of-jupyter-due-to-the-use-of-pyarray-enableflags/54278170#54278170 - // Note that in general it was disadvised to build all C code with MinGW on - // Windows. We do it anyway, see if we find any problems on the way. - PyObject *capsule = PyCapsule_New(data, "data destructor", capsule_cleanup); - int res = PyArray_SetBaseObject(arr, capsule); - if(res != 0) { - fprintf(stderr, "Failed to set base object of array!"); - return NULL; - } - #endif - /* fprintf(stderr, "============Ownership transfer================\n"); */ - PyArray_ENABLEFLAGS(arr, NPY_OWNDATA); - } - /* fprintf(stderr, "Exit data_to_ndarray\n"); */ - - return (PyObject *) arr; + return (PyObject *) arr; } +#undef LASP_CAPSULE_NAME #endif // LASP_PYARRAY_H ////////////////////////////////////////////////////////////////////// diff --git a/lasp/c/lasp_python.h b/lasp/c/lasp_python.h index 4ae3ff4..409b082 100644 --- a/lasp/c/lasp_python.h +++ b/lasp/c/lasp_python.h @@ -13,7 +13,7 @@ /** * Create a numpy array from an existing dmat. * - * @param mat dmat struccture containing array data and metadata. + * @param mat dmat structure containing array data and metadata. * @param transfer_ownership If set to true, Numpy array will be responsible * for freeing the data. *