Filter/C++: Fix potential memory leak.

dlclose() should not be called if the target-library's contents may
be refered later.

dlclose() should be called at the exit of a pipeline to avoid
memory leaks of user applications.

Thus, we need to call dlclose for all handles at the ensured exit
point.

They are named with underbar (_) because register is a keyword.

Changes in v2:
- Added lock for handles
- Don't call dlclose at exit context if it's GLIBC 2.23

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>

diff --git a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc
index 8d29fbf..f4163d0 100644 (file)
--- a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc
+++ b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc
@@ -27,6 +27,7 @@
  */
 #include <iostream>
 #include <string>
+#include <assert.h>
 
 #include <dlfcn.h>
 #include <errno.h>
@@ -39,8 +40,11 @@
 #include "tensor_filter_cpp.hh"
 
 std::unordered_map<std::string, tensor_filter_cpp*> tensor_filter_cpp::filters;
+std::vector<void *> tensor_filter_cpp::handles;
+G_LOCK_DEFINE_STATIC (lock_handles);
 
 static gchar filter_subplugin_cpp[] = "cpp";
+bool tensor_filter_cpp::dlclose_all_called = false;
 
 static GstTensorFilterFramework NNS_support_cpp = {
   .name = filter_subplugin_cpp,
@@ -59,8 +63,9 @@ static GstTensorFilterFramework NNS_support_cpp = {
 
 #define loadClass(name, ptr) \
   class tensor_filter_cpp *name = (tensor_filter_cpp *) *(ptr); \
-  g_assert (*(ptr)); \
-  g_assert (name->isValid());
+  assert (false == dlclose_all_called); \
+  assert (*(ptr)); \
+  assert (name->isValid());
 
 /**
  * @brief Class constructor
@@ -200,16 +205,21 @@ int tensor_filter_cpp::open (const GstTensorFilterProperties *prop, void **priva
       return -EINVAL; /** Model file / name not found */
     }
 
-    /** At this stage, the constructor of the .so file should have registered
-      * itself with tensor_filter_cpp_register() */
-    dlclose (handle);
-
     if (filters.find(prop->model_files[0]) == filters.end()) {
       /** It's still not found. it's not there. */
+      dlclose (handle);
       g_printerr_once (__FILE__, __LINE__, "C++ custom filter %s is not found in %s.\n",
           prop->model_files[0], prop->model_files[1]);
       return -EINVAL;
     }
+
+    /** We do not know until when this handle might be required: user may
+      * invoke functions from it at anytime while the pipeline is not
+      * closed */
+    G_LOCK (lock_handles);
+    handles.push_back (handle);
+    G_UNLOCK (lock_handles);
+
   }
 
   *private_data = cpp = filters[prop->model_files[0]];
@@ -237,6 +247,33 @@ G_BEGIN_DECLS
 void init_filter_cpp (void) __attribute__ ((constructor));
 void fini_filter_cpp (void) __attribute__ ((destructor));
 
+/**
+ * @brief Call dlclose for all handle
+ */
+void tensor_filter_cpp::dlclose_all ()
+{
+  assert (false == dlclose_all_called);
+/**
+ * Ubuntu 16.04 / GLIBC 2.23 Workaround
+ * If we do dlclose at exit() function, it may incur
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1264556#c42
+ * , which is a GLIBC bug at 2.23.
+ * The corresponding error message is:
+ * Inconsistency detected by ld.so: dl-close.c: 811:
+ * _dl_close: Assertion `map->l_init_called' failed!
+ */
+#if defined(__GLIBC__) && (__GLIBC__ == 2) && (__GLIBC_MINOR__ == 23)
+  /* Do not call dlclose */
+#else
+  G_LOCK (lock_handles);
+  for (void *handle : handles) {
+    dlclose (handle);
+  }
+  G_UNLOCK (lock_handles);
+#endif
+  dlclose_all_called = true;
+}
+
 /** @brief Initialize this object for tensor_filter subplugin runtime register */
 void
 init_filter_cpp (void)
@@ -249,5 +286,6 @@ void
 fini_filter_cpp (void)
 {
   nnstreamer_filter_exit (NNS_support_cpp.name);
+  tensor_filter_cpp::dlclose_all ();
 }
 G_END_DECLS

diff --git a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.hh b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.hh
index 9b8525c..49e712f 100644 (file)
--- a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.hh
+++ b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.hh
@@ -50,17 +50,22 @@
  *          - - so's init() { myfilter fx("myfilter");
  *                            fx.register(); }
  *
+ *     User should aware that the model name of a filter should be
+ *     unique across all the .so files used in a pipeline. Otherwise,
+ *     "Already registered (-EINVAL)" may occur.
+ *
  */
 #ifndef __NNS_TENSOR_FITLER_CPP_H__
 #define __NNS_TENSOR_FITLER_CPP_H__
 
+#ifdef __cplusplus
+
 #include <atomic>
+#include <vector>
 #include <stdint.h>
 #include <unordered_map>
 #include <nnstreamer_plugin_api_filter.h>
 
-#ifdef __cplusplus
-
 /**
  * @brief This allows to have a c++ class inserted as a filter in a neural network pipeline of nnstreamer
  * @note This is experimental.
@@ -76,6 +81,8 @@ class tensor_filter_cpp {
 
     std::atomic_uint ref_count;
     static std::unordered_map<std::string, tensor_filter_cpp*> filters;
+    static std::vector<void *> handles;
+    static bool dlclose_all_called;
 
   protected:
     const GstTensorFilterProperties *prop;
@@ -106,6 +113,7 @@ class tensor_filter_cpp {
            (constructed) by tensor_filter at run-time.
            If you don't want to touch initial ref_count, keep it 0.
       */
+    /** @brief Register this instance (same effect with __register) */
     int _register(unsigned int ref_count = 0) {
       return __register(this, ref_count);
     }
@@ -113,6 +121,7 @@ class tensor_filter_cpp {
       /**< Unregister the run-time registered c++ filter.
            Do not call this to filters loaded as a .so (independent)
            filter. */
+    /** @brief Unregister this instance (same effect with __unregister */
     int _unregister() {
       return __unregister(this->name);
     }
@@ -127,6 +136,7 @@ class tensor_filter_cpp {
     static int invoke (const GstTensorFilterProperties *prop, void **private_data, const GstTensorMemory *input, GstTensorMemory *output);
     static int open (const GstTensorFilterProperties *prop, void **private_data);
     static void close (const GstTensorFilterProperties *prop, void **private_data);
+    static void dlclose_all (void);
 };
 
 #endif /* __cplusplus */