From ed83dcd5be227a6d9df3fc3d03f0fcfcee4d7819 Mon Sep 17 00:00:00 2001 From: Adeel Kazmi Date: Tue, 5 Nov 2024 12:07:11 +0000 Subject: [PATCH] (Addon Manager) Add dlclose & show error logs instead of asserting Change-Id: I151a341cb65a27b716eb4e97cc9d3fe92330dd4e --- .../dali-adaptor-internal/utc-Dali-AddOns.cpp | 15 +++ .../addons/linux/addon-manager-impl-linux.cpp | 103 +++++++++++------- 2 files changed, 77 insertions(+), 41 deletions(-) diff --git a/automated-tests/src/dali-adaptor-internal/utc-Dali-AddOns.cpp b/automated-tests/src/dali-adaptor-internal/utc-Dali-AddOns.cpp index a08c635a7..5585f2618 100644 --- a/automated-tests/src/dali-adaptor-internal/utc-Dali-AddOns.cpp +++ b/automated-tests/src/dali-adaptor-internal/utc-Dali-AddOns.cpp @@ -39,6 +39,7 @@ struct TestAddOn : public Dali::AddOn::AddOnBinder } ADDON_BIND_FUNCTION(GetLifecycleStatus, bool()); + ADDON_BIND_FUNCTION(FakeFunction, void()); // Function doesn't exist - for a negative test }; int UtcDaliTestAddOnInterface(void) @@ -70,6 +71,20 @@ int UtcDaliTestAddOnInterface(void) END_TEST; } +int UtcDaliTestAddOnInterfaceN(void) +{ + TestApplication application; + // Create AddOnManager using internal factory + auto addOnManager = CreateAddOnManager(); + + TestAddOn addon; + + DALI_TEST_EQUALS(addon.IsValid(), true, TEST_LOCATION); + DALI_TEST_CHECK(addon.FakeFunction == nullptr); // Function should not exist + + END_TEST; +} + int UtcDaliTestAddOnManager(void) { TestApplication application; diff --git a/dali/internal/addons/linux/addon-manager-impl-linux.cpp b/dali/internal/addons/linux/addon-manager-impl-linux.cpp index 13bff75b0..4f142f924 100644 --- a/dali/internal/addons/linux/addon-manager-impl-linux.cpp +++ b/dali/internal/addons/linux/addon-manager-impl-linux.cpp @@ -29,13 +29,21 @@ #include #include -namespace Dali -{ -namespace Internal +namespace Dali::Internal { AddOnManagerLinux::AddOnManagerLinux() = default; -AddOnManagerLinux::~AddOnManagerLinux() = default; +AddOnManagerLinux::~AddOnManagerLinux() +{ + // Close all the libraries + for(auto& addon : mAddOnCache) + { + if(addon.libHandle) + { + dlclose(addon.libHandle); + } + } +} void AddOnManagerLinux::RegisterAddOnDispatchTable(const AddOnDispatchTable* dispatchTable) { @@ -97,20 +105,30 @@ std::vector AddOnManagerLinux::EnumerateAddOns() fullPath += "/"; fullPath += name; + // Get cache count before we load the library to ensure the library does indeed load an addon + const auto cacheCountBeforeLibraryLoad = mAddOnCache.size(); + // open lib, look for essential symbols. The libary is opened with RTLD_DEEPBIND flag // to make sure the local symbol table is going to be used during lookup first. auto* handle = dlopen(fullPath.c_str(), RTLD_DEEPBIND | RTLD_LAZY); if(handle) { - DALI_ASSERT_ALWAYS(!mAddOnCache.empty() && "AddOnCache should not be empty!"); - - auto& cacheEntry = mAddOnCache.back(); - AddOnInfo info{}; - cacheEntry.GetAddOnInfo(info); - cacheEntry.info = info; - cacheEntry.addOnLib = fullPath; - cacheEntry.libHandle = handle; - cacheEntry.opened = false; + // Addon Cache size should have increased by 1 + const auto cacheCountAfterLibraryLoad = mAddOnCache.size(); + if((cacheCountBeforeLibraryLoad + 1) == cacheCountAfterLibraryLoad) + { + auto& cacheEntry = mAddOnCache.back(); + AddOnInfo info{}; + cacheEntry.GetAddOnInfo(info); + cacheEntry.info = info; + cacheEntry.addOnLib = fullPath; + cacheEntry.libHandle = handle; + cacheEntry.opened = false; + } + else + { + DALI_LOG_ERROR("Can't find any addon in %s library\n", fullPath.c_str()); + } } else { @@ -208,25 +226,32 @@ AddOnLibrary AddOnManagerLinux::LoadAddOn(const std::string& addonName, const st } else { + // Get cache count before we load the library to ensure the library does indeed load an addon + const auto cacheCountBeforeLibraryLoad = mAddOnCache.size(); + // Attempt to load the library if not found in the cache auto* handle = dlopen(libraryName.c_str(), RTLD_DEEPBIND | RTLD_LAZY); if(handle) { - // Can only have one addon per library so just check if the last added item to the cache is the addon we want - DALI_ASSERT_ALWAYS(!mAddOnCache.empty() && "AddOnCache should not be empty!"); - - auto& cacheEntry = mAddOnCache.back(); - AddOnInfo info{}; - cacheEntry.GetAddOnInfo(info); - if(info.name == addonName) + // Addon Cache size should have increased by 1 + const auto cacheCountAfterLibraryLoad = mAddOnCache.size(); + if((cacheCountBeforeLibraryLoad + 1) == cacheCountAfterLibraryLoad) { - cacheEntry.info = info; - cacheEntry.addOnLib = libraryName; - cacheEntry.libHandle = handle; - cacheEntry.opened = true; - addOnLibrary = reinterpret_cast(mAddOnCache.size()); + // Can only have one addon per library so just check if the last added item to the cache is the addon we want + auto& cacheEntry = mAddOnCache.back(); + AddOnInfo info{}; + cacheEntry.GetAddOnInfo(info); + if(info.name == addonName) + { + cacheEntry.info = info; + cacheEntry.addOnLib = libraryName; + cacheEntry.libHandle = handle; + cacheEntry.opened = true; + addOnLibrary = reinterpret_cast(cacheCountAfterLibraryLoad); + } } - else + + if(!addOnLibrary) { DALI_LOG_ERROR("Can't find %s addon in %s library\n", addonName.c_str(), libraryName.c_str()); } @@ -241,16 +266,14 @@ AddOnLibrary AddOnManagerLinux::LoadAddOn(const std::string& addonName, const st void* AddOnManagerLinux::GetGlobalProc(const Dali::AddOnLibrary& addonHandle, const char* procName) { - if(!addonHandle) + auto index = (intptr_t(addonHandle)); + if(index < 1 && index > static_cast(mAddOnCache.size())) { + DALI_LOG_ERROR("Invalid AddOn handle!\n"); return nullptr; } - auto index = (intptr_t(addonHandle)); - DALI_ASSERT_ALWAYS(index >= 1 && index <= static_cast(mAddOnCache.size()) && "Invalid AddOn handle!"); - const auto& entry = mAddOnCache[index - 1]; - if(entry.opened && entry.libHandle) { // First call into dispatch table @@ -262,23 +285,20 @@ void* AddOnManagerLinux::GetGlobalProc(const Dali::AddOnLibrary& addonHandle, co } return retval; } - else - { - DALI_LOG_ERROR("AddOn: GetGlobalProc() library failed!\n"); - } + + DALI_LOG_ERROR("AddOn: GetGlobalProc() library failed!\n"); return nullptr; } void* AddOnManagerLinux::GetInstanceProc(const Dali::AddOnLibrary& addonHandle, const char* procName) { - if(!addonHandle) + auto index = (intptr_t(addonHandle)); + if(index < 1 && index > static_cast(mAddOnCache.size())) { + DALI_LOG_ERROR("Invalid AddOn handle!\n"); return nullptr; } - auto index = (intptr_t(addonHandle)); - DALI_ASSERT_ALWAYS(index >= 1 && index <= static_cast(mAddOnCache.size()) && "Invalid AddOn handle!"); - const auto& entry = mAddOnCache[index - 1]; if(entry.opened && entry.libHandle) { @@ -291,6 +311,8 @@ void* AddOnManagerLinux::GetInstanceProc(const Dali::AddOnLibrary& addonHandle, } return retval; } + + DALI_LOG_ERROR("AddOn: GetInstanceProc() library failed!\n"); return nullptr; } @@ -334,5 +356,4 @@ void AddOnManagerLinux::InvokeLifecycleFunction(uint32_t lifecycleEvent) } } -} // namespace Internal -} // namespace Dali \ No newline at end of file +} // namespace Dali::Internal \ No newline at end of file -- 2.34.1