Fix bug when destructor of PluginManager throws exception (#1196)

* mark destructor as noexcept(false)
* write unittest to test that destructor throws exception correctly
* not to throw exception from destructor if stack is unwinding

Signed-off-by: Roman Rusyaev <r.rusyaev@samsung.com>

diff --git a/contrib/nnc/include/support/PluginManager.h b/contrib/nnc/include/support/PluginManager.h
index 0ae66d3..1d55894 100644 (file)
--- a/contrib/nnc/include/support/PluginManager.h
+++ b/contrib/nnc/include/support/PluginManager.h
@@ -29,7 +29,7 @@ public:
   /**
    * @throw PluginException if couldn't unload plugin
    */
-  ~PluginManager();
+  ~PluginManager() noexcept(false);
 
   /**
    * @brief get plugin

diff --git a/contrib/nnc/support/PluginManager.cpp b/contrib/nnc/support/PluginManager.cpp
index b9faccb..88ce9d5 100644 (file)
--- a/contrib/nnc/support/PluginManager.cpp
+++ b/contrib/nnc/support/PluginManager.cpp
@@ -3,6 +3,7 @@
 #include <dirent.h>
 #include <cstring>
 #include <vector>
+#include <exception>
 
 #include "support/PluginProxy.h"
 #include "support/PluginManager.h"
@@ -28,9 +29,16 @@ PluginManager::PluginManager(const std::string &plugin_path)
 
 } // PluginManager
 
-PluginManager::~PluginManager()
+PluginManager::~PluginManager() noexcept(false)
 {
-  unloadPlugin();
+  // if exception is already being thrown then we can't throw yet
+  // another exception from destructor because std::terminate will
+  // be called and terminates program so that we don't unload plugin
+  // because this function can throw exception
+  if ( !std::uncaught_exception() )
+  {
+    unloadPlugin();
+  }
 
 } // ~PluginManager
 

diff --git a/contrib/nnc/unittests/module/PluginManager.cpp b/contrib/nnc/unittests/module/PluginManager.cpp
index ad7e7d8..2f7652d 100644 (file)
--- a/contrib/nnc/unittests/module/PluginManager.cpp
+++ b/contrib/nnc/unittests/module/PluginManager.cpp
@@ -1,3 +1,5 @@
+#include <dlfcn.h>
+
 #include "support/CommandLine.h"
 #include "support/PluginManager.h"
 
@@ -36,3 +38,54 @@ TEST(CONTRIB_NNC, PluginManager)
   std::string plugin(STRING(CMAKE_SAMPLE_PLUGIN_ABS_PATH) "\n");
   EXPECT_TRUE(os.str() == plugin );
 }
+
+// Unload shared library
+static void unloadShared(std::string plugin_path)
+{
+  void *handle = dlopen(plugin_path.c_str(), RTLD_LAZY);
+  assert(handle);
+
+  // try to close library until it is closed
+  for ( int i = 0; !dlclose(handle) && i < 10; i++ ) ;
+
+} // unloadShared
+
+// Test PluginManager that destructor throws exception correctly
+TEST(SUPPORT_NNC, verifyPluginManager4UnloadPlugin)
+{
+  std::string plugin_path{STRING(CMAKE_SAMPLE_PLUGIN_ABS_PATH)};
+  bool is_thrown = false;
+
+  // test that destructor throws exception
+  try
+  {
+    // load the library
+    PluginManager pluginManager(plugin_path);
+
+    // unload the library directly
+    unloadShared(plugin_path);
+  }
+  catch ( const PluginException &e )
+  {
+    // the exception must be thrown from destructor because the library is already unloaded
+    is_thrown = true;
+  }
+
+  ASSERT_EQ(is_thrown, true);
+
+  // test that destructor doesn't throw exception while stack is unwinding
+  try
+  {
+    // load the library
+    PluginManager pluginManager(plugin_path);
+
+    // unload the library directly
+    unloadShared(plugin_path);
+
+    throw PluginException("test exception");
+  }
+  catch (const PluginException &e)
+  {
+    ASSERT_EQ(e.what(), "test exception");
+  }
+}