-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Potential Issue with 2-D ARGOUTVIEWM Wrapping #17398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Not sure, looking at the swig code, I don't really see much of a reason these should differ. Could it be that the problem is just the EDIT: To be a bit more clear, please try this using |
I don't think so, as the getw function does the same thing and runs without error, and it is only the getw2 function that has the seg fault issue. Moreover, swig only issues a warning when wrapping the getw2 function, and the warning points to int* begin freed via the PyCapsule object. In the getw function, it is the double** pointer that is being freed via the PyCapsule object. Also, since replacing ARGOUTVIEWM with ARGOUTVIEW for the getw2 function results in working code, this leads me to believe that the free is working fine, and it is ARGOUTVIEWM that is causing the problem. EDIT: I just tried replacing the new with malloc, and the same issue persists. |
I've never heard of |
Well, the main difference between @leakec sorry for being pedantic, but you tried replacing both The only other idea I have is that dimensions are normally @eric-wieser it seems to be defined here: Line 2521 in fcca6e7
|
Ah, found it: Lines 2509 to 2542 in cb107d1
Lines 2544 to 2577 in cb107d1
|
@seberg No worries, yes I replaced both calls to new with malloc, and still the same issue. Its probably worth noting that I have used swig to wrap C++ in the past and have used new almost every time without issue. However, in the past I have always used ARGOUTVIEW arrays, as the programs were small enough that I did not need to use ARGOUTVIEWM arrays to release the memory after it is used. @eric-wieser Yes, that is what I see as well. When using (double** ARGOUTVIEWM_ARRAY2, int* DIM1, int* DIM2) the program appears to wrap correctly, and the free is called on the double** argument. Whereas when using (int* DIM1, int* DIM2, double** ARGOUTVIEWM_ARRAY2) the free is called on one of the int* arguments instead. My guess is that there is an issue in the typemap that has the ARGOUTVIEWM argument last. |
I think I found it! See this snippet of the numpy.i file: /* Typemap suite for (DATA_TYPE** ARGOUTVIEWM_ARRAY2, DIM_TYPE* DIM1, DIM_TYPE* DIM2)
*/
%typemap(in,numinputs=0)
(DATA_TYPE** ARGOUTVIEWM_ARRAY2, DIM_TYPE* DIM1 , DIM_TYPE* DIM2 )
(DATA_TYPE* data_temp = NULL , DIM_TYPE dim1_temp, DIM_TYPE dim2_temp)
{
$1 = &data_temp;
$2 = &dim1_temp;
$3 = &dim2_temp;
}
%typemap(argout,
fragment="NumPy_Backward_Compatibility,NumPy_Utilities")
(DATA_TYPE** ARGOUTVIEWM_ARRAY2, DIM_TYPE* DIM1, DIM_TYPE* DIM2)
{
npy_intp dims[2] = { *$2, *$3 };
PyObject* obj = PyArray_SimpleNewFromData(2, dims, DATA_TYPECODE, (void*)(*$1));
PyArrayObject* array = (PyArrayObject*) obj;
if (!array) SWIG_fail;
%#ifdef SWIGPY_USE_CAPSULE
PyObject* cap = PyCapsule_New((void*)(*$1), SWIGPY_CAPSULE_NAME, free_cap);
%#else
PyObject* cap = PyCObject_FromVoidPtr((void*)(*$1), free);
%#endif
%#if NPY_API_VERSION < 0x00000007
PyArray_BASE(array) = cap;
%#else
PyArray_SetBaseObject(array,cap);
%#endif
$result = SWIG_Python_AppendOutput($result,obj);
}
/* Typemap suite for (DIM_TYPE* DIM1, DIM_TYPE* DIM2, DATA_TYPE** ARGOUTVIEWM_ARRAY2)
*/
%typemap(in,numinputs=0)
(DIM_TYPE* DIM1 , DIM_TYPE* DIM2 , DATA_TYPE** ARGOUTVIEWM_ARRAY2)
(DIM_TYPE dim1_temp, DIM_TYPE dim2_temp, DATA_TYPE* data_temp = NULL )
{
$1 = &dim1_temp;
$2 = &dim2_temp;
$3 = &data_temp;
}
%typemap(argout,
fragment="NumPy_Backward_Compatibility,NumPy_Utilities")
(DIM_TYPE* DIM1, DIM_TYPE* DIM2, DATA_TYPE** ARGOUTVIEWM_ARRAY2)
{
npy_intp dims[2] = { *$1, *$2 };
PyObject* obj = PyArray_SimpleNewFromData(2, dims, DATA_TYPECODE, (void*)(*$3));
PyArrayObject* array = (PyArrayObject*) obj;
if (!array) SWIG_fail;
%#ifdef SWIGPY_USE_CAPSULE
PyObject* cap = PyCapsule_New((void*)(*$1), SWIGPY_CAPSULE_NAME, free_cap);
%#else
PyObject* cap = PyCObject_FromVoidPtr((void*)(*$1), free);
%#endif
%#if NPY_API_VERSION < 0x00000007
PyArray_BASE(array) = cap;
%#else
PyArray_SetBaseObject(array,cap);
%#endif
$result = SWIG_Python_AppendOutput($result,obj);
} As you can see, the ($1) argument is freed in both cases. If I switch the freed arguments to ($3) in the second case, the case where the ARGOUTVIEWM array is the last argument , it works! |
I see the bug.
should be
@seberg's warnings about |
@seberg Great minds think alike :) I'll propose the fix in a few minutes. |
Looks like we cross-posted. Mind making a PR? Perhaps even check the others too, if you're feeling generous. |
@leakec thanks for digging it out, had not noticed that typo obviously! A PR would be great, I am not sure if we run tests, but there do seem swig specific tests in the folder. |
@seberg @eric-wieser Sure thing, I will check the other ARGOUTVIEWM arrays as well. I'm pretty new to open source thing, so it will take me a few minutes to figure out how to make the PR correctly. |
@seberg @eric-wieser I was able to correct all of the ARGOUTVIEWM memory de-allocation. However, adding an ARGOUTVIEWM case to the current unit tests for testArray.py results in a double free. I believe this is because the Array class already deallocates that memory. Would you like me to create a separate unit test for the ARGOUTVIEWM arrays? |
BLD: Fixed ARGOUTVIEWM memory deallocation. Closes #17398.
I am having an issue with 2-D ARGOUTVIEWM wrapping with swig. The code works when using the form (double** ARGOUTVIEWM_ARRAY2, int* DIM1, int* DIM2), but not when using the form (int* DIM1, int* DIM2, double** ARGOUTVIEWM_ARRAY2). Moreover, when using the latter form, the form that causes a seg fault, swig issues a warning:
"warning: cast to pointer from integer of different size." Looking at the wrapped swig code, it appears that the wrong argument is being freed. When using the first form, this issue does not arise. It should also be noted that if ARGOUTVIEW arrays are used rather than ARGOUTVIEWM arrays, this issue also does not arise.
Since this is a swig wrapping issue, I will include the C++ code, swig code, setup file, and test file. After building the code with "python setup.py build" and moving the library from the build folder to the current directly, the test.py file can be used to reproduce the error, i.e. just run "python test.py".
Reproducing code example:
BF.h:
BF.cxx:
BF.i
setup.py
test.py:
Error message:
Terminal printout:
Running getw
Running getw2
Segmentation fault (core dumped)
NumPy/Python version information:
1.17.4 3.8.2 (default, Jul 16 2020, 14:00:26)
[GCC 9.3.0]
The text was updated successfully, but these errors were encountered: