Skip to content

Conversation

@francoisk
Copy link
Contributor

@francoisk francoisk commented Sep 3, 2025

Potential fix for issue #4139

(Not-so-)Brief summary of changes

Based on comments and commits these LoadOpenSimLibrary("osimActuators") calls exist because the osimActuators shared library "is not automatically loaded on anything other than clang". The reason for this is that the tests in question do not actually reference anything in osimActuators directly (e.g., call any of its functions) so the linker sees this and decides there's no need to record the dependency in the executable. And thus osimActuators is not loaded when the executable is run.

If we instead just call one of its functions the linker will record the dependency in the test executable and the osimActuators library will get loaded automatically.

Given that the only reason we need osimActuators is because the data files these tests load contain objects defined in and registered by that library, perhaps the most appropriate function to call would be RegisterTypes_osimActuators(). That works but then we end up registering the osimActuators types twice (first time being when the library is loaded). So I went with just default-instantiating what looks like one of the most common Actuator classes given that we're loading Actuators from the data files.

I think there are probably linker-specific methods to make this work but it seems like the above is probably going to be simpler.

Testing I've completed

Ran tests locally on Linux. Only 5 Moco tests failed, but this is true even for the main branch.

Looking for feedback on...

The method is still a bit of a hack so any suggestions are welcome but I think it's definitely better than calling LoadOpenSimLibrary().

CHANGELOG.md

  • no need to update because these are not user-visible changes.

This change is Reviewable

Based on comments and commits these calls exist because the osimActuators
shared library "is not automatically loaded on anything other than clang". The
reason for this is that the tests in question do not actually reference
anything in osimActuators directly (e.g., call any of its functions) so the
linker sees this and decides there's no need to record the dependency in the
executable. And thus osimActuators is not loaded when the executable is run.

If we instead just call one of its functions the linker will record the
dependency in the test executable and the osimActuators library will get
loaded automatically.

Given that the only reason we need osimActuators is because the data files
these tests load contain objects defined in and registered by that library,
perhaps the most appropriate function to call would be
RegisterTypes_osimActuators(). That works but then we end up registering the
osimActuators types twice (first time being when the library is loaded). So I
went with just default-instantiating what looks like one of the most common
Actuator classes given that we're loading Actuators from the data files.

I think there are probably linker-specific methods to make this work but it
seems like the above is probably going to be simpler.
@adamkewley
Copy link
Contributor

adamkewley commented Sep 5, 2025

Thanks @francoisk ,

I've encountered this issue a few times when playing around with OpenSim for various projects, and I understand that one way of getting the compiler to play ball is to use a symbol from the library, but I'd say that it's certainly better to just use/add a symbol specifically for that intent, such as (as you suggest) RegisterTypes_osimActuators. The RegisterTypes_* functions can always be updated with a static (i.e. global) function-local boolean flag to prevent double-registration:

static bool actuallyRegisterStuff()
{
    Object::registerBlaBla(...);
    return true;
}

void OpenSim::RegisterTypes_osimActuators()
{
    [[maybe_unused]] static const bool registered = actuallyRegisterStuff();
}

There's also std::call_once, if you're feeling fancy.

OpenSim Creator had similar issues over the years, I ended up wrapping everything into an osim::init function that calls all of the relevant OpenSim initializers: that function should only be called once (hopefully ;)) and, in my case, must be called because I'm statically linking OpenSim:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants