Declare the rosbag storage plugins, default to mcap, and serialize the plugin loader

[bburda]

Pull Request

Summary

Two defects, both surfaced by the same build farm job once its launch-test noise was out of the way.

1. Black-box rosbag capture had no storage plugin in its install closure.

rosbag2_storage is the plugin interface and libsqlite3-dev is the C library the fault database uses. Neither provides a storage backend. An installed fault manager therefore found no backend, logged a warning and disabled black-box capture, which is a feature the README leads with. The Humble binarydeb job is the only place these packages meet a minimal dependency closure, which is why this had been invisible.

The default format returns to mcap. That is what rosbag2 has defaulted to since Iron and what Foxglove and Lichtblick open without a conversion step. This restores the default this project chose at 0.4.0; it was moved to sqlite3 in a later fix on the stated premise that sqlite3 always ships, and that premise was false for the same reason as above. Both plugins are now exec and test dependencies, and the runtime stage of the image installs them, so the published image can write the format it advertises.

Storage resolution is symmetric now: whichever format is configured, if its plugin fails to load the other is probed and used, naming the missing package. Capture disables only when neither loads. Before, a configured sqlite3 that failed to load disabled capture even when mcap was usable, and blamed a broken base install.

Both warnings name the package an operator installs (ros-$ROS_DISTRO-rosbag2-storage-mcap), not the ROS package name, which differs by the distro prefix and by underscore-versus-hyphen.

2. A SIGSEGV in the storage plugin loader.

rosbag2_cpp::Writer loads its storage plugin inside open(), and ~Writer can unload it. The racing state is class_loader's process-global library registry, so the crash is a double unload of one shared handle: the winner calls dlclose and zeroes the rcutils_shared_library_t, the loser's dlclose returns shared object not open and it then calls the now-null allocator.deallocate. The faulting frame is rcutils_unload_shared_library.

This is a bug in class_loader, not in this package; what changes here is that this package stops entering it. It reproduces on the merge base, and on both backends, so it is neither new nor caused by the default change above. It is worth reporting upstream, and a minimal reproduction exists that does not involve this code.

---

Issue

Closes #619

---

Type

• Bug fix
• New feature or tests
• Breaking change
• Documentation only

---

Testing

The default format is proven on disk, not from a reported field. A launch test starts a fault manager with no snapshots.rosbag.format parameter, triggers a fault, and requires a .mcap file in the resulting bag directory. A unit case pins the struct default separately, and a manifest test pins both plugin declarations, including that neither carries a condition attribute that would silently drop them on one distro.

The loader race, measured. A two-thread program that does nothing but open and close a writer on one storage id: 0 of 20 runs clean at baseline, on mcap and on sqlite3 alike. With the plugin mutex, 140 of 140 runs clean over 112000 operations per backend. A control with open() outside the lock still leaked failures, which is why it is inside. A regression test crosses two captures' plugin traffic in one process; a per-thread lock makes it crash, and a reversed lock order makes it hang, which ctest reports as a named timeout.

The install hints are checked against the log, not against the source. A test captures the rcutils output around the constructor for all three storage outcomes and requires the apt package name in each message. Two mutations were run against it: putting the ROS package names back in the disable warning, and dropping the package name from the fallback hint. Both fail the test.

Cost. The plugin mutex is held across a close: about 0.37 ms for an empty bag and about 1.1 ms for a 256 MB one, which is the largest a single flush can produce. writer_mutex_ is no longer held across an open or a close, so a close cannot stall the post-roll callbacks.

Suites. ros2_medkit_fault_manager and the full lint set clean, locally, on this branch rebased on current main.

What this cannot prove. The dependency half is proven by the next Humble binarydeb run, not by a green suite on machines that have both plugins installed anyway.

---

Note for reviewers

Breaking: an installation that upgrades without setting snapshots.rosbag.format starts producing .mcap bags instead of .db3, and the bulk-data download filename changes with it. Anything reading the old extension directly has to follow.

One test's measurement was corrected alongside: test_rosbag_integration's helper counted messages arriving at the test node's own subscription and assumed the fault manager had buffered a comparable amount, but the fault manager buffers nothing while a post-fault recording is in flight. The assertion was right and the instrument feeding it was not.

---

Checklist

• Breaking changes are clearly described (and announced in docs / changelog if needed)
• Tests were added or updated if needed
• Docs were updated if behavior or public API changed

[Copilot]

Pull request overview

This PR fixes rosbag “black-box” capture reliability in ros2_medkit_fault_manager by (1) ensuring storage backend plugins are actually present in installed/test dependency closures, (2) switching the default bag format back to MCAP, and (3) preventing a known process-global rosbag2/class_loader unload race by serializing plugin load/unload paths.

Changes:

• Declare rosbag2 storage backends as runtime + test dependencies, update Docker runtime stage accordingly, and validate the manifest via a new pytest.
• Default rosbag capture format to mcap again, with symmetric fallback to the other backend when a configured plugin is unavailable, plus operator-actionable warnings.
• Add locking around writer open/close/probe paths to avoid storage plugin loader SIGSEGV; expand unit/integration test coverage and update docs/design notes.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp	Clarifies “historical” sqlite3 default for older bag metadata.
src/ros2_medkit_gateway/README.md	Updates gateway docs to reflect mcap default and fallback behavior.
src/ros2_medkit_fault_manager/test/test_rosbag_storage_dependency.py	New manifest-level test to enforce plugin dependency declarations (and no conditional deps).
src/ros2_medkit_fault_manager/test/test_rosbag_integration.test.py	Improves buffering readiness logic by accounting for post-fault diversion using events.
src/ros2_medkit_fault_manager/test/test_rosbag_default_format.test.py	New launch test asserting the on-disk default format is .mcap when unset.
src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp	Expands unit/integration tests for default format, symmetric fallback, warnings, and concurrency race regression.
src/ros2_medkit_fault_manager/src/rosbag_capture.cpp	Implements symmetric backend resolution + package-name hints; adds process-wide plugin mutex around writer open/close/probe to avoid unload race.
src/ros2_medkit_fault_manager/src/fault_manager_node.cpp	Changes declared parameter default for snapshots.rosbag.format to mcap.
src/ros2_medkit_fault_manager/README.md	Documents default mcap, fallback behavior, and plugin dependencies.
src/ros2_medkit_fault_manager/package.xml	Adds exec/test dependencies for rosbag2_storage_mcap and rosbag2_storage_default_plugins; adds pytest deps.
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp	Updates RosbagConfig default format and documentation.
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp	Documents new lock ordering including plugin_mutex() -> writer_mutex_.
src/ros2_medkit_fault_manager/design/index.rst	Adds design rationale and measurements for the plugin loader serialization lock.
src/ros2_medkit_fault_manager/config/snapshots.yaml	Updates default format to mcap and describes fallback/dependency expectations.
src/ros2_medkit_fault_manager/CMakeLists.txt	Registers new pytest manifest test and new launch test for default format.
QUALITY_DECLARATION.md	Adds rosbag storage plugin packages to declared dependency list.
docs/tutorials/snapshots.rst	Updates tutorial docs for mcap default and symmetric fallback semantics.
docs/config/fault-manager.rst	Updates config docs for new default and clarifies behavior.
Dockerfile	Ensures runtime image installs both rosbag storage plugins needed at runtime.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.