Accept null dlhandle and support custom dlopen flags

loader/include/ignition/plugin/Loader.hh

@@ -48,6 +48,17 @@ namespace ignition
       /// \returns A pretty string
       public: std::string PrettyStr() const;
 
+      /// \brief Set custom flags to be used to dlopen libraries.
+      /// Defaults to (RTLD_LAZY | RTLD_LOCAL).
+      ///
+      /// \param[in] _flags Flags for dlopen.
+      public: void SetFlags(int _flags);
+
+      /// \brief Get custom flags to be used to dlopen libraries.
+      ///
+      /// \return Flags for dlopen.
+      public: int Flags() const;
+
       /// \brief Get demangled names of interfaces that the loader has plugins
       /// for.
       ///
@@ -124,7 +135,7 @@ namespace ignition
       /// if no such plugin is known.
       public: std::string LookupPlugin(const std::string &_nameOrAlias) const;
 
-      /// \brief Load a library at the given path
+      /// \brief Load a library at the given path.
       ///
       /// \param[in] _pathToLibrary
       ///   The path to a library

loader/src/Loader.cc

@@ -122,6 +122,9 @@ namespace ignition
       /// \brief A map from the shared library handle to the names of the
       /// plugins that it provides.
       public: DlHandleToPluginMap dlHandleToPluginMap;
+
+      /// \brief Flags to dlopen.
+      public: int flags{RTLD_LAZY | RTLD_LOCAL};
     };
 
     /////////////////////////////////////////////////
@@ -201,6 +204,18 @@ namespace ignition
       // Do nothing.
     }
 
+    /////////////////////////////////////////////////
+    void Loader::SetFlags(int _flags)
+    {
+      this->dataPtr->flags = _flags;
+    }
+
+    /////////////////////////////////////////////////
+    int Loader::Flags() const
+    {
+      return this->dataPtr->flags;
+    }
+
     /////////////////////////////////////////////////
     std::unordered_set<std::string> Loader::LoadLib(
         const std::string &_pathToLibrary)
@@ -368,7 +383,7 @@ namespace ignition
 #endif
 
       void *dlHandle = dlopen(_pathToLibrary.c_str(),
-                              RTLD_NOLOAD | RTLD_LAZY | RTLD_LOCAL);
+                              RTLD_NOLOAD | this->dataPtr->flags);
 
       if (!dlHandle)
         return false;
@@ -454,7 +469,7 @@ namespace ignition
 
       // NOTE: We open using RTLD_LOCAL instead of RTLD_GLOBAL to prevent the
       // symbols of different libraries from writing over each other.
-      void *dlHandle = dlopen(_full_path.c_str(), RTLD_LAZY | RTLD_LOCAL);
+      void *dlHandle = dlopen(_full_path.c_str(), this->flags);
 
       const char *loadError = dlerror();
       if (nullptr == dlHandle || nullptr != loadError)
@@ -536,12 +551,6 @@ namespace ignition
         const std::string& _pathToLibrary) const
     {
       std::vector<Info> loadedPlugins;
-
-      // This function should never be called with a nullptr _dlHandle
-      assert(_dlHandle &&
-             "Bug in code: Loader::Implementation::LoadPlugins was called with "
-             "a nullptr value for _dlHandle.");
-
       const std::string infoSymbol = "IgnitionPluginHook";
       void *infoFuncPtr = dlsym(_dlHandle.get(), infoSymbol.c_str());
 
@@ -556,7 +565,7 @@ namespace ignition
       }
 
       using PluginLoadFunctionSignature =
-          void(*)(void * const, const void ** const,
+          void(*)(const void *, const void ** const,
                   int *, std::size_t *, std::size_t *);
 
       // Note: InfoHook (below) is a function with a signature that matches
@@ -636,6 +645,15 @@ namespace ignition
         loadedPlugins.push_back(info.second);
       }
 
+      // If no plugins could be loaded, try passing a null handle to dlsym
+      if (loadedPlugins.size() == 0)
+      {
+        if (_dlHandle != nullptr)
+        {
+          return LoadPlugins(nullptr, _pathToLibrary);
+        }
+      }
+
       return loadedPlugins;
     }
 

loader/src/Loader_TEST.cc

@@ -15,6 +15,8 @@
  *
 */
 
+#include <dlfcn.h>
+
 #include <gtest/gtest.h>
 
 #include <algorithm>
@@ -64,6 +66,15 @@ TEST(Loader, InstantiateUnloadedPlugin)
   EXPECT_FALSE(loader.ForgetLibraryOfPlugin("plugin::that::is::not::loaded"));
 }
 
+/////////////////////////////////////////////////
+TEST(Loader, SetFlags)
+{
+  ignition::plugin::Loader loader;
+  EXPECT_EQ((RTLD_LAZY | RTLD_LOCAL), loader.Flags());
+  loader.SetFlags(RTLD_LAZY | RTLD_GLOBAL);
+  EXPECT_EQ((RTLD_LAZY | RTLD_GLOBAL), loader.Flags());
+}
+
 class SomeInterface { };
 
 /////////////////////////////////////////////////

test/integration/CMakeLists.txt

@@ -20,6 +20,7 @@ foreach(test ${test_targets})
       IGNBadPluginSize
       IGNDummyPlugins
       IGNFactoryPlugins
+      IGNLoadsAnotherPlugin
       IGNTemplatedPlugins)
 
     target_compile_definitions(${test} PRIVATE

test/integration/flags.cc

@@ -0,0 +1,217 @@
+/*
+ * Copyright (C) 2021 Open Source Robotics Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+*/
+
+#include <dlfcn.h>
+#include <gtest/gtest.h>
+
+#include <ignition/plugin/Loader.hh>
+
+#include "../plugins/LoadsAnotherPlugin.hh"
+
+/////////////////////////////////////////////////
+TEST(Flags, DefaultFlags)
+{
+  ignition::plugin::Loader loader;
+  EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+
+  // Load plugin
+  auto loadedByTest = loader.LoadLib(IGNLoadsAnotherPlugin_LIB);
+  EXPECT_EQ(1u, loader.InterfacesImplemented().size());
+  EXPECT_EQ(1u, loadedByTest.size());
+  ASSERT_EQ(1u, loadedByTest.count("test::util::LoadsAnotherPlugin"));
+
+  auto plugin = loader.Instantiate(*loadedByTest.begin());
+  EXPECT_FALSE(plugin.IsEmpty());
+
+  auto interface = plugin->QueryInterface<
+      test::util::LoadsAnotherPluginInterface>();
+  ASSERT_NE(nullptr, interface);
+
+  // Load other plugins from this plugin
+  auto loadedByPlugin = interface->Load();
+  EXPECT_EQ(3u, loadedByPlugin.size());
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummySinglePlugin"));
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyMultiPlugin"));
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyNoAliasPlugin"));
+
+  // Load another plugin
+  loadedByTest = loader.LoadLib(IGNTemplatedPlugins_LIB);
+  EXPECT_EQ(5u, loader.InterfacesImplemented().size());
+  EXPECT_EQ(4u, loadedByTest.size());
+  ASSERT_EQ(1u, loadedByTest.count(
+      "test::plugins::GenericTemplatePlugin<std::__cxx11::basic_string"
+      "<char, std::char_traits<char>, std::allocator<char> > >"));
+  ASSERT_EQ(1u, loadedByTest.count(
+      "test::plugins::GenericTemplatePlugin<int>"));
+  ASSERT_EQ(1u, loadedByTest.count("test::plugins::StringTemplatePlugin"));
+  ASSERT_EQ(1u, loadedByTest.count("test::plugins::DoubleTemplatePlugin"));
+
+  // Forget
+  EXPECT_TRUE(interface->Unload());
+  EXPECT_TRUE(loader.ForgetLibrary(IGNLoadsAnotherPlugin_LIB));
+  EXPECT_TRUE(loader.ForgetLibrary(IGNTemplatedPlugins_LIB));
+  EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+}
+
+/////////////////////////////////////////////////
+TEST(Flags, ChildPluginsGlobal)
+{
+  ignition::plugin::Loader loader;
+  EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+
+  // Load plugin
+  auto loadedByTest = loader.LoadLib(IGNLoadsAnotherPlugin_LIB);
+  EXPECT_EQ(1u, loader.InterfacesImplemented().size());
+  EXPECT_EQ(1u, loadedByTest.size());
+  ASSERT_EQ(1u, loadedByTest.count("test::util::LoadsAnotherPlugin"));
+
+  auto plugin = loader.Instantiate(*loadedByTest.begin());
+  EXPECT_FALSE(plugin.IsEmpty());
+
+  auto interface = plugin->QueryInterface<
+      test::util::LoadsAnotherPluginInterface>();
+  ASSERT_NE(nullptr, interface);
+
+  // Load other plugins from this plugin, globally
+  auto loadedByPlugin = interface->Load(RTLD_LAZY | RTLD_GLOBAL);
+  EXPECT_EQ(3u, loadedByPlugin.size());
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummySinglePlugin"));
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyMultiPlugin"));
+  ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyNoAliasPlugin"));
+
+  // Load another plugin
+  loadedByTest = loader.LoadLib(IGNTemplatedPlugins_LIB);
+  EXPECT_EQ(12u, loader.InterfacesImplemented().size());
+  EXPECT_EQ(7u, loadedByTest.size());
+  ASSERT_EQ(1u, loadedByTest.count(
+      "test::plugins::GenericTemplatePlugin<std::__cxx11::basic_string"
+      "<char, std::char_traits<char>, std::allocator<char> > >"));
+  ASSERT_EQ(1u, loadedByTest.count(
+      "test::plugins::GenericTemplatePlugin<int>"));
+  ASSERT_EQ(1u, loadedByTest.count("test::plugins::StringTemplatePlugin"));
+  ASSERT_EQ(1u, loadedByTest.count("test::plugins::DoubleTemplatePlugin"));
+  ASSERT_EQ(1u, loadedByTest.count("test::util::DummySinglePlugin"));
+  ASSERT_EQ(1u, loadedByTest.count("test::util::DummyMultiPlugin"));
+  ASSERT_EQ(1u, loadedByTest.count("test::util::DummyNoAliasPlugin"));
+
+  // Forget
+  EXPECT_TRUE(interface->Unload());
+  EXPECT_TRUE(loader.ForgetLibrary(IGNLoadsAnotherPlugin_LIB));
+  EXPECT_TRUE(loader.ForgetLibrary(IGNTemplatedPlugins_LIB));
+  EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+}
+
+/////////////////////////////////////////////////
+TEST(Flags, ParentPluginsGlobal)
+{
+  // ignition::plugin::Loader loader;
+  // loader.SetFlags(RTLD_LAZY | RTLD_GLOBAL);
+  // EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+
+  // // Load plugin
+  // auto loadedByTest = loader.LoadLib(IGNLoadsAnotherPlugin_LIB);
+  // EXPECT_EQ(1u, loader.InterfacesImplemented().size());
+  // EXPECT_EQ(1u, loadedByTest.size());
+  // ASSERT_EQ(1u, loadedByTest.count("test::util::LoadsAnotherPlugin"));
+
+  // auto plugin = loader.Instantiate(*loadedByTest.begin());
+  // EXPECT_FALSE(plugin.IsEmpty());
+
+  // auto interface = plugin->QueryInterface<
+  //     test::util::LoadsAnotherPluginInterface>();
+  // ASSERT_NE(nullptr, interface);
+
+  // // Load other plugins from this plugin, it gets the global plugin too
+  // auto loadedByPlugin = interface->Load();
+  // EXPECT_EQ(4u, loadedByPlugin.size());
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummySinglePlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyMultiPlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyNoAliasPlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::LoadsAnotherPlugin"));
+
+  // // Load another plugin
+  // loadedByTest = loader.LoadLib(IGNTemplatedPlugins_LIB);
+  // EXPECT_EQ(5u, loader.InterfacesImplemented().size());
+  // EXPECT_EQ(4u, loadedByTest.size());
+  // ASSERT_EQ(1u, loadedByTest.count(
+  //     "test::plugins::GenericTemplatePlugin<std::__cxx11::basic_string"
+  //     "<char, std::char_traits<char>, std::allocator<char> > >"));
+  // ASSERT_EQ(1u, loadedByTest.count(
+  //     "test::plugins::GenericTemplatePlugin<int>"));
+  // ASSERT_EQ(1u, loadedByTest.count("test::plugins::StringTemplatePlugin"));
+  // ASSERT_EQ(1u, loadedByTest.count("test::plugins::DoubleTemplatePlugin"));
+
+  // // Forget
+  // EXPECT_TRUE(interface->Unload());
+  // EXPECT_TRUE(loader.ForgetLibrary(IGNLoadsAnotherPlugin_LIB));
+  // // EXPECT_TRUE(loader.ForgetLibrary(IGNTemplatedPlugins_LIB));
+  // EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+}
+
+/////////////////////////////////////////////////
+TEST(Flags, AllPluginsGlobal)
+{
+  ignition::plugin::Loader loader;
+  loader.SetFlags(RTLD_LAZY | RTLD_GLOBAL);
+  EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+
+  // // Load plugin
+  // auto loadedByTest = loader.LoadLib(IGNLoadsAnotherPlugin_LIB);
+  // EXPECT_EQ(1u, loader.InterfacesImplemented().size());
+  // EXPECT_EQ(1u, loadedByTest.size());
+  // ASSERT_EQ(1u, loadedByTest.count("test::util::LoadsAnotherPlugin"));
+
+  // auto plugin = loader.Instantiate(*loadedByTest.begin());
+  // EXPECT_FALSE(plugin.IsEmpty());
+
+  // auto interface = plugin->QueryInterface<
+  //     test::util::LoadsAnotherPluginInterface>();
+  // ASSERT_NE(nullptr, interface);
+
+  // // Load other plugins from this plugin, it gets the global plugin too
+  // auto loadedByPlugin = interface->Load(RTLD_LAZY | RTLD_GLOBAL);
+  // EXPECT_EQ(4u, loadedByPlugin.size());
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummySinglePlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyMultiPlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::DummyNoAliasPlugin"));
+  // ASSERT_EQ(1u, loadedByPlugin.count("test::util::LoadsAnotherPlugin"));
+
+  // // Load another plugin
+  // loadedByTest = loader.LoadLib(IGNTemplatedPlugins_LIB);
+  // EXPECT_EQ(5u, loader.InterfacesImplemented().size());
+  // EXPECT_EQ(4u, loadedByTest.size());
+  // ASSERT_EQ(1u, loadedByTest.count(
+  //     "test::plugins::GenericTemplatePlugin<std::__cxx11::basic_string"
+  //     "<char, std::char_traits<char>, std::allocator<char> > >"));
+  // ASSERT_EQ(1u, loadedByTest.count(
+  //     "test::plugins::GenericTemplatePlugin<int>"));
+  // ASSERT_EQ(1u, loadedByTest.count("test::plugins::StringTemplatePlugin"));
+  // ASSERT_EQ(1u, loadedByTest.count("test::plugins::DoubleTemplatePlugin"));
+
+  // // Forget
+  // EXPECT_TRUE(interface->Unload());
+  // EXPECT_TRUE(loader.ForgetLibrary(IGNLoadsAnotherPlugin_LIB));
+  // // EXPECT_TRUE(loader.ForgetLibrary(IGNTemplatedPlugins_LIB));
+  // EXPECT_EQ(0u, loader.InterfacesImplemented().size());
+}
+
+/////////////////////////////////////////////////
+int main(int argc, char **argv)
+{
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/plugins/CMakeLists.txt

@@ -10,6 +10,14 @@ add_library(IGNDummyPlugins SHARED
   DummyPlugins.cc
   DummyPluginsOtherTranslationUnit.cc)
 
+add_library(IGNLoadsAnotherPlugin SHARED
+  LoadsAnotherPlugin.cc)
+
+target_link_libraries(IGNLoadsAnotherPlugin PRIVATE
+  ${PROJECT_LIBRARY_TARGET_NAME}-loader)
+target_compile_definitions(IGNLoadsAnotherPlugin PRIVATE
+  "IGNDummyPlugins_LIB=\"$<TARGET_FILE:IGNDummyPlugins>\"")
+
 # Create a variable for the name of the header which will contain the dummy plugin path.
 # This variable gets put in the cache so that it is available at generation time.
 foreach(plugin_target
@@ -20,6 +28,7 @@ foreach(plugin_target
     IGNBadPluginSize
     IGNDummyPlugins
     IGNFactoryPlugins
+    IGNLoadsAnotherPlugin
     IGNTemplatedPlugins)
 
   target_link_libraries(${plugin_target} PRIVATE