Fix data race on AbstractMetaObjectBase's owning-loader list

[thomasmoore-torc]

Problem

AbstractMetaObjectBase::addOwningClassLoader() / removeOwningClassLoader() / isOwnedBy() (and the other accessors of associated_class_loaders_) take no lock of their own. createInstance() in class_loader_core.hpp releases getPluginBaseToFactoryMapMapMutex() before reading factory->isOwnedBy(loader), so a concurrent addOwningClassLoader() / removeOwningClassLoader() on another thread (e.g. from loadLibrary()'s already-loaded branch) can race with that read — and, since it's a std::vector, with any other concurrent read/write of the same metaobject's owner list.

I hit this with ThreadSanitizer as a data race on std::vector<ClassLoader *>'s push_back()/erase() (via the vector's internal reallocation). It's straightforward to trigger by constructing many independent ClassLoader instances for the same library concurrently — e.g. one rosbag2_cpp::Writer per thread, each internally constructing its own pluginlib::ClassLoader for the serialization format converter, all for the first time, simultaneously.

This is separate from #234 (loadLibrary()/unloadLibrary() overlap) and #231 (has_unmanaged_instance_been_created_) — both already merged, and both fix different unsynchronized globals in this same area. I checked the current rolling HEAD before writing this: createInstance() still releases the mutex before the isOwnedBy() checks, and meta_object.cpp's accessors are still fully unguarded.

Fix

• Give AbstractMetaObjectBaseImpl its own recursive_mutex guarding associated_class_loaders_, so every accessor (addOwningClassLoader, removeOwningClassLoader, isOwnedBy, isOwnedByAnybody, getAssociatedClassLoadersCount, getAssociatedClassLoader) is self-contained and safe regardless of what lock, if any, the caller happens to be holding.
• Also extend createInstance()'s existing getPluginBaseToFactoryMapMapMutex() scope to cover the isOwnedBy() checks themselves: between releasing that mutex (previously done right after the map lookup) and reading isOwnedBy(), the factory pointer looked up under it could in principle be invalidated by a concurrent destroyMetaObjectsForLibrary() on another thread. The mutex-per-vector fix above closes the vector-corruption race on its own; this closes the separate use-after-free risk on the factory pointer itself.

Testing

Added ClassLoaderTest.threadSafetyMultipleLoadersPerLibrary to test/utest.cpp. Unlike the existing ClassLoaderTest.threadSafety (one ClassLoader shared by all STRESS_TEST_NUM_THREADS threads, so isOwnedBy() is always queried with the loader that already, and permanently, owns the metaobject), this constructs a separate ClassLoader per thread for the same library, so construction/destruction concurrently mutates associated_class_loaders_ while other threads concurrently read it via isOwnedBy() in createInstance(). An anchor ClassLoader is kept alive for the whole test so the library's owner count never reaches zero and the worker threads never trigger the actual unload/dlclose() path — that path has its own pre-existing, unrelated refcounting bug (a library can be dlclose()-d twice when independent ClassLoaders for the same library race to be "last owner out"), which isn't what this test is targeting and would otherwise fail it intermittently regardless of this fix.

I built and ran this locally against a ros:rolling-ros-base container (colcon, -DBUILD_TESTING=ON), both with a plain build and with ThreadSanitizer. Unpatched, ThreadSanitizer reliably reports the addOwningClassLoader()/removeOwningClassLoader() race on associated_class_loaders_ (9-10 reports per run); with the fix, zero reports across many runs. The full colcon test --packages-select class_loader suite passes 100% (9/9) with the fix applied.

Generative AI

This fix was investigated, written, and iteratively verified (ThreadSanitizer, stress-testing, and a local colcon/ROS 2 rolling build) by Claude (Anthropic), operating as Claude Code, at the direction of and reviewed by me. The regression test and this PR description were also written by Claude; I reviewed both before submitting.

[thomasmoore-torc]

For reference, tracking the two related PRs mentioned above, both already merged:

• Make loadLibrary()/unloadLibrary() safe against concurrent load and unload #234 (loadLibrary()/unloadLibrary() concurrent load/unload): 221ae79
• concurrent accesses well-defined #231 (has_unmanaged_instance_been_created_ atomic): ae7ea8a

This PR fixes a third, still-unaddressed race in the same area (AbstractMetaObjectBase's owning-loader list).