-
Notifications
You must be signed in to change notification settings - Fork 25
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
Help with Armadillo memory bug in pybind11 #89
Comments
Also posted as a StackOverflow question: https://stackoverflow.com/questions/69226084/why-might-i-get-heap-corruption-using-armadillo-matrices-with-pybind11 |
Never mind, this does have something to do with Carma! Something about the way Carma causes Armadillo to get configured leads to the crash. I will confirm by adding Carma to the MRE and reproducing the bug. Still working on this, but it looks like the code fails because |
@kjohnsen The order of includes appears wrong. This matters, as carma changes the configuration of Armadillo. The carma header must be included before the armadillo header. Change the order to:
|
Thank you for helping bring that to my attention. I didn't notice all the warnings about include order and about I do think I've found a bug though, and my MRE is now working. Basically, even with the right include order, the crash occurs. I speculate that carma's memory functions are being used and failing even for the Armadillo stuff going on under the hood, when I'm not using carma converters at all. |
@kjohnsen Sorry, I've been away on holiday so couldn't respond earlier. Can you run with How were you building before? Because the
You are most likely mixing the allocation/deallocation functions, if you include carma in one library and not in the other it may happen that you are passing ownership to the other library which does not uses the corresponding deallocator for the allocator used. I.e the one that includes carma uses Numpy's wrapper around malloc and the other will call malloc directly. @conradsnicta Thanks for helping out |
I had already been using CARMA_EXTRA_DEBUG but I just tried with DEV_DEBUG_MODE too and I'm not seeing any evidence of the
Before, I was simply linking to the Yes, the mixing scenario seems to be what's happening. Creating the object from the bindings module might use NumPy's allocator whereas the separately compiled library would use the standard |
🤔 Except for the fact that none of Carma's memory functions seem to be getting called... Are you saying that normally it's necessary to include carma everywhere in the project? I'm trying to create bindings for a separate, standalone library. Would I need to include in all its source files? |
@kjohnsen I was right, the de-allocation is being done with the deallocation function from CARMA and the allocation is not. project(pybind-arma-bug LANGUAGES CXX)
cmake_minimum_required(VERSION 3.16 FATAL_ERROR)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
if (WIN32)
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()
find_package(Armadillo "10.6" CONFIG REQUIRED)
add_subdirectory(pybind11)
add_subdirectory(carma)
add_library(mc SHARED mat_container.cpp)
# use the commented line to reproduce the not_linked output
# target_link_libraries(mc public armadillo)
target_link_libraries(mc public armadillo carma::carma)
target_compile_definitions(mc PUBLIC ARMA_EXTRA_DEBUG CARMA_EXTRA_DEBUG CARMA_DEV_DEBUG)
pybind11_add_module(pymod MODULE pymod.cpp)
target_link_libraries(pymod PUBLIC mc)
FILE(TOUCH "${PROJECT_BINARY_DIR}/__init__.py")
Have a look at what happens when we don't link carma::carma to the When we link carma to the other target it works because we pre-compile the header
No you don't need to include but you have to link carma against the standalone library at compilation time |
@conradsnicta This is an issue for projects that link against an external library using armadillo but not linked with carma. It appears that the allocation of the memory is handled by code from external library, i.e. without ARMA_ALIEN_MEM_ALLOC_FUNCTION set, but destructed using the code compiled with ARMA_ALIEN_MEM_FREE_FUNCTION from the lib that includes carma. Any ideas how we could avoid this other than requiring that the external lib is linked with carma at compile time? |
@RUrlus Ok, that is all working for me now. Many thanks! I don't know why I wasn't getting carma debug statements after using both CARMA_EXTRA_DEBUG and -DCMAKE_DEBUG_DEV_MODE=ON, but after using the compile definition CARMA_DEV_DEBUG like you did I'm seeing them now. This has been quite the debugging rabbit hole, but I'm glad to see the fix is so simple. |
@kjohnsen Good to hear, I can imagine it was quite the rabbit hole. A type of bug that is quite hard to find unless you very familiar with the internals of the library. Bad timing that I was away. |
@RUrlus I probably should have asked for help sooner. I didn't even think it was a Carma problem at first and without an MRE I wasn't sure anyone would be able to help. |
Hello, even though this bug doesn't seem to be with Carma itself, I turn to you for help because I'm really stuck and I don't know if the pybind11 or Armadillo communities would be as much help in this.
EDIT: it does appear to be a Carma bug and the MRE is working.
I've been trying to boil it down to an MRE and have been unsuccessful. I will explain what I know here, but since this isn't enough to reproduce the bug, what I need most are some ideas as to where to look. This is basically a very hard-to-reproduce memory corruption error (heap corruption,
Windows fatal exception: code 0xc0000374
). It seems to happen when I have a matrix A, not initialized, and assign a matrix to it large enough that Armadillo acquires memory. The crash happens when that memory is released.I'm on Windows 10, using Armadillo 10.6.2 and pybind11 v2.7.1, compiling using Clang 11.
Binding code
All I need to do to trigger the crash is from Python call
example.MC(11)
and it crashes upon releasing memory in the matrix destructor of the member variable (not the temporary one assigned to it). Armadillo debug messages right before crash:Weird things
np.ones
but notnp.eye
) matrix to an object's matrix member variable. But this only happens on a class in my source code, not on the smaller test class I created!ARMA_EXTRA_DEBUG
), because I don't get a similar error when I use a class with another dynamic memory class like a vectorWhen A is using acquired memory and is replaced by a matrix using local memory, the crash occurs
Arma debug messages right before crash, as mc.A is being assigned:
but not when it's the other way around; this runs successfully. It does not crash as the acquired memory is released as A is destroyed:
And when an 11x11 A is replaced by another 11x11 matrix from numpy, the crash is again
Mat::destructor: releasing memory
as in the very first example without assignment from Python. What I deduce from these examples is that the crash is averted when an acquired memory matrix prepared by Carma (not Armadillo) is assigned to a local memory AAttached files bug1.txt and bug2.txt are the logs from the first example and the failed assignment through Carma
The text was updated successfully, but these errors were encountered: