From 52abd45f2bc9ad36589aa5111036ea588981d5ff Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Mon, 20 Jan 2020 19:06:42 +0900 Subject: [PATCH] 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 --- ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc | 50 ++++++++++++++++++++--- ext/nnstreamer/tensor_filter/tensor_filter_cpp.hh | 14 ++++++- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc index 8d29fbf..f4163d0 100644 --- a/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc +++ b/ext/nnstreamer/tensor_filter/tensor_filter_cpp.cc @@ -27,6 +27,7 @@ */ #include #include +#include #include #include @@ -39,8 +40,11 @@ #include "tensor_filter_cpp.hh" std::unordered_map tensor_filter_cpp::filters; +std::vector 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 --- 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 +#include #include #include #include -#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 filters; + static std::vector 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 */ -- 2.7.4