RUrlus / carma

Converters between Armadillo matrices (C++) and Numpy arrays using Pybind11
Apache License 2.0
86 stars 25 forks source link

Potentially mismatched allocator-deallocator when linking to external shared library #91

Open RUrlus opened 3 years ago

RUrlus commented 3 years ago

Found in #89, there is an issue with the free from cnalloc.h is used against a plain malloc used by armadillo when a library that includes carma is linked against a library that doesn't.

Say we have shared library A that uses Armadillo but does not use CARMA. Library A has a function foo that creates a filled matrix and returns it, i.e. allocates memory for matrix using the default Armadillo allocater malloc.

Additionally, we have library B which contains the python bindings for library A and uses CARMA for the conversion to Numpy. Library B links to lib A.

The issue

If a function in lib B calls foo from lib A the memory of the matrix returned from foo is free'd using the free from cnalloc.h set using ARMA_ALIEN_MEM_FREE_FUNCTION. This triggers a segfault on Windows as there is a mismatch between the allocator and de-allocator.

Work-around

A work-around is to link CARMA using the CMake target with the pre-compiled cnalloc.h header to library A at compilation, for example:

find_package(pybind11 "2.7.1" CONFIG REQUIRED)
find_package(Armadillo "10.6" CONFIG REQUIRED)
find_package(carma "0.6" CONFIG REQUIRED)

add_library(A SHARED a.cpp)
target_link_libraries(A PUBLIC armadillo carma::carma)

pybind11_add_module(B MODULE b.cpp)
target_link_libraries(B PUBLIC A)

However, this does not work when linking against installed shared or static libraries.

conradsnicta commented 3 years ago

(quotation copied from https://github.com/RUrlus/carma/issues/89#issuecomment-923320672)

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?

Perhaps one approach would be always copy data to/from numpy arrays, so that numpy memory is always managed by Python/numpy, and Armadillo related data is always managed by arma::memory::acquire() and arma::memory::release().

The above approach can be made the default. The "unsafe" approach (eg. stealing memory) can be optionally user enabled (either at compile time, or through an option to a function). The "unsafe" approach would be aimed folks that know what they are doing and/or can easily control all dependencies.

RUrlus commented 3 years ago

Thanks, I was hoping to avoid this but it seems like a safe and easier to use mode is needed where we make the current behaviour available to those that can use it

RUrlus commented 1 year ago

@chemiskyy Could you open a separate issue with some more information regarding versions of CPython, platform etc, it's not directly related as far as I can tell.

The original issue is a run-time issue that only affects windows.