wsd: test: make dlhandle static and properly cleanup

We now properly cleanup unit-tests .so libs.

This helps having multiple tests by making
the dlhandle an independent variable of
all the tests.

Change-Id: Ifc8db400aebfe8ea41dcc4df2d11856c25b36074
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>

common/Unit.cpp

@@ -33,6 +33,7 @@ UnitTool *GlobalTool = nullptr;
 UnitBase** UnitBase::GlobalArray = nullptr;
 int UnitBase::GlobalIndex = -1;
 char * UnitBase::UnitLibPath;
+void* UnitBase::DlHandle = nullptr;
 static std::thread TimeoutThread;
 static std::atomic<bool> TimeoutThreadRunning(false);
 std::timed_mutex TimeoutThreadMutex;
@@ -43,8 +44,8 @@ bool EnableExperimental = false;
 UnitBase** UnitBase::linkAndCreateUnit(UnitType type, const std::string& unitLibPath)
 {
 #if !MOBILEAPP
-    void *dlHandle = dlopen(unitLibPath.c_str(), RTLD_GLOBAL|RTLD_NOW);
-    if (!dlHandle)
+    DlHandle = dlopen(unitLibPath.c_str(), RTLD_GLOBAL|RTLD_NOW);
+    if (!DlHandle)
     {
         LOG_ERR("Failed to load " << unitLibPath << ": " << dlerror());
         return nullptr;
@@ -67,7 +68,7 @@ UnitBase** UnitBase::linkAndCreateUnit(UnitType type, const std::string& unitLib
             break;
     }
     CreateUnitHooksFunction* createHooks;
-    createHooks = reinterpret_cast<CreateUnitHooksFunction *>(dlsym(dlHandle, symbol));
+    createHooks = reinterpret_cast<CreateUnitHooksFunction *>(dlsym(DlHandle, symbol));
     if (!createHooks)
     {
         LOG_ERR("No " << symbol << " symbol in " << unitLibPath);
@@ -76,7 +77,7 @@ UnitBase** UnitBase::linkAndCreateUnit(UnitType type, const std::string& unitLib
     UnitBase* hooks = createHooks();
     if (hooks)
     {
-        hooks->setHandle(dlHandle);
+        hooks->setHandle();
         return new UnitBase* [1] { hooks };
     }
 #endif
@@ -204,10 +205,38 @@ void UnitBase::rememberInstance(UnitType type, UnitBase* instance)
     }
 }
 
+void UnitBase::uninit()
+{
+    if (GlobalArray)
+    {
+        for (; GlobalIndex >= 0; --GlobalIndex)
+        {
+            delete GlobalArray[GlobalIndex];
+        }
+
+        delete[] GlobalArray;
+        GlobalArray = nullptr;
+    }
+
+    GlobalIndex = -1;
+
+    free(UnitBase::UnitLibPath);
+    UnitBase::UnitLibPath = nullptr;
+
+    GlobalKit = nullptr;
+    GlobalWSD = nullptr;
+    GlobalTool = nullptr;
+
+    // Close the DLL last, after deleting the test instances.
+    if (DlHandle)
+        dlclose(DlHandle);
+    DlHandle = nullptr;
+
+}
+
 bool UnitBase::isUnitTesting()
 {
-    return GlobalArray && GlobalIndex >= 0 && GlobalArray[GlobalIndex] &&
-           GlobalArray[GlobalIndex]->_dlHandle;
+    return DlHandle && GlobalArray && GlobalArray[GlobalIndex];
 }
 
 void UnitBase::setTimeout(std::chrono::milliseconds timeoutMilliSeconds)
@@ -221,10 +250,6 @@ UnitBase::~UnitBase()
 {
     LOG_TST(getTestname() << ": ~UnitBase: " << (_retValue ? "FAILED" : "SUCCESS"));
 
-// FIXME: we should really clean-up properly.
-//    if (_dlHandle)
-//        dlclose(_dlHandle);
-    _dlHandle = nullptr;
     _socketPoll->joinThread();
 }
 
@@ -415,7 +440,6 @@ void UnitKit::returnValue(int &retValue)
 {
     UnitBase::returnValue(retValue);
 
-    delete GlobalKit;
     GlobalKit = nullptr;
 }
 
@@ -423,7 +447,6 @@ void UnitWSD::returnValue(int &retValue)
 {
     UnitBase::returnValue(retValue);
 
-    delete GlobalWSD;
     GlobalWSD = nullptr;
 }
 

common/Unit.hpp

@@ -116,8 +116,7 @@ protected:
 
     /// Construct a UnitBase instance with a default name.
     explicit UnitBase(const std::string& name, UnitType type)
-        : _dlHandle(nullptr)
-        , _setRetValue(false)
+        : _setRetValue(false)
         , _retValue(0)
         , _timeoutMilliSeconds(std::chrono::seconds(30))
         , _type(type)
@@ -132,6 +131,8 @@ public:
     /// Load unit test hook shared library from this path
     static bool init(UnitType type, const std::string& unitLibPath);
 
+    static void uninit();
+
     /// Do we have a unit test library hooking things & loaded
     static bool isUnitTesting();
 
@@ -245,11 +246,9 @@ public:
     std::shared_ptr<SocketPoll> socketPoll() { return _socketPoll; }
 
 private:
-    void setHandle(void *dlHandle)
+    void setHandle()
     {
-        assert(_dlHandle == nullptr && "setHandle must only be called once");
-        assert(dlHandle != nullptr && "Invalid handle to set");
-        _dlHandle = dlHandle;
+        assert(DlHandle != nullptr && "Invalid handle to set");
         _socketPoll->startThread();
     }
 
@@ -272,7 +271,7 @@ private:
     /// setup global instance for get() method
     static void rememberInstance(UnitType type, UnitBase* instance);
 
-    void *_dlHandle;
+    static void* DlHandle; //< The handle to the unit-test .so.
     static char *UnitLibPath;
     static UnitBase** GlobalArray; //< All the tests.
     static int GlobalIndex; //< The index of the current test.

kit/ForKit.cpp

@@ -773,6 +773,8 @@ int main(int argc, char** argv)
     int returnValue = EX_OK;
     UnitKit::get().returnValue(returnValue);
 
+    UnitBase::uninit();
+
     LOG_INF("ForKit process finished.");
     Util::forcedExit(returnValue);
 }

wsd/COOLWSD.cpp

@@ -5596,6 +5596,7 @@ int COOLWSD::innerMain()
     int returnValue = EX_OK;
     UnitWSD::get().returnValue(returnValue);
 
+    UnitBase::uninit();
     LOG_INF("Process [coolwsd] finished with exit status: " << returnValue);
 
     // At least on centos7, Poco deadlocks while
@@ -5678,6 +5679,8 @@ int COOLWSD::main(const std::vector<std::string>& /*args*/)
 
     UnitWSD::get().returnValue(returnValue);
 
+    UnitBase::uninit();
+
     LOG_INF("Process [coolwsd] finished with exit status: " << returnValue);
 
 #if CODE_COVERAGE