Fix SEGV with Python 3. #58
Conversation
|
I can confirm that this fix SEGV on Arch (boost 1.69, python3.7 & numpy 1.16.2) |
|
It also works on 18.04. On both Arch & 18.04, the |
|
https://gepgitlab.laas.fr/gsaurel/eigenpy/pipelines/4046 LGTM ! It would be really nice if we could make a release with this, so that I can ship it with pinocchio v2.1.2 & HPP 4.5 on my next robotpkg PR |
|
I ended up with this fix because the SEGV arises when deleting the numpy module object. I guess it is deleted twice. Increasing the reference count may have the side effect of Numpy module not being deleted. I didn't check that. It shouldn't be hard with a break point in gdb. |
|
|
||
| NumpyMatrixObject = pyModule.attr("matrix"); | ||
| NumpyMatrixType = reinterpret_cast<PyTypeObject*>(NumpyMatrixObject.ptr()); | ||
| NumpyAsMatrixObject = pyModule.attr("asmatrix"); |
There was a problem hiding this comment.
Why have you deleted that? Is it deprecated on newer versions of NumPy?
There was a problem hiding this comment.
It isn't used so I thought it was here as a result of development code not being cleaned up.
There was a problem hiding this comment.
I would prefer to keep as they are not related to the current bug.
|
@jmirabel Indeed it may happen that some data are kept in memory with this increase of reference. Maybe, we should check the Python version and not apply it to Python 2 & 3. |
|
I am not gonna do a specific case for Python 2. I would rather say: for Python 2, use eigenpy <= 1.5. We can open an issue in case someone is able to spend time on the possibly not deleted object. |
|
I don’t agree as part of the current users are still using Python 2. We cannot deprecate now this feature even if Python will be over soon. |
|
|
||
| NumpyMatrixObject = pyModule.attr("matrix"); | ||
| NumpyMatrixType = reinterpret_cast<PyTypeObject*>(NumpyMatrixObject.ptr()); | ||
| NumpyAsMatrixObject = pyModule.attr("asmatrix"); |
There was a problem hiding this comment.
I would prefer to keep as they are not related to the current bug.
3 participants
Related to #35
I don't know exactly why this works (and whether this works for everyone).