From 48e65e93bbc02002fdfe8ca1f4467306a740f7ec Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Wed, 21 Jan 2015 12:53:38 +0100 Subject: [PATCH] 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 --- server/zone.cpp | 20 -------------------- server/zone.hpp | 8 -------- server/zones-manager.cpp | 17 +++++++++++------ server/zones-manager.hpp | 2 +- tests/unit_tests/client/ut-client.cpp | 1 + tests/unit_tests/dbus/ut-connection.cpp | 6 +++--- tests/unit_tests/ipc/ut-ipc.cpp | 4 ++++ 7 files changed, 20 insertions(+), 38 deletions(-) diff --git a/server/zone.cpp b/server/zone.cpp index d38a0c2..f35a360 100644 --- 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 --- 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 --- 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 --- 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 mSwitchingSequenceMonitor; std::unique_ptr mProxyCallPolicy; - typedef std::unordered_map> ZoneMap; + typedef std::unordered_map> 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 --- 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 --- a/tests/unit_tests/dbus/ut-connection.cpp +++ b/tests/unit_tests/dbus/ut-connection.cpp @@ -42,6 +42,7 @@ #include #include +//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 --- 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 sentData(new SendData(34)); std::shared_ptr recvData = c.callSync(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 sentData(new SendData(56)); std::shared_ptr recvData = s.callSync(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 sentData(new SendData(56)); std::shared_ptr recvData = s.callSync(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 sentDataA(new LongSendData(34, SHORT_OPERATION_TIME)); std::shared_ptr recvData = c.callSync(1, sentDataA, TIMEOUT); + BOOST_REQUIRE(recvData); BOOST_CHECK_EQUAL(recvData->intVal, sentDataA->intVal); // Test timeout on write -- 2.7.4