Fix memory leak in get_dft_array #577
Conversation
python/meep.i
Outdated
| @@ -372,6 +372,7 @@ PyObject *_get_dft_array(meep::fields *f, dft_type dft, meep::component c, int n | |||
| } | |||
|
|
|||
| PyObject *py_arr = PyArray_SimpleNewFromData(rank, arr_dims, NPY_CDOUBLE, dft_arr); | |||
| PyArray_ENABLEFLAGS((PyArrayObject*)py_arr, NPY_ARRAY_OWNDATA); | |||
There was a problem hiding this comment.
This will eventually cause NumPy to call free, so it is not safe to set this flag for an array allocated with new like in get_dft_array.
There was a problem hiding this comment.
It would be nice to be able to get the array size and allocate a numpy array from Python like we do in Simulation.get_array, but I don't see a way to get the dft array size, so I'm copying the dft data to my own malloced array.
There was a problem hiding this comment.
I don't think you need to set the NPY_ARRAY_OWNDATA flag if you are using PyArray_SimpleNew
There was a problem hiding this comment.
You're right. Fixed.
python/meep.i
Outdated
| } | ||
|
|
||
| PyObject *py_arr = PyArray_SimpleNewFromData(rank, arr_dims, NPY_CDOUBLE, dft_arr); | ||
| // Allocate our own version of dft_arr with malloc because | ||
| // numpy will eventually call free on it. |
There was a problem hiding this comment.
Seems like it would be simpler to just do:
PyObject *py_arr = PyArray_SimpleNew(rank, arr_dims, NPY_CDOUBLE);
memcpy(PyArray_DATA(py_arr), dft_arr, sizeof(std::complex<double>) * length);i.e. to let Python do the allocation.
* Fix memory leak in get_dft_array * Copy dft array so numpy can free it * Let numpy allocate via PyArray_SimpleNew * OWNDATA flag is already true with PyArray_SimpleNew
Fixes #575.
@stevengj @oskooi