Fix possible UB in unit tests

[Bug/Feature]   Possible use of destroyed object in case when timeout
                was reached.
[Cause]         N/A
[Solution]      N/A
[Verification]  Build, install, run tests

Change-Id: Ie4cf064c4c5e5171079e836f1b4ca24b2962121c

diff --git a/tests/unit_tests/client/ut-client.cpp b/tests/unit_tests/client/ut-client.cpp
index dc6045a..f3c636f 100644 (file)
--- a/tests/unit_tests/client/ut-client.cpp
+++ b/tests/unit_tests/client/ut-client.cpp
@@ -47,8 +47,8 @@ const std::string TEST_DBUS_CONFIG_PATH =
     SC_TEST_CONFIG_INSTALL_DIR "/client/ut-client/test-dbus-daemon.conf";
 
 struct Loop {
-    Loop() { sc_start_glib_loop(); };
-    ~Loop() { sc_stop_glib_loop(); };
+    Loop() { sc_start_glib_loop(); }
+    ~Loop() { sc_stop_glib_loop(); }
 };
 
 struct Fixture {
@@ -58,7 +58,7 @@ struct Fixture {
     Fixture(): cm(TEST_DBUS_CONFIG_PATH)
     {
         cm.startAll();
-    };
+    }
 };
 
 const int EVENT_TIMEOUT = 5000; ///< ms
@@ -99,15 +99,6 @@ int getArrayStringLength(ScArrayString astring, int max_len = -1)
     return i;
 }
 
-Latch signalReceivedLatch;
-std::vector< std::tuple<std::string, std::string, std::string> > receivedSignalMsg;
-
-void NotificationTestCallback(const char* container, const char* application, const char* message)
-{
-    receivedSignalMsg.push_back(std::make_tuple(container, application, message));
-    signalReceivedLatch.set();
-}
-
 } // namespace
 
 BOOST_FIXTURE_TEST_SUITE(Client, Fixture)
@@ -198,6 +189,16 @@ BOOST_AUTO_TEST_CASE(SetActiveContainerTest)
 
 BOOST_AUTO_TEST_CASE(NotificationTest)
 {
+    // TODO add a void* to callback parameter and pass this two variables
+    // so that they no longer need to be global
+    static Latch signalReceivedLatch;
+    static std::vector< std::tuple<std::string, std::string, std::string> > receivedSignalMsg;
+
+    auto callback = [](const char* container, const char* application, const char* message) {
+        receivedSignalMsg.push_back(std::make_tuple(container, application, message));
+        signalReceivedLatch.set();
+    };
+
     std::map<std::string, ScClient> clients;
     for (const auto& it : EXPECTED_DBUSES_STARTED) {
         ScClient client = sc_client_create();
@@ -206,7 +207,7 @@ BOOST_AUTO_TEST_CASE(NotificationTest)
         clients[it.first] = client;
     }
     for (auto& client : clients) {
-        ScStatus status = sc_notification(client.second, NotificationTestCallback);
+        ScStatus status = sc_notification(client.second, callback);
         BOOST_REQUIRE_EQUAL(SCCLIENT_SUCCESS, status);
     }
     for (auto& client : clients) {

diff --git a/tests/unit_tests/dbus/ut-connection.cpp b/tests/unit_tests/dbus/ut-connection.cpp
index 18d4f7b..1352f85 100644 (file)
--- a/tests/unit_tests/dbus/ut-connection.cpp
+++ b/tests/unit_tests/dbus/ut-connection.cpp
@@ -375,6 +375,7 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallTest)
     ScopedDbusDaemon daemon;
     ScopedGlibLoop loop;
     Latch nameAcquired;
+    Latch callDone;
 
     DbusConnection::Pointer conn1 = DbusConnection::create(DBUS_ADDRESS);
     DbusConnection::Pointer conn2 = DbusConnection::create(DBUS_ADDRESS);
@@ -406,8 +407,6 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallTest)
     };
     conn1->registerObject(TESTAPI_OBJECT_PATH, TESTAPI_DEFINITION, handler);
 
-    Latch callDone;
-
     auto asyncResult1 = [&](dbus::AsyncMethodCallResult& asyncMethodCallResult) {
         BOOST_CHECK(g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT));
         callDone.set();
@@ -455,6 +454,8 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
     ScopedDbusDaemon daemon;
     ScopedGlibLoop loop;
     Latch nameAcquired;
+    Latch handlerDone;
+    Latch callDone;
 
     DbusConnection::Pointer conn1 = DbusConnection::create(DBUS_ADDRESS);
     DbusConnection::Pointer conn2 = DbusConnection::create(DBUS_ADDRESS);
@@ -464,7 +465,6 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
                    [] {});
     BOOST_REQUIRE(nameAcquired.wait(EVENT_TIMEOUT));
 
-    Latch handlerDone;
     std::string strResult;
     MethodResultBuilder::Pointer deferredResult;
 
@@ -486,8 +486,6 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
     };
     conn1->registerObject(TESTAPI_OBJECT_PATH, TESTAPI_DEFINITION, handler);
 
-    Latch callDone;
-
     auto asyncResult = [&](dbus::AsyncMethodCallResult& asyncMethodCallResult) {
         const gchar* ret = NULL;
         g_variant_get(asyncMethodCallResult.get(), "(&s)", &ret);

diff --git a/tests/unit_tests/server/ut-containers-manager.cpp b/tests/unit_tests/server/ut-containers-manager.cpp
index e8dc5de..e578a35 100644 (file)
--- a/tests/unit_tests/server/ut-containers-manager.cpp
+++ b/tests/unit_tests/server/ut-containers-manager.cpp
@@ -415,13 +415,14 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerTest)
     ContainersManager cm(TEST_DBUS_CONFIG_PATH);
     cm.startAll();
 
+    Latch signalReceivedLatch;
+    std::map<int, std::vector<std::string>> signalReceivedSourcesMap;
+
     std::map<int, std::unique_ptr<DbusAccessory>> dbuses;
     for (int i = 1; i <= TEST_DBUS_CONNECTION_CONTAINERS_COUNT; ++i) {
         dbuses[i] = std::unique_ptr<DbusAccessory>(new DbusAccessory(i));
     }
 
-    Latch signalReceivedLatch;
-    std::map<int, std::vector<std::string>> signalReceivedSourcesMap;
     auto handler = [](Latch& latch,
                       std::vector<std::string>& receivedSignalSources,
                       const std::string& /*senderBusName*/,
@@ -518,15 +519,16 @@ BOOST_AUTO_TEST_CASE(MoveFileTest)
     ContainersManager cm(TEST_DBUS_CONFIG_PATH);
     cm.startAll();
 
+    Latch notificationLatch;
+    std::string notificationSource;
+    std::string notificationPath;
+    std::string notificationRetcode;
+
     std::map<int, std::unique_ptr<DbusAccessory>> dbuses;
     for (int i = 1; i <= 2; ++i) {
         dbuses[i] = std::unique_ptr<DbusAccessory>(new DbusAccessory(i));
     }
 
-    Latch notificationLatch;
-    std::string notificationSource;
-    std::string notificationPath;
-    std::string notificationRetcode;
     auto handler = [&](const std::string& /*senderBusName*/,
                        const std::string& objectPath,
                        const std::string& interface,
@@ -763,11 +765,11 @@ BOOST_AUTO_TEST_CASE(GetContainerDbusesTest)
 
 BOOST_AUTO_TEST_CASE(ContainerDbusesSignalsTest)
 {
-    DbusAccessory host(DbusAccessory::HOST_ID);
-
     Latch signalLatch;
     DbusAccessory::Dbuses collectedDbuses;
 
+    DbusAccessory host(DbusAccessory::HOST_ID);
+
     auto onSignal = [&] (const std::string& /*senderBusName*/,
                          const std::string& objectPath,
                          const std::string& interface,

diff --git a/tests/unit_tests/server/ut-input-monitor.cpp b/tests/unit_tests/server/ut-input-monitor.cpp
index 13f1d51..99b0bd2 100644 (file)
--- a/tests/unit_tests/server/ut-input-monitor.cpp
+++ b/tests/unit_tests/server/ut-input-monitor.cpp
@@ -113,6 +113,8 @@ BOOST_AUTO_TEST_CASE(Config_deviceFilePathNotExisting)
                         InputMonitorException);
 }
 
+namespace {
+
 void sendNEvents(Fixture& f, unsigned int noOfEventsToSend)
 {
     Latch eventLatch;
@@ -146,6 +148,8 @@ void sendNEvents(Fixture& f, unsigned int noOfEventsToSend)
     BOOST_CHECK(!eventLatch.wait(10));
 }
 
+} // namespace
+
 BOOST_AUTO_TEST_CASE(Event_oneAtATime)
 {
     sendNEvents(*this, 1);
@@ -156,6 +160,8 @@ BOOST_AUTO_TEST_CASE(Event_tenAtATime)
     sendNEvents(*this, 10);
 }
 
+namespace {
+
 void sendNEventsWithPauses(Fixture& f, unsigned int noOfEventsToSend)
 {
     Latch eventLatch;
@@ -194,6 +200,8 @@ void sendNEventsWithPauses(Fixture& f, unsigned int noOfEventsToSend)
     BOOST_CHECK(!eventLatch.wait(10));
 }
 
+} // namespace
+
 BOOST_AUTO_TEST_CASE(Event_oneAtATimeWithPauses)
 {
     sendNEventsWithPauses(*this, 1);

diff --git a/tests/unit_tests/utils/ut-callback-guard.cpp b/tests/unit_tests/utils/ut-callback-guard.cpp
index 6a503b0..9e1e6b2 100644 (file)
--- a/tests/unit_tests/utils/ut-callback-guard.cpp
+++ b/tests/unit_tests/utils/ut-callback-guard.cpp
@@ -68,9 +68,9 @@ BOOST_AUTO_TEST_CASE(SimpleTest)
 
 BOOST_AUTO_TEST_CASE(ThreadTest)
 {
-    CallbackGuard guard;
     Latch trackerCreated;
     Latch trackerCanBeDestroyed;
+    CallbackGuard guard;
 
     std::future<bool> future = std::async(std::launch::async, [&]() -> bool {
             CallbackGuard::Tracker tracker = guard.spawn();

diff --git a/tests/unit_tests/utils/ut-glib-loop.cpp b/tests/unit_tests/utils/ut-glib-loop.cpp
index 63fc3ee..af07eef 100644 (file)
--- a/tests/unit_tests/utils/ut-glib-loop.cpp
+++ b/tests/unit_tests/utils/ut-glib-loop.cpp
@@ -29,7 +29,7 @@
 #include "utils/latch.hpp"
 #include "utils/glib-loop.hpp"
 
-#include <memory>
+#include <atomic>
 
 BOOST_AUTO_TEST_SUITE(UtilsGlibLoopSuite)
 
@@ -48,11 +48,12 @@ const unsigned int TIMER_WAIT_FOR    = 2 * TIMER_NUMBER * TIMER_INTERVAL_MS;
 BOOST_AUTO_TEST_CASE(GlibTimerEventTest)
 {
     ScopedGlibLoop loop;
-    CallbackGuard guard;
     Latch latch;
+    std::atomic_uint counter(0);
+
+    CallbackGuard guard;
 
     Glib::OnTimerEventCallback callback = [&]()->bool {
-        static unsigned int counter = 0;
         latch.set();
         if (++counter >= TIMER_NUMBER) {
             return false;