Some synchronizations

[Bug/Feature]   Segfault may happen
[Cause]         Access to expired memory (stack), unset memory or data races
[Solution]      Synchronizations
[Verification]  Run tests in slow machine

Change-Id: Iebf1a3d7dc93341d4200f6380c0110ecd2d96075

diff --git a/server/zone.cpp b/server/zone.cpp
index d38a0c2..f35a360 100644 (file)
--- a/server/zone.cpp
+++ b/server/zone.cpp
@@ -127,26 +127,6 @@ void Zone::start()
     goBackground();
 }
 
-void Zone::startAsync(const StartAsyncResultCallback& callback)
-{
-    auto startWrapper = [this, callback]() {
-        bool succeeded = false;
-
-        try {
-            start();
-            succeeded = true;
-        } catch(std::exception& e) {
-            LOGE(getId() << ": failed to start: " << e.what());
-        }
-
-        if (callback) {
-            callback(succeeded);
-        }
-    };
-
-    mWorker->addTask(startWrapper);
-}
-
 void Zone::stop()
 {
     Lock lock(mReconnectMutex);

diff --git a/server/zone.hpp b/server/zone.hpp
index dd0daca..7eb34cf 100644 (file)
--- a/server/zone.hpp
+++ b/server/zone.hpp
@@ -98,14 +98,6 @@ public:
     void start();
 
     /**
-     * Boot the zone to the background in separate thread. This function immediately exits
-     * after zone booting is started in another thread.
-     *
-     * @param callback Called after starting the zone. Passes bool with result of starting.
-     */
-    void startAsync(const StartAsyncResultCallback& callback);
-
-    /**
      * Try to shutdown the zone, if failed, destroy it.
      */
     void stop();

diff --git a/server/zones-manager.cpp b/server/zones-manager.cpp
index af1da8d..f65f9e3 100644 (file)
--- a/server/zones-manager.cpp
+++ b/server/zones-manager.cpp
@@ -965,17 +965,22 @@ void ZonesManager::handleStartZoneCall(const std::string& id,
 
     LOGT("Start zone " << id);
 
-    auto resultCallback = [this, id, result](bool succeeded) {
-        if (succeeded) {
+    auto startAsync = [this, id, result]() {
+        try {
+            ZoneMap::mapped_type zone;
+            {
+                Lock lock(mMutex);
+                zone = mZones.at(id);
+            }
+            zone->start();
             focus(id);
             result->setVoid();
-        } else {
-            LOGE("Failed to start zone.");
+        } catch (const std::exception& e) {
+            LOGE(id << ": failed to start: " << e.what());
             result->setError(api::ERROR_INTERNAL, "Failed to start zone");
         }
     };
-
-    mZones[id]->startAsync(resultCallback);
+    mWorker->addTask(startAsync);
 }
 
 void ZonesManager::handleLockZoneCall(const std::string& id,

diff --git a/server/zones-manager.hpp b/server/zones-manager.hpp
index 5bdb6b5..e074fc9 100644 (file)
--- a/server/zones-manager.hpp
+++ b/server/zones-manager.hpp
@@ -117,7 +117,7 @@ private:
     // to hold InputMonitor pointer to monitor if zone switching sequence is recognized
     std::unique_ptr<InputMonitor> mSwitchingSequenceMonitor;
     std::unique_ptr<ProxyCallPolicy> mProxyCallPolicy;
-    typedef std::unordered_map<std::string, std::unique_ptr<Zone>> ZoneMap;
+    typedef std::unordered_map<std::string, std::shared_ptr<Zone>> ZoneMap;
     ZoneMap mZones; // map of zones, id is the key
     bool mDetachOnExit;
 

diff --git a/tests/unit_tests/client/ut-client.cpp b/tests/unit_tests/client/ut-client.cpp
index 9536ccb..49e18d2 100644 (file)
--- a/tests/unit_tests/client/ut-client.cpp
+++ b/tests/unit_tests/client/ut-client.cpp
@@ -158,6 +158,7 @@ BOOST_AUTO_TEST_CASE(GetZoneDbusesTest)
     BOOST_REQUIRE_EQUAL(VSMCLIENT_SUCCESS, status);
     VsmArrayString keys, values;
     status = vsm_get_zone_dbuses(client, &keys, &values);
+    //TODO: Clean up if BOOST_REQUIRE_EQUAL fail (remove client). Same in other client tests.
     BOOST_REQUIRE_EQUAL(VSMCLIENT_SUCCESS, status);
 
     BOOST_CHECK_EQUAL(getArrayStringLength(keys, EXPECTED_DBUSES_STARTED.size() + 1u),

diff --git a/tests/unit_tests/dbus/ut-connection.cpp b/tests/unit_tests/dbus/ut-connection.cpp
index d4eb1de..96175ee 100644 (file)
--- a/tests/unit_tests/dbus/ut-connection.cpp
+++ b/tests/unit_tests/dbus/ut-connection.cpp
@@ -42,6 +42,7 @@
 #include <mutex>
 #include <condition_variable>
 
+//TODO: BOOST_* macros aren't thread-safe. Remove it from callbacks
 
 BOOST_AUTO_TEST_SUITE(DbusSuite)
 
@@ -461,6 +462,8 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
     Latch nameAcquired;
     Latch handlerDone;
     Latch callDone;
+    std::string strResult;
+    MethodResultBuilder::Pointer deferredResult;
 
     DbusConnection::Pointer conn1 = DbusConnection::create(DBUS_ADDRESS);
     DbusConnection::Pointer conn2 = DbusConnection::create(DBUS_ADDRESS);
@@ -470,9 +473,6 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
                    [] {});
     BOOST_REQUIRE(nameAcquired.wait(EVENT_TIMEOUT));
 
-    std::string strResult;
-    MethodResultBuilder::Pointer deferredResult;
-
     auto handler = [&](const std::string& objectPath,
                       const std::string& interface,
                       const std::string& methodName,

diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp
index 65ee470..c14b2a0 100644 (file)
--- a/tests/unit_tests/ipc/ut-ipc.cpp
+++ b/tests/unit_tests/ipc/ut-ipc.cpp
@@ -234,6 +234,7 @@ void testEcho(Client& c, const MethodID methodID)
 {
     std::shared_ptr<SendData> sentData(new SendData(34));
     std::shared_ptr<SendData> recvData = c.callSync<SendData, SendData>(methodID, sentData, TIMEOUT);
+    BOOST_REQUIRE(recvData);
     BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal);
 }
 
@@ -241,6 +242,7 @@ void testEcho(Service& s, const MethodID methodID, const FileDescriptor peerFD)
 {
     std::shared_ptr<SendData> sentData(new SendData(56));
     std::shared_ptr<SendData> recvData = s.callSync<SendData, SendData>(methodID, peerFD, sentData, TIMEOUT);
+    BOOST_REQUIRE(recvData);
     BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal);
 }
 
@@ -376,6 +378,7 @@ BOOST_AUTO_TEST_CASE(SyncServiceToClientEcho)
 
     std::shared_ptr<SendData> sentData(new SendData(56));
     std::shared_ptr<SendData> recvData = s.callSync<SendData, SendData>(1, peerFD, sentData);
+    BOOST_REQUIRE(recvData);
     BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal);
 }
 
@@ -530,6 +533,7 @@ BOOST_AUTO_TEST_CASE(WriteTimeout)
     // Test echo with a minimal timeout
     std::shared_ptr<LongSendData> sentDataA(new LongSendData(34, SHORT_OPERATION_TIME));
     std::shared_ptr<SendData> recvData = c.callSync<LongSendData, SendData>(1, sentDataA, TIMEOUT);
+    BOOST_REQUIRE(recvData);
     BOOST_CHECK_EQUAL(recvData->intVal, sentDataA->intVal);
 
     // Test timeout on write