Assimp is missing symbols on Debian and Ubuntu

[mkoval]

There is a major bug (Debian issue tracker, duplicate on Launchpad) in the version of Assimp released on Debian and Ubuntu. The packaging incorrectly strips several C++ symbols from the shared library, including the aiScene constructor and destructor. This explains the this Assimp issue and why DART has to define these methods itself.

Impact

DART programmatically constructs an aiScene to represent deformable geometry in the MeshShape associated with each SoftBodyNode. There are a few potential dangers in defining our own constructor and destructor:

1. We cannot initialize the mPrivate pointer in the aiScene object because the necessary ScenePrivateData class is not exposed in Assimp's public headers. This is assumed to exist in the exporter and will likely lead to SEGFAULT if it is not present.
2. There will be duplicate symbols for these functions if the user upgrades Assimp to a version without the packaging bug (e.g. 3.0~dfsg-4). It's unclear how catastrophic the implications of this are: I suspect it largely depends on what linker you are using.
3. The default behavior of Assimp's constructors and destructors may change, e.g. if a new member is added to aiScene, which could lead to a memory leak.

Long-Term Fix

This is fixed in the 3.0~dfsg-4 release of the libassimp package, which is available for Debian unstable and Ubuntu Wiley. I started the stable release process (SRU) on Ubuntu to make this change available for the currently supported distributions (Utopic, Trusty, and Vivid). An #ubuntu-bugs team member sponsored the SRU, so the next step is to prepare the updated packages for those repositories. I don't have the background knowledge necessary to do so, this may take a while.

Changes to DART

I suggest that we remove the Assimp constructors and destructors from DART and require a correct version of Assimp. We can add a check_cxx_source_compiles entry in DART's CMakeLists.txt file to check for the missing symbols and alert the user to upgrade their version of Assimp. This should only affect Debian and Ubuntu users, so we can provide the fixed version of Assimp in the DART PPA until it migrates to universe. To be safe, we should also make the DART package depend on this version (or greater) of Assimp in the debian/control file.

This should be a relatively painless process for those of us build from source. All that should be required is a quick apt-get update && apt-get install libassimp3 libassimp-dev.

Thoughts?

[Edit] Tagging @psigen.

[mxgrey]

The check_cxx_source_compiles sounds brilliant; I didn't know about that CMake feature.

I wonder if we might be able to leverage that into defining a preprocessor definition that will include our custom constructor and destructor if the build attempt failed and leave them out if it succeeded.

The catch is, we would also have to write a custom release function for deleting the scene. If aiScene::mPrivate is a nullptr (which is what we would set it to with our custom constructor), then it just calls delete on the scene. Otherwise if mPrivate has a value, it calls the traditional aiReleaseImport.

With that custom release function, we could use std::shared_ptr for managing the aiScenes.

[mkoval]

I wonder if we might be able to leverage that into defining a preprocessor macro that will include our custom constructor and destructor if the build attempt failed and leave them out if it succeeded.

I'm nervous about this because Assimp is a dynamic library, so there could be a symbol conflict at runtime. This is the situation I am thinking of:

1. DART is built against Assimp 3.0~dfsg-2.
2. DART is installed on a machine that has Assimp 3.0~dfsg-4
3. There is an application that dynamically links with both DART and Assimp.
4. The application calls the aiScene constructor or destructor.

My hunch is that (4) will cause linking to fail with a duplicate symbol error because the aiScene constructor and destructor are defined in both Assimp and DART.

The catch is, we would also have to write a custom release function for deleting the scene. If aiScene::mPrivate is a nullptr (which is what we would set it to with our custom constructor), then it just calls delete on the scene. Otherwise if mPrivate has a value, it calls the traditional aiReleaseImport.

I believe that this would work, as long as we do not hit the issue I describe above. However, it makes me nervous to rely this much on the internal behavior of Assimp. I would much rather fix Assimp, since it is not all that much more effort (and, eventually, will be available in the default Ubuntu repository).

With that custom release function, we could use std::shared_ptr for managing the aiScenes.

I think this is a great idea. 😄 See #452.

[mxgrey]

I'm thinking the version that we provide with the package manager should be the version that links against assimp correctly, so the only practical way I can imagine this happening is:

1. DART is built and installed on a machine with Assimp 3.0~dfsg-2
2. The machine is updated to have Assimp 3.0~dfsg-4
3. The user neglects to rerun cmake and reinstall DART

So it could definitely happen, and it would probably be painful and confusing for the user, so I can understand why you'd be nervous. My primary motivation for proposing this is to make the transition easier, because I think it might be difficult to require zero-day adoption.

[mkoval]

I'm thinking the version that we provide with the package manager should be the version that links against assimp correctly.

No matter what we do here, we should definitely modify the debian/control file ASAP to depend on Assimp < 3.0~dfsg-4 and release a new version of DART. This will, at least, stop DART from breaking when the new version of Assimp is available in Ubuntu.

My primary motivation for proposing this is to make the transition easier, because I think it might be difficult to require zero-day adoption.

I don't think this will be a burden as long as: (1) the updated version of Assimp is available in the DART PPA and (2) the error message makes it clear that you need to upgrade Assimp. The DART PPA is required for CCD and FCL, so I suspect that most people fall into one of the following categories:

1. built DART and all of it dependencies from source
2. built DART from source, but installed the dependencies from the PPA
3. installed DART and the dependencies both from the PPA

Group (1) already has a good version of Assimp, (2) will be able to install the new version of Assimp simply by running apt-get update && apt-get install libassimp-dev, and (3) will get a consistent version of DART and Assimp from the version bounds on the Debian package.

DART will only break in an unpredictable way if the user installed Assimp from source and installed an old version of DART from a Debian package.

[mxgrey]

I suppose that's fair. If we print a warning with a clear explanation and instructions, it should be painless enough.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[jslee02]

Closing as it has been confirmed that this issue is no longer relevant with the use of Assimp version 5 (see 9001773)