Increase coverage with a graveyard behavior test and unmanaged instance test

QUALITY_DECLARATION.md

@@ -114,7 +114,9 @@ This includes:
 
 Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
 
-Current coverage statistics can be viewed [here](https://ci.ros2.org/job/ci_linux_coverage/85/cobertura/src_ros_class_loader_include_class_loader/) and [here](https://ci.ros2.org/job/ci_linux_coverage/85/cobertura/src_ros_class_loader_include_class_loader/). This package does not yet meet the 95% coverage guideline, but it is currently above 90%.
+This package has testing coverage of at least 95%.
+Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/).
+A description of how coverage statistics are calculated is summarized in the [ROS 2 On-boarding Guide](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
 
 ### Performance [4.iv]
 

test/CMakeLists.txt

@@ -26,6 +26,20 @@ endif()
 target_link_libraries(${PROJECT_NAME}_TestPlugins2 ${PROJECT_NAME})
 class_loader_hide_library_symbols(${PROJECT_NAME}_TestPlugins2)
 
+# These plugins are loaded using dlopen() with RTLD_GLOBAL in utest and may cause side effects
+# in other tests if they are used elsewhere.
+add_library(${PROJECT_NAME}_TestGlobalPlugins EXCLUDE_FROM_ALL SHARED global_plugins.cpp)
+if(ament_cmake_FOUND)
+  target_include_directories(${PROJECT_NAME}_TestGlobalPlugins
+    PUBLIC "../include")
+  ament_target_dependencies(${PROJECT_NAME}_TestGlobalPlugins "console_bridge")
+else()
+  target_include_directories(${PROJECT_NAME}_TestGlobalPlugins
+    PUBLIC "../include" ${console_bridge_INCLUDE_DIRS})
+endif()
+target_link_libraries(${PROJECT_NAME}_TestGlobalPlugins ${PROJECT_NAME})
+class_loader_hide_library_symbols(${PROJECT_NAME}_TestGlobalPlugins)
+
 if(WIN32)
   set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>;$<TARGET_FILE_DIR:${PROJECT_NAME}_TestPlugins1>")
 else()
@@ -52,7 +66,8 @@ if(TARGET ${PROJECT_NAME}_utest)
   endif()
   add_dependencies(${PROJECT_NAME}_utest
     ${PROJECT_NAME}_TestPlugins1
-    ${PROJECT_NAME}_TestPlugins2)
+    ${PROJECT_NAME}_TestPlugins2
+    ${PROJECT_NAME}_TestGlobalPlugins)
 endif()
 
 ament_add_gtest(${PROJECT_NAME}_unique_ptr_test unique_ptr_test.cpp

test/global_plugins.cpp

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2012, Willow Garage, Inc.
+ * All rights reserved.
+ * Copyright (c) 2020, Open Source Robotics Foundation, Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of the Willow Garage, Inc. nor the names of its
+ *       contributors may be used to endorse or promote products derived from
+ *       this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * 2020, Copied from plugins1.cpp and plugins2.cpp and changing class names
+ */
+
+#include <iostream>
+
+#include "class_loader/class_loader.hpp"
+
+#include "./base.hpp"
+
+class Kangaroo : public Base
+{
+public:
+  void saySomething()
+  {
+    printf("[Angry growl]\n");
+  }
+};
+
+class Panda : public Base
+{
+public:
+  void saySomething()
+  {
+    printf("[Excited squeaks!!!]\n");
+  }
+};
+
+class Hyena : public Base
+{
+public:
+  void saySomething()
+  {
+    printf("[Cackling laugh]\n");
+  }
+};
+
+class Alpaca : public Base
+{
+public:
+  void saySomething()
+  {
+    printf("hhhaaaaaaaaaa\n");
+  }
+};
+
+
+CLASS_LOADER_REGISTER_CLASS(Kangaroo, Base)
+CLASS_LOADER_REGISTER_CLASS(Panda, Base)
+CLASS_LOADER_REGISTER_CLASS(Hyena, Base)
+CLASS_LOADER_REGISTER_CLASS(Alpaca, Base)

test/utest.cpp

@@ -29,6 +29,11 @@
 
 #include <chrono>
 #include <cstddef>
+
+#ifndef _WIN32
+#include <dlfcn.h>
+#endif
+
 #include <iostream>
 #include <memory>
 #include <string>
@@ -45,6 +50,11 @@
 const std::string LIBRARY_1 = class_loader::systemLibraryFormat("class_loader_TestPlugins1");  // NOLINT
 const std::string LIBRARY_2 = class_loader::systemLibraryFormat("class_loader_TestPlugins2");  // NOLINT
 
+// These are loaded with dlopen() and RTLD_GLOBAL in loadUnloadLoadFromGraveyard and may cause
+// unexpected side-effects if used elsewhere
+const std::string GLOBAL_PLUGINS =  // NOLINT
+  class_loader::systemLibraryFormat("class_loader_TestGlobalPlugins");
+
 TEST(ClassLoaderTest, basicLoad) {
   try {
     class_loader::ClassLoader loader1(LIBRARY_1, false);
@@ -57,6 +67,21 @@ TEST(ClassLoaderTest, basicLoad) {
   SUCCEED();
 }
 
+// Requires separate namespace so static variables are isolated
+TEST(ClassLoaderUnmanagedTest, basicLoadUnmanaged) {
+  try {
+    class_loader::ClassLoader loader1(LIBRARY_1, false);
+    Base * unmanaged_instance = loader1.createUnmanagedInstance<Base>("Dog");
+    ASSERT_NE(unmanaged_instance, nullptr);
+    unmanaged_instance->saySomething();
+    delete unmanaged_instance;
+  } catch (class_loader::ClassLoaderException & e) {
+    FAIL() << "ClassLoaderException: " << e.what() << "\n";
+  }
+
+  SUCCEED();
+}
+
 TEST(ClassLoaderUniquePtrTest, basicLoadFailures) {
   class_loader::ClassLoader loader1(LIBRARY_1, false);
   class_loader::ClassLoader loader2("", false);
@@ -337,6 +362,53 @@ TEST(MultiClassLoaderTest, noWarningOnLazyLoad) {
   SUCCEED();
 }
 
+#ifndef _WIN32
+// Not run on Windows because this tests dlopen-specific behavior
+
+// This is a different class name so that static variables in ClassLoader are isolated
+TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) {
+  // This first load/unload adds the plugin to the graveyard
+  try {
+    class_loader::ClassLoader loader(GLOBAL_PLUGINS, false);
+    loader.createInstance<Base>("Kangaroo")->saySomething();
+    loader.unloadLibrary();
+  } catch (class_loader::ClassLoaderException & e) {
+    FAIL() << "ClassLoaderException: " << e.what() << "\n";
+  }
+
+  // Not all platforms use RTLD_GLOBAL as a default, and rcutils doesn't explicitly choose either.
+  // In order to invoke graveyard behavior, this needs to be loaded first for global relocation.
+
+  void * handle = dlopen(GLOBAL_PLUGINS.c_str(), RTLD_NOW | RTLD_GLOBAL);
+  ASSERT_NE(handle, nullptr);
+
+  // This load will cause system to use globally relocatable library.
+  // For testing purposes, this will cause ClassLoader to revive the library from the graveyard.
+  try {
+    class_loader::ClassLoader loader(GLOBAL_PLUGINS, false);
+    loader.createInstance<Base>("Panda")->saySomething();
+    loader.unloadLibrary();
+
+    loader.loadLibrary();
+    loader.createInstance<Base>("Hyena")->saySomething();
+    loader.unloadLibrary();
+  } catch (class_loader::ClassLoaderException & e) {
+    FAIL() << "ClassLoaderException: " << e.what() << "\n";
+  }
+
+  dlclose(handle);
+  // With all libraries closed, this should act like a normal load/unload.
+  try {
+    class_loader::ClassLoader loader(GLOBAL_PLUGINS, false);
+    loader.createInstance<Base>("Alpaca")->saySomething();
+    loader.unloadLibrary();
+  } catch (class_loader::ClassLoaderException & e) {
+    FAIL() << "ClassLoaderException: " << e.what() << "\n";
+  }
+}
+
+#endif  // ifndef _WIN32
+
 // Run all the tests that were declared with TEST()
 int main(int argc, char ** argv)
 {