A crash occurs when the storage object returned after the destruction of StorageFactory is destructed.

[zhihaoshang]

Description

A crash will occur during the destruction of the storage object if the StorageFactory is destructed before the ReadWriteInterface object it returned.

Expected Behavior

no crash

Actual Behavior

crash

To Reproduce

Test Case

#include <gtest/gtest.h>
#include <memory>
#include <string>
#include "rosbag2_storage/storage_interfaces/read_only_interface.hpp"
#include "rosbag2_storage/storage_interfaces/read_write_interface.hpp"
#include "rosbag2_storage/storage_factory.hpp"
#include "test_constants.hpp"

using rosbag2_storage::storage_interfaces::ReadWriteInterface;
using rosbag2_storage::storage_interfaces::ReadOnlyInterface;

class StorageFactoryTest : public ::testing::Test
{
protected:
  void SetUp() override {
    factory = std::make_unique<rosbag2_storage::StorageFactory>();
  }

  void TearDown() override {
    factory.reset();
  }

  std::unique_ptr<rosbag2_storage::StorageFactory> factory;
  const std::string bag_file_path = "path/to/be/loaded.bag";
  const std::string test_unavailable_plugin_id = "my_unavailable_plugin";
};

TEST_F(StorageFactoryTest, test) {
  auto local_factory = std::make_unique<rosbag2_storage::StorageFactory>();
  auto storage = local_factory->open_read_write(
    {bag_file_path, test_constants::READ_WRITE_PLUGIN_IDENTIFIER});
  ASSERT_NE(nullptr, storage);
  local_factory.reset();
}

Output

[ RUN      ] StorageFactoryTest.test
opening testplugin read write: storage uri: path/to/be/loaded.bag.
config file uri: .
AddressSanitizer:DEADLYSIGNAL
=================================================================
==5578==ERROR: AddressSanitizer: SEGV on unknown address 0x7585e818c5e8 (pc 0x7585eb5b1aaa bp 0x7ffd3714c700 sp 0x7ffd3714c6f0 T0)
==5578==The signal is caused by a READ memory access.
    #0 0x7585eb5b1aaa in std::_Sp_counted_ptr<rosbag2_storage::storage_interfaces::ReadWriteInterface*, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (/home/shangzh/ros2_jazzy/install/rosbag2_storage/lib/librosbag2_storage.so+0x18faaa) (BuildId: 9356fba64f173b3892930a1219d31e451df57aea)
    #1 0x5b1eb81d21f3 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb41f3) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #2 0x5b1eb81d30f7 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb50f7) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #3 0x5b1eb81d2eb7 in std::__shared_ptr<rosbag2_storage::storage_interfaces::ReadWriteInterface, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb4eb7) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #4 0x5b1eb81d2ed7 in std::shared_ptr<rosbag2_storage::storage_interfaces::ReadWriteInterface>::~shared_ptr() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb4ed7) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #5 0x5b1eb81d1952 in StorageFactoryTest_test_Test::TestBody() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb3952) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #6 0x5b1eb8273a21 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x155a21) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #7 0x5b1eb8260de9 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x142de9) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #8 0x5b1eb82075bf in testing::Test::Run() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xe95bf) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #9 0x5b1eb8208d9b in testing::TestInfo::Run() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xead9b) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #10 0x5b1eb820a0f4 in testing::TestSuite::Run() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xec0f4) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #11 0x5b1eb8230e1f in testing::internal::UnitTestImpl::RunAllTests() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x112e1f) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #12 0x5b1eb8276e7c in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x158e7c) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #13 0x5b1eb8264086 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x146086) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #14 0x5b1eb822d429 in testing::UnitTest::Run() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0x10f429) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #15 0x5b1eb81d78ca in RUN_ALL_TESTS() (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb98ca) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #16 0x5b1eb81d7816 in main (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb9816) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)
    #17 0x7585e962a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #18 0x7585e962a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #19 0x5b1eb81d1154 in _start (/home/shangzh/rosbag2_test/build/rosbag2_storage/test_storage_factory+0xb3154) (BuildId: d99f98e73cb208d0c124722d27d1ac9f4889eb5f)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/shangzh/ros2_jazzy/install/rosbag2_storage/lib/librosbag2_storage.so+0x18faaa) (BuildId: 9356fba64f173b3892930a1219d31e451df57aea) in std::_Sp_counted_ptr<rosbag2_storage::storage_interfaces::ReadWriteInterface*, (__gnu_cxx::_Lock_policy)2>::_M_dispose()
==5578==ABORTING

System (please complete the following information)

OS: ubuntu 24.04
ROS 2 Distro: ros 2 jazzy
Install Method: source
Version: ros 2 jazzy
build options: --mixin asan-gcc

[Barry-Xu-2018]

The destructor of storage depends on libtest_plugin.so, which is dynamically loaded. When local_factory is destructed, all loaded libraries (including libtest_plugin.so) will be unloaded.
So an exception is thrown while destructor storage.

Based on the current implementation, it is the user's responsibility to ensure that any std::shared_ptr<ReadWriteInterface> or std::shared_ptr<ReadOnlyInterface> obtained by calling open_read_write() or open_read_only() is released before the rosbag2_storage::StorageFactory is destructed.

At present, no simple modification has been found to ensure this order. I suggest adding a note to describe this "limitation" at function declaration.

rosbag2/rosbag2_storage/include/rosbag2_storage/storage_factory.hpp

Lines 51 to 55 in e02dc60

	std::shared_ptr<storage_interfaces::ReadOnlyInterface>
	open_read_only(const StorageOptions & storage_options) override;
	std::shared_ptr<storage_interfaces::ReadWriteInterface>
	open_read_write(const StorageOptions & storage_options) override;