Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,22 @@ See the [`ShapeFactory` plugin for an example implementation](examples/shape/sha

## Keep plugins in scope during use

Once the plugin object goes out of scope, the library providing it will be unloaded, resulting in undefined behavior and potential segfaults.
Thus, the plugin object must be kept in scope for as long as it (and objects created by it) are being used.
The plugin loader internally retains a pointer to all plugins that it has created.
Therefore, the plugin loader must be kept in scope for the lifetime of any plugin that it creates (or any object that the plugin creates).

Here is an example of what **not** to do:

```c++
boost_plugin_loader::PluginLoader<ShapeFactory> plugin_loader;

Shape::Ptr shape;

{
boost_plugin_loader::PluginLoader<ShapeFactory> plugin_loader;

ShapeFactory::Ptr square_factory = plugin_loader.createInstance("Square");
shape = square_factory.create(2.0);

// Library providing "Square" plugin is unloaded here when `square_factory` goes out of scope
// The library providing the definition of the `square_factory` plugin and `shape` objects is unloaded here when `plugin_looader` goes out of scope
}

std::cout << "Square area: " << shape->area() << std::endl; // <-- segfault because the library providing plugin factory (and the object generated by it) was unloaded
std::cout << "Square area: " << shape->area() << std::endl; // <-- segfault because the library providing plugin factory (and the object generated by it) was unloaded when `plugin_loader` went out of scope
```
6 changes: 6 additions & 0 deletions examples/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Example plugin definition and explict template instantiation of plugin loader for test plugin class
add_library(${PROJECT_NAME}_example_plugin plugin.cpp)
target_link_libraries(${PROJECT_NAME}_example_plugin PUBLIC ${PROJECT_NAME})
target_compile_definitions(${PROJECT_NAME}_example_plugin PUBLIC SECTION_PRINTER=printer SECTION_SHAPE=shape)
target_cxx_version(${PROJECT_NAME}_example_plugin PUBLIC VERSION 17)
target_clang_tidy(${PROJECT_NAME}_example_plugin ENABLE ${ENABLE_CLANG_TIDY})

Expand All @@ -15,6 +16,11 @@ target_link_libraries(${PROJECT_NAME}_example_plugin_impl PUBLIC ${PROJECT_NAME}
target_cxx_version(${PROJECT_NAME}_example_plugin_impl PUBLIC VERSION 17)
target_clang_tidy(${PROJECT_NAME}_example_plugin_impl ENABLE ${ENABLE_CLANG_TIDY})

# Add a target compile definition to the plugin base library denoting the name of the library in which the plugins are
# implemented
target_compile_definitions(${PROJECT_NAME}_example_plugin
PUBLIC PLUGIN_LIBRARY_EXAMPLES="${PROJECT_NAME}_example_plugin_impl")

# Example
add_executable(${PROJECT_NAME}_example example.cpp)
target_link_libraries(${PROJECT_NAME}_example PRIVATE ${PROJECT_NAME}_example_plugin)
Expand Down
7 changes: 4 additions & 3 deletions examples/example.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ void demoPrinterPlugins(const PluginLoader& loader)
const Printer::Ptr plugin = loader.createInstance<Printer>(plugin_name);
assert(plugin != nullptr);
plugin->operator()();
// Note: `plugin` goes out of scope here, and the library providing its definition will be unloaded
// Note: `plugin` goes out of scope here. However, the plugin loader maintains a reference to it, so the library
// providing its definition will not be unload until the plugin loader goes out of scope.
}
}

Expand All @@ -60,8 +61,8 @@ void demoShapePlugins(const PluginLoader& loader)
assert(shape_plugins.size() == 2);

// Create the square and triangle factory plugins
// Note: these factories must stay in scope as long as they and any object they create are being used. Otherwise, the
// library providing them will be unloaded, resulting in undefined behavior and potential segfaults
// Note: the plugin loader must stay in scope as long as the plugins and any object the plugins create are being used.
// Otherwise, the library providing them will be unloaded, resulting in undefined behavior and potential segfaults
auto square_factory = loader.createInstance<ShapeFactory>("Square");
auto triangle_factory = loader.createInstance<ShapeFactory>("Triangle");

Expand Down
8 changes: 6 additions & 2 deletions examples/plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,25 @@
#include <boost_plugin_loader/plugin_loader.h>
#include <boost_plugin_loader/plugin_loader.hpp> // NOLINT(misc-include-cleaner)

// Macros for converting a non-string target compile definition into a string
#define STRINGIFY_HELPER(x) #x
#define STRINGIFY(x) STRINGIFY_HELPER(x)

namespace boost_plugin_loader
{
// Define the section name for loading plugins of base class `Printer`
// This should match the section name specified in the plugin export macro for this class
std::string Printer::getSection()
{
return "printer";
return STRINGIFY(SECTION_PRINTER);
}
INSTANTIATE_PLUGIN_LOADER(Printer)

// Define the section name for loading plugins of base class `ShapeFactory`
// This should match the section name specified in the plugin export macro for this class
std::string ShapeFactory::getSection()
{
return "shape";
return STRINGIFY(SECTION_SHAPE);
}
INSTANTIATE_PLUGIN_LOADER(ShapeFactory)

Expand Down
2 changes: 1 addition & 1 deletion examples/printer/printer.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ class Printer

#include <boost_plugin_loader/macros.h>
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
#define EXPORT_PRINTER_PLUGIN(DERIVED_CLASS, ALIAS) EXPORT_CLASS_SECTIONED(DERIVED_CLASS, ALIAS, printer)
#define EXPORT_PRINTER_PLUGIN(DERIVED_CLASS, ALIAS) EXPORT_CLASS_SECTIONED(DERIVED_CLASS, ALIAS, SECTION_PRINTER)
2 changes: 1 addition & 1 deletion examples/shape/shape.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ class ShapeFactory

#include <boost_plugin_loader/macros.h>
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
#define EXPORT_SHAPE_PLUGIN(DERIVED_CLASS, ALIAS) EXPORT_CLASS_SECTIONED(DERIVED_CLASS, ALIAS, shape)
#define EXPORT_SHAPE_PLUGIN(DERIVED_CLASS, ALIAS) EXPORT_CLASS_SECTIONED(DERIVED_CLASS, ALIAS, SECTION_SHAPE)
7 changes: 7 additions & 0 deletions include/boost_plugin_loader/plugin_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <string>
#include <memory>
#include <vector>
#include <mutex>

/** @brief Macro for explicitly template instantiating a plugin loader for a given base class */
#define INSTANTIATE_PLUGIN_LOADER(PluginBase) \
Expand Down Expand Up @@ -176,6 +177,12 @@ class PluginLoader
template <class ClassBase>
typename std::enable_if<has_getSection<ClassBase>::value, bool>::type hasSymbol(const boost::dll::shared_library& lib,
const std::string& symbol_name) const;

/**
* @brief Container for loaded plugins to keep them in scope as long as the plugin loader is alive
*/
mutable std::vector<std::shared_ptr<void>> loaded_plugins_;
mutable std::mutex plugin_cache_mutex_;
};

} // namespace boost_plugin_loader
Expand Down
10 changes: 9 additions & 1 deletion include/boost_plugin_loader/plugin_loader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,15 @@ std::shared_ptr<PluginBase> PluginLoader::createInstance(const std::string& plug
for (const auto& lib : libraries)
{
if (hasSymbol<PluginBase>(lib, plugin_name))
return createSharedInstance<PluginBase>(lib, plugin_name);
{
auto plugin = createSharedInstance<PluginBase>(lib, plugin_name);

// Add the plugin to the internal cache to keep it in scope for the lifetime of the plugin loader
std::lock_guard<std::mutex> lock{ plugin_cache_mutex_ };
loaded_plugins_.push_back(std::dynamic_pointer_cast<void>(plugin));

return plugin;
}
}

std::stringstream msg;
Expand Down
7 changes: 4 additions & 3 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ target_link_libraries(
PRIVATE GTest::GTest
GTest::Main
${PROJECT_NAME}
${PROJECT_NAME}_test_plugin)
${PROJECT_NAME}_test_plugin
${PROJECT_NAME}_example_plugin)
target_compile_definitions(${PROJECT_NAME}_plugin_loader_unit PRIVATE ${COMPILE_DEFINITIONS})
target_compile_definitions(
${PROJECT_NAME}_plugin_loader_unit
PRIVATE PLUGIN_DIR="${CMAKE_CURRENT_BINARY_DIR}" PLUGINS_MULTIPLY="${PROJECT_NAME}_test_plugin_multiply"
PLUGINS_ADD="${PROJECT_NAME}_test_plugin_add")
PRIVATE PLUGIN_DIR="${CMAKE_CURRENT_BINARY_DIR}" PLUGIN_LIBRARY_MULTIPLY="${PROJECT_NAME}_test_plugin_multiply"
PLUGIN_LIBRARY_ADD="${PROJECT_NAME}_test_plugin_add")
target_clang_tidy(${PROJECT_NAME}_plugin_loader_unit ENABLE ${ENABLE_CLANG_TIDY})
target_cxx_version(${PROJECT_NAME}_plugin_loader_unit PUBLIC VERSION 17)
target_code_coverage(
Expand Down
61 changes: 49 additions & 12 deletions test/plugin_loader_unit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@
#include <boost_plugin_loader/plugin_loader.h>
#include <boost_plugin_loader/plugin_loader.hpp>
#include "test_plugin.h"
#include "../examples/shape/shape.h"

TEST(BoostPluginLoaderUnit, Utils) // NOLINT
{
using namespace boost_plugin_loader;
const std::string lib_name = std::string(PLUGINS_MULTIPLY);
const std::string lib_name = std::string(PLUGIN_LIBRARY_MULTIPLY);
const std::string lib_dir = std::string(PLUGIN_DIR);

{
Expand Down Expand Up @@ -172,7 +173,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
{
PluginLoader plugin_loader;
plugin_loader.search_paths.insert(std::string(PLUGIN_DIR));
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));

EXPECT_TRUE(plugin_loader.isPluginAvailable(getSymbolName()));
auto plugin = plugin_loader.createInstance<TestPluginMultiply>(getSymbolName());
Expand All @@ -197,7 +198,8 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT

{ // Use full path
PluginLoader plugin_loader;
const std::string full_path = boost_plugin_loader::decorate(std::string(PLUGINS_MULTIPLY), std::string(PLUGIN_DIR));
const std::string full_path =
boost_plugin_loader::decorate(std::string(PLUGIN_LIBRARY_MULTIPLY), std::string(PLUGIN_DIR));
plugin_loader.search_libraries.insert(full_path);

EXPECT_TRUE(plugin_loader.isPluginAvailable(getSymbolName()));
Expand Down Expand Up @@ -227,7 +229,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
PluginLoader plugin_loader;
EXPECT_EQ(plugin_loader.count(), 0);
EXPECT_TRUE(plugin_loader.empty());
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));
EXPECT_EQ(plugin_loader.count(), 1);
EXPECT_FALSE(plugin_loader.empty());

Expand All @@ -243,7 +245,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
PluginLoader plugin_loader;
plugin_loader.search_system_folders = false;
plugin_loader.search_paths.insert("does_not_exist");
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));

EXPECT_FALSE(plugin_loader.isPluginAvailable(getSymbolName()));
// Behavior change: used to return nullptr but now throws exception
Expand All @@ -255,7 +257,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
{
PluginLoader plugin_loader;
plugin_loader.search_system_folders = false;
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));

{
EXPECT_FALSE(plugin_loader.isPluginAvailable("does_not_exist"));
Expand Down Expand Up @@ -299,7 +301,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
{
PluginLoader plugin_loader;
plugin_loader.search_system_folders = false;
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));

// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
const std::vector<std::string> plugins = plugin_loader.getAvailablePlugins(TestPluginMultiply::getSection());
Expand All @@ -309,7 +311,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPlugin) // NOLINT
{
PluginLoader plugin_loader;
plugin_loader.search_system_folders = true;
plugin_loader.search_libraries.insert(std::string(PLUGINS_MULTIPLY));
plugin_loader.search_libraries.insert(std::string(PLUGIN_LIBRARY_MULTIPLY));

const std::vector<std::string> plugins = plugin_loader.getAvailablePlugins(TestPluginMultiply::getSection());
EXPECT_EQ(plugins.size(), 1);
Expand Down Expand Up @@ -362,7 +364,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPluginsSameSymbolDifferentSections) // NOLI
// Try to load an instance of `TestPluginAddImpl` from the library in which `TestPluginMultiplyImpl` was defined
{
PluginLoader plugin_loader;
plugin_loader.search_libraries.insert(PLUGINS_MULTIPLY);
plugin_loader.search_libraries.insert(PLUGIN_LIBRARY_MULTIPLY);
plugin_loader.search_paths.insert(PLUGIN_DIR);

// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
Expand All @@ -372,7 +374,7 @@ TEST(BoostPluginLoaderUnit, LoadTestPluginsSameSymbolDifferentSections) // NOLI
// Try to load an instance of `TestPluginMultiplyImpl` from the library in which `TestPluginAddImpl` was defined
{
PluginLoader plugin_loader;
plugin_loader.search_libraries.insert(PLUGINS_ADD);
plugin_loader.search_libraries.insert(PLUGIN_LIBRARY_ADD);
plugin_loader.search_paths.insert(PLUGIN_DIR);

// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
Expand All @@ -382,8 +384,8 @@ TEST(BoostPluginLoaderUnit, LoadTestPluginsSameSymbolDifferentSections) // NOLI
// Given both libraries, correctly load and use each plugin, even though they share the same symbol name
{
PluginLoader plugin_loader;
plugin_loader.search_libraries.insert(PLUGINS_ADD);
plugin_loader.search_libraries.insert(PLUGINS_MULTIPLY);
plugin_loader.search_libraries.insert(PLUGIN_LIBRARY_ADD);
plugin_loader.search_libraries.insert(PLUGIN_LIBRARY_MULTIPLY);
plugin_loader.search_paths.insert(PLUGIN_DIR);

// Load and use a multiply plugin
Expand All @@ -406,6 +408,41 @@ TEST(BoostPluginLoaderUnit, LoadTestPluginsSameSymbolDifferentSections) // NOLI
}
}

TEST(BoostPluginLoaderUnit, TestPluginLoaderScope) // NOLINT
Copy link
Contributor Author

@marip8 marip8 Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Levi-Armstrong this unit test appears to work whether or not the internal cache introduced in this PR exists.

According to ldd the unit test only links to libboost_plugin_loader_example_plugin.so and libbboost_plugin_loader.so (not the library in which the triangle plugin was implemented - libboost_plugin_loader_example_plugin_impl.so), so the unit test itself shouldn't be loading the plugin library as part of its runtime requirement.

Any idea why this would work without holding onto the pointer to the plugin internally?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the plugin_loader object which holds the pointer to the plugin (triangle factory)? So I think what you want is to place the creation of the plugin_loader object inside the brackets and have it go out of scope. Another option, if the method exist would be to remove the triangle factory from the plugin loader also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the plugin_loader object which holds the pointer to the plugin (triangle factory)?

Yes, this PR introduces this feature. However, the unit test still succeeds without the addition of this feature.

If this unit test is run after removing b587d66 it should fail because the library should be unloaded when the plugin (i.e., triangle_factory) goes out of scope. But it doesn't, and I'm wondering why it doesn't segfault at line 441

{
using boost_plugin_loader::PluginLoader;
using boost_plugin_loader::Shape;
using boost_plugin_loader::ShapeFactory;

// Create a plugin loader
PluginLoader plugin_loader;

// Configure the plugin loader to be able to find the shape plugins from the example subdirectory
plugin_loader.search_libraries.insert(PLUGIN_LIBRARY_EXAMPLES);
plugin_loader.search_paths.insert(PLUGIN_DIR);

// Create an instance of a triangle shape using a plugin that goes out of scope
Shape::Ptr triangle;
{
ShapeFactory::Ptr triangle_factory;
// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
ASSERT_NO_THROW(triangle_factory = plugin_loader.createInstance<ShapeFactory>("Triangle"));
// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
ASSERT_NO_THROW(triangle = triangle_factory->create(std::make_tuple<double, double>(2.0, 4.0)));

// The plugin factory goes out of scope here, but the plugin loader maintains a pointer to it internally, keeping
// the plugin library loaded
}

// Use an object created by a plugin whose local instance has gone out of scope but is still maintained in-scope by
// the plugin loader
// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
double area;
EXPECT_NO_THROW(area = triangle->area());
// NOLINTNEXTLINE(cppcoreguidelines-avoid-goto)
EXPECT_NEAR(area, 4.0, 1.0e-6);
}

int main(int argc, char** argv)
{
testing::InitGoogleTest(&argc, argv);
Expand Down
Loading