From 2d733998cf865db08b652deeb2f59733a8acb57e Mon Sep 17 00:00:00 2001 From: Pierre Wielders Date: Wed, 28 Feb 2024 08:44:43 +0100 Subject: [PATCH] [LIBRARIES] Refernce counted Library COMRPC objects. (#1533) * [LIBRARIES] Refernce counted Library COMRPC objects. If COMRPC objects are created through the C interface, the object itself should hold on to a reference of the library it is created from this way, the library will not be unloaded as long as there are still COMRPC object living which reside in this SO. This PR makes sure that any instance instantiated through Core::ServiceType will also grab a refenrence onto the SO it is located in. On deletion (Last reference release of the COMRPC object) this reference on the SO will be dropped as well! * Update Library.cpp * Cleanup Services.h --- Source/core/Library.cpp | 85 +++++++++++++++++++++++++++-------------- Source/core/Library.h | 1 + Source/core/Services.h | 77 +++++++++++++++++-------------------- 3 files changed, 92 insertions(+), 71 deletions(-) diff --git a/Source/core/Library.cpp b/Source/core/Library.cpp index 84bdbcb75..4c710f5b8 100644 --- a/Source/core/Library.cpp +++ b/Source/core/Library.cpp @@ -28,15 +28,19 @@ namespace WPEFramework { namespace Core { + static const TCHAR* GlobalSymbols = "Global Symbols"; + Library::Library() : _refCountedHandle(nullptr) , _error() { } - Library::Library(const void* functionInLibrary) { - TCHAR filename[512]; - + Library::Library(const void* functionInLibrary) + : _refCountedHandle(nullptr) + , _error() + { #ifdef __WINDOWS__ + TCHAR filename[256]; HMODULE handle = nullptr; GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, @@ -47,29 +51,43 @@ namespace Core { // Looks like we need to add a ref count by opening it.. handle = ::LoadLibrary(filename); + if (handle != nullptr) { + // Seems we have an dynamic library opened.. + _refCountedHandle = new RefCountedHandle; + _refCountedHandle->_referenceCount = 1; + _refCountedHandle->_handle = handle; + _refCountedHandle->_name = filename; + + TRACE_L1("Took a reference on library: %s", filename); + } } -#endif -#ifdef __LINUX__ + if (_refCountedHandle == nullptr) { + _error = "Loading library by address failed!"; + TRACE_L1("Failed to load library: %p, error %s", functionInLibrary, _error.c_str()); + } +#else void* handle = nullptr; Dl_info info; if (dladdr(functionInLibrary, &info) != 0) { - _tcsncpy (filename, info.dli_fname, sizeof(filename) - 1); - handle = ::dlopen(filename, RTLD_NOLOAD); + handle = ::dlopen(info.dli_fname, RTLD_NOLOAD|RTLD_LAZY); + if (handle != nullptr) { + // Seems we have an dynamic library opened.. + _refCountedHandle = new RefCountedHandle; + _refCountedHandle->_referenceCount = 1; + _refCountedHandle->_handle = handle; + _refCountedHandle->_name = info.dli_fname; + + TRACE_L1("Took a reference on library: %s", info.dli_fname); + } } -#endif - if (handle != nullptr) { - // Seems we have an dynamic library opened.. - _refCountedHandle = new RefCountedHandle; - _refCountedHandle->_referenceCount = 1; - _refCountedHandle->_handle = handle; - _refCountedHandle->_name = filename; + if (_refCountedHandle == nullptr) { + const char* result = dlerror(); + if (result != nullptr) { + _error = result; + TRACE_L1("Failed to load library: %p, error %s", functionInLibrary, _error.c_str()); + } } - else { -#ifdef __LINUX__ - _error = dlerror(); - TRACE_L1("Failed to load library: %s, error %s", filename, _error.c_str()); #endif - } } Library::Library(const TCHAR fileName[]) : _refCountedHandle(nullptr) @@ -87,17 +105,21 @@ namespace Core { _refCountedHandle = new RefCountedHandle; _refCountedHandle->_referenceCount = 1; _refCountedHandle->_handle = handle; + if(fileName != nullptr) { _refCountedHandle->_name = fileName; TRACE_L1("Loaded library: %s", fileName); } else { + _refCountedHandle->_name = GlobalSymbols; TRACE_L1("Loaded library with global symbols of the program"); } + + TRACE_L1("Loaded library: %s", fileName); } else { #ifdef __LINUX__ _error = dlerror(); - TRACE_L1("Failed to load library: %s, error %s", (fileName != nullptr ? fileName : "Global Symbols"), _error.c_str()); + TRACE_L1("Failed to load library: %s, error %s", (fileName != nullptr ? fileName : GlobalSymbols), _error.c_str()); #endif } } @@ -117,6 +139,19 @@ namespace Core { Release(); } + Library& Library::operator=(Library&& RHS) + { + // Only do this if we have different libraries.. + Release(); + + // Assigne the new handler + _refCountedHandle = RHS._refCountedHandle; + _error = RHS._error; + RHS._refCountedHandle = nullptr; + + return (*this); + } + Library& Library::operator=(const Library& RHS) { // Only do this if we have different libraries.. @@ -176,21 +211,13 @@ namespace Core { if (_refCountedHandle != nullptr) { ASSERT(_refCountedHandle->_referenceCount > 0); if (Core::InterlockedDecrement(_refCountedHandle->_referenceCount) == 0) { - - ModuleUnload function = reinterpret_cast(LoadFunction(_T("ModuleUnload"))); - - if (function != nullptr) { - // Cleanup class - function(); - } - #ifdef __LINUX__ dlclose(_refCountedHandle->_handle); #endif #ifdef __WINDOWS__ ::FreeLibrary(_refCountedHandle->_handle); #endif - TRACE_L1("Unloaded library: %s", _refCountedHandle->_name.c_str()); + TRACE_L1("Dropping reference on library: %s", _refCountedHandle->_name.c_str()); delete _refCountedHandle; } _refCountedHandle = nullptr; diff --git a/Source/core/Library.h b/Source/core/Library.h index 9ef11280e..8f26c13f6 100644 --- a/Source/core/Library.h +++ b/Source/core/Library.h @@ -49,6 +49,7 @@ namespace Core { Library(const Library& copy); ~Library(); + Library& operator=(Library&& RHS); Library& operator=(const Library& RHS); public: diff --git a/Source/core/Services.h b/Source/core/Services.h index a563bb4b2..434e74d8f 100644 --- a/Source/core/Services.h +++ b/Source/core/Services.h @@ -174,8 +174,8 @@ namespace Core { template ServiceType(Args&&... args) : ACTUALSERVICE(std::forward(args)...) + , _referenceLib() { - ServiceAdministrator::Instance().AddRef(); } public: @@ -189,13 +189,34 @@ namespace Core { { Core::ProxyType< ServiceType > object = Core::ProxyType< ServiceType >::Create(std::forward(args)...); + ASSERT (object.IsValid() == true); + + TRACE_L1("Loading the reference library for: [%s] using [%s]", typeid(ACTUALSERVICE).name(), System::MODULE_NAME); + object->_referenceLib = Library(&System::MODULE_NAME); + return (Extract(object, TemplateIntToType::value>())); } ~ServiceType() override { - ServiceAdministrator::Instance().Release(); + // We can not use the default destructor here a that + // requires a destructor on the union member, but we + // do not want a union destructor, so lets create our + // own!!!! } + void LockLibrary(const Library& library) { + ASSERT(_referenceLib.IsLoaded() == false); + _referenceLib = library; + } + + protected: + // Destructed is a method called through SFINAE just before the memory + // associated with the object is freed from a Core::ProxyObject. If this + // method is called, be aware that the destructor of the object has run + // to completion!!! + void Destructed() { + ServiceAdministrator::Instance().ReleaseLibrary(std::move(_referenceLib)); + } private: template inline static INTERFACE* Extract(const Core::ProxyType< ServiceType >& object, const TemplateIntToType&) @@ -210,6 +231,14 @@ namespace Core { object->AddRef(); return (object.operator->()); } + + // The union here is used to avoid the destruction of the _referenceLib during + // the destructor call. That is required to make sure that the whole object, + // the actual service, is first fully destructed before we offer it to the + // service destructor (done in the Destructed call). This avoids the unloading + // of the referenced library before the object (part of this library) is fully + // destructed... + union { Library _referenceLib; }; }; template @@ -223,45 +252,8 @@ namespace Core { template class PublishedServiceType : public IService { private: - template - class Implementation : public ServiceType { - public: - Implementation() = delete; - Implementation(Implementation&&) = delete; - Implementation(const Implementation&) = delete; - Implementation& operator=(Implementation&&) = delete; - Implementation& operator=(const Implementation&) = delete; - - explicit Implementation(const Library& library) - : ServiceType() - , _referenceLib(library) { - } - ~Implementation() override { - // We can not use the default destructor here a that - // requires a destructor on the union member, but we - // do not want a union destructor, so lets create our - // own!!!! - } - - protected: - // Destructed is a method called through SFINAE just before the memory - // associated with the object is freed from a Core::ProxyObject. If this - // method is called, be aware that the destructor of the object has run - // to completion!!! - void Destructed() { - ServiceAdministrator::Instance().ReleaseLibrary(std::move(_referenceLib)); - } - - private: - // The union here is used to avoid the destruction of the _referenceLib during - // the destructor call. That is required to make sure that the whole object, - // the actual service, is first fully destructed before we offer it to the - // service destructor (done in the Destructed call). This avoids the unloading - // of the referenced library before the object (part of this library) is fully - // destructed... - union { Library _referenceLib; }; - }; - + using Implementation = ServiceType; + template class Info : public IService::IMetadata { public: @@ -321,9 +313,10 @@ namespace Core { public: void* Create(const Library& library, const uint32_t interfaceNumber) override { void* result = nullptr; - Core::ProxyType< Implementation > object = Core::ProxyType< Implementation >::Create(library); + Core::ProxyType< Implementation > object = Core::ProxyType< Implementation >::Create(); if (object.IsValid() == true) { + object->LockLibrary(library); // This query interface will increment the refcount of the Service at least to 1. result = object->QueryInterface(interfaceNumber); }