From c26341627e1591ddb55a77db0f75df18a2ba4599 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Tue, 23 Dec 2014 16:30:14 +0100 Subject: [PATCH 01/16] Fix creating zone when USE_EXEC is defined [Bug/Feature] Broken zone creation using lxc-create [Cause] N/A [Solution] N/A [Verification] Compile with -DUSE_EXEC, sun server and create zone Change-Id: I164ea2ea17e93e4e22e7b74d2f8539484cedea2f --- common/lxc/zone.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/common/lxc/zone.cpp b/common/lxc/zone.cpp index b142de4..e4371f8 100644 --- a/common/lxc/zone.cpp +++ b/common/lxc/zone.cpp @@ -133,6 +133,10 @@ bool LxcZone::create(const std::string& templatePath, const char* const* argv) .add("-t").add(templatePath.c_str()) .add("-P").add(mLxcContainer->config_path); + if (*argv) { + args.add("--"); + } + while (*argv) { args.add(*argv++); } @@ -177,8 +181,11 @@ bool LxcZone::start(const char* const* argv) args.add("lxc-start") .add("-d") .add("-n").add(mLxcContainer->name) - .add("-P").add(mLxcContainer->config_path) - .add("--"); + .add("-P").add(mLxcContainer->config_path); + + if (*argv) { + args.add("--"); + } while (*argv) { args.add(*argv++); -- 2.7.4 From 5be6a32fead5470d8cae59ecb061105819d4b558 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Mon, 22 Dec 2014 12:01:20 +0100 Subject: [PATCH 02/16] Debug utility class: SameThreadGuard [Bug/Feature] A utility for checking the code against invalid assumes about synchronization needs. [Cause] N/A [Solution] N/A [Verification] Run tests Change-Id: I880f9c334d8d461e2472052db5244d9f5eab1bb8 --- common/utils/same-thread-guard.cpp | 75 +++++++++++++++++++++++ common/utils/same-thread-guard.hpp | 80 +++++++++++++++++++++++++ tests/unit_tests/utils/ut-same-thread-guard.cpp | 69 +++++++++++++++++++++ 3 files changed, 224 insertions(+) create mode 100644 common/utils/same-thread-guard.cpp create mode 100644 common/utils/same-thread-guard.hpp create mode 100644 tests/unit_tests/utils/ut-same-thread-guard.cpp diff --git a/common/utils/same-thread-guard.cpp b/common/utils/same-thread-guard.cpp new file mode 100644 index 0000000..5b54c73 --- /dev/null +++ b/common/utils/same-thread-guard.cpp @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) + * @brief Same thread guard + */ + +#include "config.hpp" +#include "utils/same-thread-guard.hpp" + +#ifdef ENABLE_SAME_THREAD_GUARD + +#include "logger/logger.hpp" +#include "logger/formatter.hpp" + +namespace vasum { +namespace utils { + +namespace { + +typedef decltype(logger::LogFormatter::getCurrentThread()) ThreadId; +const ThreadId NOT_SET = 0; + +ThreadId getCurrentThreadId() { + // use the same thread id numbering mechanism as in logger + // to allow analyse accesses in log + return logger::LogFormatter::getCurrentThread(); +} + +} // namespace + +SameThreadGuard::SameThreadGuard() : mThreadId(NOT_SET) +{ + static_assert(std::is_same::value, + "thread id type mismatch"); +} + +bool SameThreadGuard::check() +{ + const ThreadId thisThreadId = getCurrentThreadId(); + + ThreadId saved = NOT_SET; + if (!mThreadId.compare_exchange_strong(saved, thisThreadId) && saved != thisThreadId) { + LOGE("Detected thread id mismatch; saved: " << saved << "; current: " << thisThreadId); + return false; + } + return true; +} + +void SameThreadGuard::reset() +{ + mThreadId.store(NOT_SET); +} + +} // namespace utils +} // namespace vasum + +#endif // ENABLE_SAME_THREAD_GUARD diff --git a/common/utils/same-thread-guard.hpp b/common/utils/same-thread-guard.hpp new file mode 100644 index 0000000..fd697fd --- /dev/null +++ b/common/utils/same-thread-guard.hpp @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) + * @brief Same thread guard + */ + +#ifndef COMMON_UTILS_SAME_THREAD_GUARD_HPP +#define COMMON_UTILS_SAME_THREAD_GUARD_HPP + +#ifndef NDEBUG +#define ENABLE_SAME_THREAD_GUARD +#endif + +#ifdef ENABLE_SAME_THREAD_GUARD +#include +#include +#endif + +namespace vasum { +namespace utils { + +/** + * Same thread guard. + * There are two purposes of this guard: + * - reports invalid assumptions about synchronization needs (only in debug builds) + * - acts as an annotation in the source code about the thread safety + * + * Usage example: + * ASSERT_SAME_THREAD(workerThreadGuard); + */ +class SameThreadGuard { +public: +#ifdef ENABLE_SAME_THREAD_GUARD +# define ASSERT_SAME_THREAD(g) assert(g.check()) + SameThreadGuard(); + + /** + * On the first call it remembers the current thread id. + * On the next call it verifies that current thread is the same as before. + */ + bool check(); + + /** + * Reset thread id + */ + void reset(); + +private: + std::atomic mThreadId; + +#else // ENABLE_SAME_THREAD_GUARD +# define ASSERT_SAME_THREAD(g) + bool check() {return true;} + void reset() {} +#endif // ENABLE_SAME_THREAD_GUARD +}; + +} // namespace utils +} // namespace vasum + + +#endif // COMMON_UTILS_SAME_THREAD_GUARD_HPP diff --git a/tests/unit_tests/utils/ut-same-thread-guard.cpp b/tests/unit_tests/utils/ut-same-thread-guard.cpp new file mode 100644 index 0000000..7dbba57 --- /dev/null +++ b/tests/unit_tests/utils/ut-same-thread-guard.cpp @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + + +/** + * @file + * @author Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) + * @brief Unit tests of same thread guard + */ + +#include "config.hpp" +#include "ut.hpp" + +#include "utils/same-thread-guard.hpp" +#include + +#ifdef ENABLE_SAME_THREAD_GUARD + +BOOST_AUTO_TEST_SUITE(SameThreadGuardSuite) + +using namespace vasum::utils; + +BOOST_AUTO_TEST_CASE(SimpleTest) +{ + SameThreadGuard guard; + BOOST_CHECK(guard.check()); + BOOST_CHECK(guard.check()); + guard.reset(); + BOOST_CHECK(guard.check()); + BOOST_CHECK(guard.check()); +} + +BOOST_AUTO_TEST_CASE(ThreadTest) +{ + SameThreadGuard guard; + + std::thread([&] { + BOOST_CHECK(guard.check()); + }).join(); + + BOOST_CHECK(!guard.check()); + BOOST_CHECK(!guard.check()); + + guard.reset(); + BOOST_CHECK(guard.check()); + + std::thread([&] { + BOOST_CHECK(!guard.check()); + }).join(); +} + +BOOST_AUTO_TEST_SUITE_END() + +#endif // ENABLE_SAME_THREAD_GUARD -- 2.7.4 From c799942eb46d800de8145478501d7a1f6c930816 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Wed, 31 Dec 2014 13:28:54 +0100 Subject: [PATCH 03/16] Small code cleanup [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Run tests Change-Id: I45d18c987c7ea90276a3cf6e1eac3760a9df1db7 --- common/lxc/cgroup.cpp | 7 ++++--- common/utils/glib-loop.cpp | 4 +--- server/zone-provision.hpp | 2 +- tests/unit_tests/dbus/ut-connection.cpp | 7 ++++++- tests/unit_tests/utils/ut-worker.cpp | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/common/lxc/cgroup.cpp b/common/lxc/cgroup.cpp index 807ddff..62f3184 100644 --- a/common/lxc/cgroup.cpp +++ b/common/lxc/cgroup.cpp @@ -24,6 +24,7 @@ #include "config.hpp" +#include "lxc/cgroup.hpp" #include "logger/logger.hpp" #include "utils/fs.hpp" @@ -105,11 +106,11 @@ bool setDeviceAccess(const std::string& zoneName, return false; } - int major = major(devStat.st_rdev); - int minor = minor(devStat.st_rdev); + unsigned int major = major(devStat.st_rdev); + unsigned int minor = minor(devStat.st_rdev); char value[100]; - snprintf(value, sizeof(value), "%c %d:%d %s", type, major, minor, perm.c_str()); + snprintf(value, sizeof(value), "%c %u:%u %s", type, major, minor, perm.c_str()); std::string name = grant ? "devices.allow" : "devices.deny"; return setCgroup(zoneName, name, value); diff --git a/common/utils/glib-loop.cpp b/common/utils/glib-loop.cpp index fb2d952..dd82204 100644 --- a/common/utils/glib-loop.cpp +++ b/common/utils/glib-loop.cpp @@ -48,9 +48,7 @@ ScopedGlibLoop::ScopedGlibLoop() #if !GLIB_CHECK_VERSION(2,36,0) g_type_init(); #endif - mLoopThread = std::thread([this] { - g_main_loop_run(mLoop.get()); - }); + mLoopThread = std::thread(g_main_loop_run, mLoop.get()); } ScopedGlibLoop::~ScopedGlibLoop() diff --git a/server/zone-provision.hpp b/server/zone-provision.hpp index d7cc1e4..c87fbe9 100644 --- a/server/zone-provision.hpp +++ b/server/zone-provision.hpp @@ -43,7 +43,7 @@ class ZoneProvision { public: /** * ZoneProvision constructor - * @param zonesPath directory where zones are defined (lxc configs, rootfs etc) + * @param zonePath directory where zones are defined (lxc configs, rootfs etc) */ ZoneProvision(const std::string& zonePath, const std::vector& validLinkPrefixes); diff --git a/tests/unit_tests/dbus/ut-connection.cpp b/tests/unit_tests/dbus/ut-connection.cpp index 3e335ed..d4eb1de 100644 --- a/tests/unit_tests/dbus/ut-connection.cpp +++ b/tests/unit_tests/dbus/ut-connection.cpp @@ -110,6 +110,12 @@ BOOST_AUTO_TEST_CASE(NoDbusTest) BOOST_CHECK_THROW(DbusConnection::create(DBUS_ADDRESS), DbusIOException); } +BOOST_AUTO_TEST_CASE(ConnectionTest) +{ + ScopedGlibLoop loop; + DbusConnection::Pointer connSystem = DbusConnection::createSystem(); +} + BOOST_AUTO_TEST_CASE(SimpleTest) { ScopedDbusDaemon daemon; @@ -273,7 +279,6 @@ BOOST_AUTO_TEST_CASE(RegisterObjectTest) BOOST_AUTO_TEST_CASE(IntrospectSystemTest) { - ScopedDbusDaemon daemon; ScopedGlibLoop loop; DbusConnection::Pointer conn = DbusConnection::createSystem(); std::string xml = conn->introspect("org.freedesktop.DBus", "/org/freedesktop/DBus"); diff --git a/tests/unit_tests/utils/ut-worker.cpp b/tests/unit_tests/utils/ut-worker.cpp index 280889b..91fa6b8 100644 --- a/tests/unit_tests/utils/ut-worker.cpp +++ b/tests/unit_tests/utils/ut-worker.cpp @@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE(NoCopyTest) struct Task { Counter& count; - Task(Counter& c) : count(c) {}; + Task(Counter& c) : count(c) {} Task(const Task& t) : count(t.count) {++count;} Task(Task&& r) : count(r.count) {} Task& operator=(const Task&) = delete; -- 2.7.4 From 1d2b75e9a25d971eefff29adc5fd4166e43cf827 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Tue, 16 Dec 2014 14:09:33 +0100 Subject: [PATCH 04/16] IPC: External polling loop with a Client [Bug/Feature] Using GMainLoop with a Client is possible. Fixed some buggs [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: Iab3350b400739bb951d84e0d6b7de15d0cccf1d3 --- common/ipc/client.cpp | 38 ++++++++-- common/ipc/client.hpp | 24 +++++++ common/ipc/internals/acceptor.cpp | 3 +- common/ipc/internals/processor.cpp | 67 +++++++++++++++-- common/ipc/internals/processor.hpp | 7 ++ common/ipc/ipc-gsource.cpp | 31 +++++--- common/ipc/ipc-gsource.hpp | 16 +++-- common/ipc/service.cpp | 6 +- common/utils/latch.hpp | 8 +-- common/utils/signal.cpp | 63 ++++++++++++++++ common/utils/signal.hpp | 37 ++++++++++ server/CMakeLists.txt | 7 +- tests/unit_tests/CMakeLists.txt | 6 ++ tests/unit_tests/ipc/ut-ipc.cpp | 144 ++++++++++++++++++++++++++++++------- zone-daemon/CMakeLists.txt | 6 ++ 15 files changed, 404 insertions(+), 59 deletions(-) create mode 100644 common/utils/signal.cpp create mode 100644 common/utils/signal.hpp diff --git a/common/ipc/client.cpp b/common/ipc/client.cpp index 835020d..3187f15 100644 --- a/common/ipc/client.cpp +++ b/common/ipc/client.cpp @@ -48,16 +48,21 @@ Client::~Client() LOGD("Destroyed client"); } -void Client::start() +void Client::connect() { - LOGD("Starting client..."); - // Initialize the connection with the server LOGD("Connecting to " + mSocketPath); auto socketPtr = std::make_shared(Socket::connectSocket(mSocketPath)); mServiceFD = mProcessor.addPeer(socketPtr); +} + +void Client::start() +{ + LOGD("Starting client..."); - // Start listening + connect(); + + // Start polling thread mProcessor.start(); LOGD("Started client"); @@ -75,6 +80,31 @@ void Client::stop() LOGD("Stopped"); } +std::vector Client::getFDs() +{ + std::vector fds; + fds.push_back(mProcessor.getEventFD()); + fds.push_back(mServiceFD); + + return fds; +} + +void Client::handle(const FileDescriptor fd, const short pollEvent) +{ + if (fd == mProcessor.getEventFD() && (pollEvent & POLLIN)) { + mProcessor.handleEvent(); + return; + + } else if (pollEvent & POLLIN) { + mProcessor.handleInput(fd); + return; + + } else if (pollEvent & POLLHUP) { + mProcessor.handleLostConnection(fd); + return; + } +} + void Client::setNewPeerCallback(const PeerCallback& newPeerCallback) { mProcessor.setNewPeerCallback(newPeerCallback); diff --git a/common/ipc/client.hpp b/common/ipc/client.hpp index 5751812..b5b00e5 100644 --- a/common/ipc/client.hpp +++ b/common/ipc/client.hpp @@ -55,6 +55,13 @@ public: Client& operator=(const Client&) = delete; /** + * Places a connection request in the internal event queue. + * + * Used with an external polling loop. + */ + void connect(); + + /** * Starts the worker thread */ void start(); @@ -70,6 +77,23 @@ public: void stop(); /** + * Used with an external polling loop + * + * @return vector of internal file descriptors + */ + std::vector getFDs(); + + /** + * Used with an external polling loop. + * Handles one event from the file descriptor. + * + * @param fd file descriptor + * @param pollEvent event on the fd. Defined in poll.h + * + */ + void handle(const FileDescriptor fd, const short pollEvent); + + /** * Set the callback called for each new connection to a peer * * @param newPeerCallback the callback diff --git a/common/ipc/internals/acceptor.cpp b/common/ipc/internals/acceptor.cpp index 1eab1c2..627e1fe 100644 --- a/common/ipc/internals/acceptor.cpp +++ b/common/ipc/internals/acceptor.cpp @@ -67,12 +67,13 @@ void Acceptor::start() void Acceptor::stop() { LOGT("Stopping Acceptor"); + if (mThread.joinable()) { - LOGT("Event::FINISH -> Acceptor"); mEventQueue.send(Event::FINISH); LOGT("Waiting for Acceptor to finish"); mThread.join(); } + LOGT("Stopped Acceptor"); } diff --git a/common/ipc/internals/processor.cpp b/common/ipc/internals/processor.cpp index 72a1788..22aa38e 100644 --- a/common/ipc/internals/processor.cpp +++ b/common/ipc/internals/processor.cpp @@ -27,9 +27,11 @@ #include "ipc/exception.hpp" #include "ipc/internals/processor.hpp" #include "ipc/internals/utils.hpp" +#include "utils/signal.hpp" #include #include +#include #include #include @@ -58,8 +60,9 @@ Processor::Processor(const PeerCallback& newPeerCallback, mMaxNumberOfPeers(maxNumberOfPeers) { LOGT("Creating Processor"); - using namespace std::placeholders; + utils::signalBlock(SIGPIPE); + using namespace std::placeholders; addMethodHandlerInternal(REGISTER_SIGNAL_METHOD_ID, std::bind(&Processor::onNewSignals, this, _1, _2)); @@ -73,6 +76,7 @@ Processor::~Processor() } catch (IPCException& e) { LOGE("Error in Processor's destructor: " << e.what()); } + LOGT("Destroyed Processor"); } @@ -93,10 +97,12 @@ void Processor::start() void Processor::stop() { LOGT("Stopping Processor"); + if (isStarted()) { mEventQueue.send(Event::FINISH); mThread.join(); } + LOGT("Stopped Processor"); } @@ -167,7 +173,10 @@ void Processor::removePeerInternal(const FileDescriptor peerFD, Status status) LOGW("Removing peer. ID: " << peerFD); { Lock lock(mSocketsMutex); - mSockets.erase(peerFD); + if (!mSockets.erase(peerFD)) { + LOGW("No such peer. Another thread called removePeerInternal"); + return; + } // Remove from signal addressees for (auto it = mSignalsPeers.begin(); it != mSignalsPeers.end();) { @@ -269,8 +278,6 @@ void Processor::run() } } - - cleanCommunication(); } bool Processor::handleLostConnections() @@ -327,6 +334,8 @@ bool Processor::handleInput(const FileDescriptor peerFD) std::shared_ptr socketPtr; try { + // Get the peer's socket + Lock lock(mSocketsMutex); socketPtr = mSockets.at(peerFD); } catch (const std::out_of_range&) { LOGE("No such peer: " << peerFD); @@ -497,7 +506,10 @@ bool Processor::handleEvent() switch (mEventQueue.receive()) { case Event::FINISH: { LOGD("Event FINISH"); + mIsRunning = false; + cleanCommunication(); + return false; } @@ -607,6 +619,15 @@ bool Processor::onCall() LOGT("Handle call (from another thread) to send a message."); CallQueue::Call call = getCall(); + if (call.parse && call.process) { + return onMethodCall(call); + } else { + return onSignalCall(call); + } +} + +bool Processor::onSignalCall(CallQueue::Call& call) +{ std::shared_ptr socketPtr; try { // Get the peer's socket @@ -614,11 +635,43 @@ bool Processor::onCall() socketPtr = mSockets.at(call.peerFD); } catch (const std::out_of_range&) { LOGE("Peer disconnected. No socket with a peerFD: " << call.peerFD); + return false; + } + + try { + // Send the call with the socket + Socket::Guard guard = socketPtr->getGuard(); + socketPtr->write(&call.methodID, sizeof(call.methodID)); + socketPtr->write(&call.messageID, sizeof(call.messageID)); + call.serialize(socketPtr->getFD(), call.data); + } catch (const std::exception& e) { + LOGE("Error during sending a signal: " << e.what()); + + removePeerInternal(call.peerFD, Status::SERIALIZATION_ERROR); + return true; + } + + return false; + +} + +bool Processor::onMethodCall(CallQueue::Call& call) +{ + std::shared_ptr socketPtr; + try { + // Get the peer's socket + Lock lock(mSocketsMutex); + socketPtr = mSockets.at(call.peerFD); + } catch (const std::out_of_range&) { + LOGE("Peer disconnected. No socket with a peerFD: " << call.peerFD); + + // Pass the error to the processing callback IGNORE_EXCEPTIONS(call.process(Status::PEER_DISCONNECTED, call.data)); + return false; } - if (call.parse && call.process) { + { // Set what to do with the return message, but only if needed Lock lock(mReturnCallbacksMutex); if (mReturnCallbacks.count(call.messageID) != 0) { @@ -636,9 +689,9 @@ bool Processor::onCall() socketPtr->write(&call.messageID, sizeof(call.messageID)); call.serialize(socketPtr->getFD(), call.data); } catch (const std::exception& e) { - LOGE("Error during sending a message: " << e.what()); + LOGE("Error during sending a method: " << e.what()); - // Inform about the error + // Inform about the error, IGNORE_EXCEPTIONS(mReturnCallbacks[call.messageID].process(Status::SERIALIZATION_ERROR, call.data)); { diff --git a/common/ipc/internals/processor.hpp b/common/ipc/internals/processor.hpp index d33d12b..728b8d2 100644 --- a/common/ipc/internals/processor.hpp +++ b/common/ipc/internals/processor.hpp @@ -76,6 +76,11 @@ const unsigned int DEFAULT_METHOD_TIMEOUT = 1000; * - callbacks for serialization/parsing * - store Sockets in a vector, maybe SocketStore? * - fix valgrind tests +* - poll loop outside. +* - waiting till the EventQueue is empty before leaving stop() +* - no new events added after stop() called +* - when using IPCGSource: addFD and removeFD can be called from addPeer removePeer callbacks, but +* there is no mechanism to ensure the IPCSource exists.. therefore SIGSEGV :) * * */ @@ -414,6 +419,8 @@ private: void run(); bool onCall(); + bool onSignalCall(CallQueue::Call& call); + bool onMethodCall(CallQueue::Call& call); bool onNewPeer(); bool onRemovePeer(); bool handleLostConnections(); diff --git a/common/ipc/ipc-gsource.cpp b/common/ipc/ipc-gsource.cpp index f5cdbb5..4c098d9 100644 --- a/common/ipc/ipc-gsource.cpp +++ b/common/ipc/ipc-gsource.cpp @@ -57,12 +57,10 @@ IPCGSource::IPCGSource(const std::vector fds, IPCGSource::~IPCGSource() { LOGD("Destroying IPCGSource"); - g_source_destroy(&mGSource); - } -IPCGSource* IPCGSource::create(const std::vector& fds, - const HandlerCallback& handlerCallback) +IPCGSource::Pointer IPCGSource::create(const std::vector& fds, + const HandlerCallback& handlerCallback) { LOGD("Creating IPCGSource"); @@ -80,9 +78,19 @@ IPCGSource* IPCGSource::create(const std::vector& fds, // Fill additional data IPCGSource* source = reinterpret_cast(gSource); - return new(source) IPCGSource(fds, handlerCallback); -} + new(source)IPCGSource(fds, handlerCallback); + + auto deleter = [](IPCGSource * ptr) { + LOGD("Deleter"); + + if (!g_source_is_destroyed(&(ptr->mGSource))) { + // This way finalize method will be run in glib loop's thread + g_source_destroy(&(ptr->mGSource)); + } + }; + return std::shared_ptr(source, deleter); +} void IPCGSource::addFD(const FileDescriptor fd) { @@ -119,7 +127,9 @@ void IPCGSource::removeFD(const FileDescriptor fd) guint IPCGSource::attach(GMainContext* context) { LOGD("Attaching to GMainContext"); - return g_source_attach(&mGSource, context); + guint ret = g_source_attach(&mGSource, context); + g_source_unref(&mGSource); + return ret; } gboolean IPCGSource::prepare(GSource* gSource, gint* timeout) @@ -148,6 +158,11 @@ gboolean IPCGSource::dispatch(GSource* gSource, GSourceFunc /*callback*/, gpointer /*userData*/) { + if (!gSource || g_source_is_destroyed(gSource)) { + // Remove the GSource from the GMainContext + return FALSE; + } + IPCGSource* source = reinterpret_cast(gSource); for (const FDInfo fdInfo : source->mFDInfos) { @@ -157,7 +172,7 @@ gboolean IPCGSource::dispatch(GSource* gSource, } } - return TRUE; // Don't remove the GSource from the GMainContext + return TRUE; } void IPCGSource::finalize(GSource* gSource) diff --git a/common/ipc/ipc-gsource.hpp b/common/ipc/ipc-gsource.hpp index 96e0a1a..bb9a096 100644 --- a/common/ipc/ipc-gsource.hpp +++ b/common/ipc/ipc-gsource.hpp @@ -30,7 +30,7 @@ #include "ipc/service.hpp" #include "ipc/types.hpp" -#include "utils/callback-wrapper.hpp" + #include @@ -43,10 +43,17 @@ namespace ipc { * * It's supposed to be constructed ONLY with the static create method * and destructed in a glib callback. + * + * TODO: + * - waiting till the managed object (Client or Service) is destroyed + * before IPCGSource stops operating. For now programmer has to ensure this. */ struct IPCGSource { public: typedef std::function HandlerCallback; + typedef std::shared_ptr Pointer; + + ~IPCGSource(); IPCGSource() = delete; IPCGSource(const IPCGSource&) = delete; @@ -83,8 +90,8 @@ public: * * @return pointer to the IPCGSource */ - static IPCGSource* create(const std::vector& fds, - const HandlerCallback& handlerCallback); + static Pointer create(const std::vector& fds, + const HandlerCallback& handlerCallback); private: @@ -116,9 +123,6 @@ private: IPCGSource(const std::vector fds, const HandlerCallback& handlerCallback); - // Called only from IPCGSource::finalize - ~IPCGSource(); - struct FDInfo { FDInfo(gpointer tag, FileDescriptor fd) : tag(tag), fd(fd) {} diff --git a/common/ipc/service.cpp b/common/ipc/service.cpp index be95cee..5e720d6 100644 --- a/common/ipc/service.cpp +++ b/common/ipc/service.cpp @@ -91,15 +91,15 @@ std::vector Service::getFDs() void Service::handle(const FileDescriptor fd, const short pollEvent) { - if (fd == mProcessor.getEventFD() && pollEvent & POLLIN) { + if (fd == mProcessor.getEventFD() && (pollEvent & POLLIN)) { mProcessor.handleEvent(); return; - } else if (fd == mAcceptor.getConnectionFD() && pollEvent & POLLIN) { + } else if (fd == mAcceptor.getConnectionFD() && (pollEvent & POLLIN)) { mAcceptor.handleConnection(); return; - } else if (fd == mAcceptor.getEventFD() && pollEvent & POLLIN) { + } else if (fd == mAcceptor.getEventFD() && (pollEvent & POLLIN)) { mAcceptor.handleEvent(); return; diff --git a/common/utils/latch.hpp b/common/utils/latch.hpp index 1fa773b..7ef1dd7 100644 --- a/common/utils/latch.hpp +++ b/common/utils/latch.hpp @@ -54,7 +54,7 @@ public: void wait(); /** - * Waits for a single occurence of event with timeout. + * Waits for a single occurrence of event with timeout. * * @param timeoutMs timeout in ms to wait for * @return false on timeout @@ -64,14 +64,14 @@ public: /** * Waits for @ref n occurrences of event. * - * @param n number of occurences to wait for + * @param n number of occurrences to wait for */ void waitForN(const unsigned int n); /** - * Waits for @ref n occurences of event with timeout. + * Waits for @ref n occurrences of event with timeout. * - * @param n number of occurences to wait for + * @param n number of occurrences to wait for * @param timeoutMs timeout in ms to wait for * @return false on timeout */ diff --git a/common/utils/signal.cpp b/common/utils/signal.cpp new file mode 100644 index 0000000..39e7fca --- /dev/null +++ b/common/utils/signal.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Jan Olszak (j.olszak@samsung.com) + * @brief Signal related functions + */ + +#include "utils/signal.hpp" +#include "utils/exception.hpp" +#include "logger/logger.hpp" + +#include +#include +#include +#include + +namespace vasum { +namespace utils { + +void signalBlock(const int signalToBlock) +{ + ::sigset_t set; + if (-1 == ::sigemptyset(&set)) { + LOGE("Error in sigemptyset: " << std::string(strerror(errno))); + UtilsException("Error in sigemptyset: " + std::string(strerror(errno))); + } + + if (-1 ==::sigaddset(&set, signalToBlock)) { + LOGE("Error in sigaddset: " << std::string(strerror(errno))); + UtilsException("Error in sigaddset: " + std::string(strerror(errno))); + } + + int ret = ::pthread_sigmask(SIG_BLOCK, &set, nullptr /*&oldSet*/); + if (ret != 0) { + LOGE("Error in pthread_sigmask: " << std::to_string(ret)); + UtilsException("Error in pthread_sigmask: " + std::to_string(ret)); + } +} + +} // namespace utils +} // namespace vasum + + + + + diff --git a/common/utils/signal.hpp b/common/utils/signal.hpp new file mode 100644 index 0000000..f26e365 --- /dev/null +++ b/common/utils/signal.hpp @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Jan Olszak + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Jan Olszak (j.olszak@samsung.com) + * @brief Signal related functions + */ + +#ifndef COMMON_UTILS_SIGNAL_HPP +#define COMMON_UTILS_SIGNAL_HPP + +namespace vasum { +namespace utils { + +void signalBlock(const int signalsToBlock); + +} // namespace utils +} // namespace vasum + + +#endif // COMMON_UTILS_SIGNAL_HPP diff --git a/server/CMakeLists.txt b/server/CMakeLists.txt index adfef3e..79ced75 100644 --- a/server/CMakeLists.txt +++ b/server/CMakeLists.txt @@ -36,8 +36,13 @@ PKG_CHECK_MODULES(SERVER_DEPS REQUIRED lxc json gio-2.0 libsystemd-journal libsy INCLUDE_DIRECTORIES(${COMMON_FOLDER}) INCLUDE_DIRECTORIES(${CLIENT_FOLDER}) INCLUDE_DIRECTORIES(SYSTEM ${SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) -TARGET_LINK_LIBRARIES(${SERVER_CODENAME} ${SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) +SET_TARGET_PROPERTIES(${SERVER_CODENAME} PROPERTIES + COMPILE_FLAGS "-pthread" + LINK_FLAGS "-pthread" +) + +TARGET_LINK_LIBRARIES(${SERVER_CODENAME} ${SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) ## Subdirectories ############################################################## ADD_SUBDIRECTORY(configs) diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index c0cc2a8..8b66b09 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -40,6 +40,12 @@ PKG_CHECK_MODULES(UT_SERVER_DEPS REQUIRED lxc json gio-2.0 libsystemd-daemon libsystemd-journal libcap-ng libLogger libSimpleDbus libConfig) INCLUDE_DIRECTORIES(${COMMON_FOLDER} ${SERVER_FOLDER} ${UNIT_TESTS_FOLDER} ${CLIENT_FOLDER}) INCLUDE_DIRECTORIES(SYSTEM ${UT_SERVER_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) + +SET_TARGET_PROPERTIES(${UT_SERVER_CODENAME} PROPERTIES + COMPILE_FLAGS "-pthread" + LINK_FLAGS "-pthread" +) + TARGET_LINK_LIBRARIES(${UT_SERVER_CODENAME} ${UT_SERVER_DEPS_LIBRARIES} ${Boost_LIBRARIES}) diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp index 9ce131d..a7f8645 100644 --- a/tests/unit_tests/ipc/ut-ipc.cpp +++ b/tests/unit_tests/ipc/ut-ipc.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include using namespace vasum; @@ -136,7 +137,7 @@ std::shared_ptr longEchoCallback(const FileDescriptor, std::shared_ptr return data; } -FileDescriptor connect(Service& s, Client& c, bool serviceUsesGlib = false) +FileDescriptor connect(Service& s, Client& c) { // Connects the Client to the Service and returns Clients FileDescriptor std::mutex mutex; @@ -149,41 +150,102 @@ FileDescriptor connect(Service& s, Client& c, bool serviceUsesGlib = false) cv.notify_all(); }; + // TODO: On timeout remove the callback + s.setNewPeerCallback(newPeerCallback); + + if (!s.isStarted()) { + s.start(); + } + + c.start(); + + + std::unique_lock lock(mutex); + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + return peerFD != 0; + })); + + return peerFD; +} + - if (!serviceUsesGlib) { - s.setNewPeerCallback(newPeerCallback); - if (!s.isStarted()) { - s.start(); - } - } else { #if GLIB_CHECK_VERSION(2,36,0) - IPCGSource* serviceGSourcePtr = IPCGSource::create(s.getFDs(), std::bind(&Service::handle, &s, _1, _2)); +std::pair connectServiceGSource(Service& s, Client& c) +{ + std::mutex mutex; + std::condition_variable cv; - auto agregateCallback = [&newPeerCallback, &serviceGSourcePtr](const FileDescriptor newFD) { - serviceGSourcePtr->addFD(newFD); - newPeerCallback(newFD); - }; + FileDescriptor peerFD = 0; + IPCGSource::Pointer ipcGSourcePtr = IPCGSource::create(s.getFDs(), std::bind(&Service::handle, &s, _1, _2)); - s.setNewPeerCallback(agregateCallback); - s.setRemovedPeerCallback(std::bind(&IPCGSource::removeFD, serviceGSourcePtr, _1)); + auto newPeerCallback = [&cv, &peerFD, &mutex, ipcGSourcePtr](const FileDescriptor newFD) { + if (ipcGSourcePtr) { + //TODO: Remove this if + ipcGSourcePtr->addFD(newFD); + } + std::unique_lock lock(mutex); + peerFD = newFD; + cv.notify_all(); + }; - serviceGSourcePtr->attach(); -#endif // GLIB_CHECK_VERSION - } + // TODO: On timeout remove the callback + s.setNewPeerCallback(newPeerCallback); + s.setRemovedPeerCallback(std::bind(&IPCGSource::removeFD, ipcGSourcePtr, _1)); + + // Service starts to process + ipcGSourcePtr->attach(); c.start(); std::unique_lock lock(mutex); - BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { return peerFD != 0; })); - return peerFD; + return std::make_pair(peerFD, ipcGSourcePtr); } +std::pair connectClientGSource(Service& s, Client& c) +{ + // Connects the Client to the Service and returns Clients FileDescriptor + std::mutex mutex; + std::condition_variable cv; + + FileDescriptor peerFD = 0; + auto newPeerCallback = [&cv, &peerFD, &mutex](const FileDescriptor newFD) { + std::unique_lock lock(mutex); + peerFD = newFD; + cv.notify_all(); + }; + // TODO: On timeout remove the callback + s.setNewPeerCallback(newPeerCallback); + + if (!s.isStarted()) { + // Service starts to process + s.start(); + } + + + c.connect(); + IPCGSource::Pointer ipcGSourcePtr = IPCGSource::create(c.getFDs(), + std::bind(&Client::handle, &c, _1, _2)); + + ipcGSourcePtr->attach(); + + std::unique_lock lock(mutex); + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + return peerFD != 0; + })); + + return std::make_pair(peerFD, ipcGSourcePtr); +} + +#endif // GLIB_CHECK_VERSION + + void testEcho(Client& c, const MethodID methodID) { std::shared_ptr sentData(new SendData(34)); @@ -194,7 +256,7 @@ void testEcho(Client& c, const MethodID methodID) 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); + std::shared_ptr recvData = s.callSync(methodID, peerFD, sentData, 1000); BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal); } @@ -481,17 +543,16 @@ BOOST_AUTO_TEST_CASE(ReadTimeout) { Service s(socketPath); auto longEchoCallback = [](const FileDescriptor, std::shared_ptr& data) { - return std::shared_ptr(new LongSendData(data->intVal)); + return std::shared_ptr(new LongSendData(data->intVal, 4000 /*ms*/)); }; s.addMethodHandler(1, longEchoCallback); - s.start(); Client c(socketPath); - c.start(); + connect(s, c); // Test timeout on read std::shared_ptr sentData(new SendData(334)); - BOOST_CHECK_THROW((c.callSync(1, sentData, 100)), IPCException); + BOOST_CHECK_THROW((c.callSync(1, sentData, 10)), IPCException); } @@ -587,12 +648,15 @@ BOOST_AUTO_TEST_CASE(ServiceGSource) isSignalCalled = true; }; + IPCGSource::Pointer serviceGSource; Service s(socketPath); s.addMethodHandler(1, echoCallback); Client c(socketPath); s.addSignalHandler(2, signalHandler); - connect(s, c, true); + + auto ret = connectServiceGSource(s, c); + serviceGSource = ret.second; testEcho(c, 1); @@ -603,6 +667,36 @@ BOOST_AUTO_TEST_CASE(ServiceGSource) BOOST_CHECK(isSignalCalled); } +BOOST_AUTO_TEST_CASE(ClientGSource) +{ + ScopedGlibLoop loop; + + std::atomic_bool isSignalCalled(false); + auto signalHandler = [&isSignalCalled](const FileDescriptor, std::shared_ptr&) { + isSignalCalled = true; + }; + + Service s(socketPath); + s.start(); + + IPCGSource::Pointer clientGSource; + Client c(socketPath); + c.addMethodHandler(1, echoCallback); + c.addSignalHandler(2, signalHandler); + + auto ret = connectClientGSource(s, c); + FileDescriptor peerFD = ret.first; + clientGSource = ret.second; + + testEcho(s, 1, peerFD); + + auto data = std::make_shared(1); + s.signal(2, data); + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); //TODO wait_for + BOOST_CHECK(isSignalCalled); +} + #endif // GLIB_CHECK_VERSION // BOOST_AUTO_TEST_CASE(ConnectionLimitTest) diff --git a/zone-daemon/CMakeLists.txt b/zone-daemon/CMakeLists.txt index 50bfa1a..7baf37f 100644 --- a/zone-daemon/CMakeLists.txt +++ b/zone-daemon/CMakeLists.txt @@ -37,6 +37,12 @@ PKG_CHECK_MODULES(ZONE_DAEMON_DEPS REQUIRED gio-2.0 libsystemd-journal libcap-ng libLogger libSimpleDbus libConfig) INCLUDE_DIRECTORIES(${COMMON_FOLDER}) INCLUDE_DIRECTORIES(SYSTEM ${ZONE_DAEMON_DEPS_INCLUDE_DIRS} ${Boost_INCLUDE_DIRS}) + +SET_TARGET_PROPERTIES(${ZONE_DAEMON_CODENAME} PROPERTIES + COMPILE_FLAGS "-pthread" + LINK_FLAGS "-pthread" +) + TARGET_LINK_LIBRARIES(${ZONE_DAEMON_CODENAME} ${ZONE_DAEMON_DEPS_LIBRARIES} ${Boost_LIBRARIES}) -- 2.7.4 From 3bf3bb34a99cb34c102467f070c1b0a6064f5515 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Wed, 7 Jan 2015 16:02:54 +0100 Subject: [PATCH 05/16] IPC: Fixed DisconnectedPeerError test [Bug/Feature] N/A [Cause] SERIALIZATION_ERROR may also be returned when peer disconnects [Solution] N/A [Verification] Build, install, run tests Change-Id: Ifaf67828796cfa0def8d5f56dc926314abb2d36b --- tests/unit_tests/ipc/ut-ipc.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp index a7f8645..18c4785 100644 --- a/tests/unit_tests/ipc/ut-ipc.cpp +++ b/tests/unit_tests/ipc/ut-ipc.cpp @@ -532,10 +532,14 @@ BOOST_AUTO_TEST_CASE(DisconnectedPeerError) // Wait for the response std::unique_lock lock(mutex); - BOOST_CHECK(cv.wait_for(lock, std::chrono::seconds(10), [&retStatus]() { + BOOST_CHECK(cv.wait_for(lock, std::chrono::seconds(100), [&retStatus]() { return retStatus != ipc::Status::UNDEFINED; })); - BOOST_CHECK(retStatus == ipc::Status::PEER_DISCONNECTED); //TODO it fails from time to time + + // The disconnection might have happened: + // - after sending the message (PEER_DISCONNECTED) + // - during external serialization (SERIALIZATION_ERROR) + BOOST_CHECK(retStatus == ipc::Status::PEER_DISCONNECTED || retStatus == ipc::Status::SERIALIZATION_ERROR); } -- 2.7.4 From 1da8243f43d8d0f7f6adc052b6f47fb6a34e1091 Mon Sep 17 00:00:00 2001 From: Dariusz Michaluk Date: Tue, 30 Dec 2014 14:46:59 +0100 Subject: [PATCH 06/16] Update tizen common (with wayland) lxc template [Bug/Feature] Adjust template to new platform image [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: Ic32677fa85a73249af5f1d84b63893b26d1eadd4 Signed-off-by: Dariusz Michaluk --- .../configs/lxc-templates/tizen-common-wayland.sh | 139 ++++----------------- 1 file changed, 24 insertions(+), 115 deletions(-) diff --git a/server/configs/lxc-templates/tizen-common-wayland.sh b/server/configs/lxc-templates/tizen-common-wayland.sh index 12fb223..9424dc4 100755 --- a/server/configs/lxc-templates/tizen-common-wayland.sh +++ b/server/configs/lxc-templates/tizen-common-wayland.sh @@ -86,6 +86,7 @@ ${rootfs}/home \ ${rootfs}/home/alice \ ${rootfs}/home/bob \ ${rootfs}/home/carol \ +${rootfs}/home/developer \ ${rootfs}/home/guest \ ${rootfs}/lib \ ${rootfs}/media \ @@ -104,12 +105,14 @@ ${path}/hooks \ ${path}/scripts \ ${path}/systemd \ ${path}/systemd/system \ +${path}/systemd/system/multi-user.target.wants \ ${path}/systemd/user " /bin/mkdir ${ROOTFS_DIRS} /bin/chown alice:users ${rootfs}/home/alice /bin/chown bob:users ${rootfs}/home/bob /bin/chown carol:users ${rootfs}/home/carol +/bin/chown developer:users ${rootfs}/home/developer /bin/chown guest:users ${rootfs}/home/guest /bin/ln -s /dev/null ${path}/systemd/system/bluetooth.service @@ -117,8 +120,11 @@ ${path}/systemd/user /bin/ln -s /dev/null ${path}/systemd/system/sshd.socket /bin/ln -s /dev/null ${path}/systemd/system/sshd@.service /bin/ln -s /dev/null ${path}/systemd/system/systemd-udevd.service -/bin/ln -s /dev/null ${path}/systemd/system/user-session-launch@seat0-5100.service +/bin/ln -s /dev/null ${path}/systemd/system/systemd-udevd-kernel.socket +/bin/ln -s /dev/null ${path}/systemd/system/systemd-udevd-control.socket /bin/ln -s /dev/null ${path}/systemd/system/vconf-setup.service +/bin/ln -s /usr/lib/systemd/system/tlm.service ${path}/systemd/system/multi-user.target.wants/tlm.service +/bin/ln -s /dev/null ${path}/systemd/user/media-server-user.service cat <>${path}/systemd/system/display-manager-run.service # Run weston with framebuffer backend on selected virtual terminal. @@ -133,7 +139,7 @@ WorkingDirectory=/run/%u ExecStart=/usr/bin/weston --backend=fbdev-backend.so -i0 --log=/tmp/weston.log --tty=${vt} #StandardInput=tty #TTYPath=/dev/tty7 -EnvironmentFile=/etc/systemd/system/weston +EnvironmentFile=/etc/sysconfig/weston Restart=on-failure RestartSec=10 @@ -144,119 +150,18 @@ CapabilityBoundingSet=CAP_SYS_TTY_CONFIG [Install] WantedBy=graphical.target EOF - -cat <>${path}/systemd/system/display-manager.path -# Wayland socket path is changed to /tmp directory. -[Unit] -Description=Wait for wayland socket -Requires=display-manager-run.service -After=display-manager-run.service - -[Path] -PathExists=/tmp/wayland-0 -EOF - -cat <>${path}/systemd/system/display-manager.service -# Wayland socket path is changed to /tmp directory. -[Unit] -Description=Display manager setup service -Requires=display-manager-run.service -After=display-manager-run.service - -[Service] -Type=oneshot -ExecStart=/usr/bin/chmod g+w /tmp/wayland-0 -#ExecStart=/usr/bin/chsmack -a User /tmp/wayland-0 - -[Install] -WantedBy=graphical.target -EOF - -cat <>${path}/systemd/system/weston -# path to display manager runtime dir -XDG_RUNTIME_DIR=/tmp -XDG_CONFIG_HOME=/etc/systemd/system -EOF - -cat <>${path}/systemd/system/weston.ini -# Weston config for zone. -[core] -modules=desktop-shell.so - -[shell] -background-image=/usr/share/backgrounds/tizen/golfe-morbihan.jpg -background-color=0xff002244 -background-type=scale-crop -panel-color=0x95333333 -locking=true -panel=false -animation=zoom -#binding-modifier=ctrl -num-workspaces=4 -#cursor-theme=whiteglass -#cursor-size=24 -startup-animation=fade - -#lockscreen-icon=/usr/share/icons/gnome/256x256/actions/lock.png -#lockscreen=/usr/share/backgrounds/gnome/Garden.jpg -#homescreen=/usr/share/backgrounds/gnome/Blinds.jpg - -## weston - -[launcher] -icon=/usr/share/icons/tizen/32x32/terminal.png -path=/usr/bin/weston-terminal - -[screensaver] -# Uncomment path to disable screensaver -duration=600 - -[input-method] -path=/usr/libexec/weston-keyboard -#path=/bin/weekeyboard - -#[keyboard] -#keymap_layout=fr - -#[output] -#name=LVDS1 -#mode=1680x1050 -#transform=90 -#icc_profile=/usr/share/color/icc/colord/Bluish.icc - -#[output] -#name=VGA1 -#mode=173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync -#transform=flipped - -#[output] -#name=X1 -#mode=1024x768 -#transform=flipped-270 - -#[touchpad] -#constant_accel_factor = 50 -#min_accel_factor = 0.16 -#max_accel_factor = 1.0 - -[output] -name=DP1 -default_output=1 -EOF - -cat <>${path}/systemd/user/weston-user.service -# Wayland socket path is changed to /tmp directory. -[Unit] -Description=Shared weston session - -[Service] -ExecStartPre=/usr/bin/ln -sf /tmp/wayland-0 /run/user/%U/ -ExecStart=/bin/sh -l -c "/usr/bin/tz-launcher -c /usr/share/applications/tizen/launcher.conf %h/.applications/desktop" -EnvironmentFile=/etc/sysconfig/weston-user - -[Install] -WantedBy=default.target -EOF +chmod 644 ${path}/systemd/system/display-manager-run.service + +sed -e 's/run\/display/tmp/g' /usr/lib/systemd/system/display-manager.path >> ${path}/systemd/system/display-manager.path +chmod 644 ${path}/systemd/system/display-manager.path +sed -e 's/run\/display/tmp/g' /usr/lib/systemd/system/display-manager.service >> ${path}/systemd/system/display-manager.service +chmod 644 ${path}/systemd/system/display-manager.service +sed -e 's/run\/display/tmp/g' /etc/sysconfig/weston >> ${path}/weston +sed -e 's/backgrounds\/tizen\/current/backgrounds\/tizen\/golfe-morbihan.jpg/g' /etc/xdg/weston/weston.ini >> ${path}/weston.ini +sed -e 's/run\/display/tmp/g' /usr/lib/systemd/user/weston-user.service >> ${path}/systemd/user/weston-user.service +sed -e 's/run\/display/tmp/g' /etc/tlm.conf >> ${path}/tlm.conf +sed -e 's/run\/display/tmp/g' /etc/session.d/user-session >> ${path}/user-session +chmod 755 ${path}/user-session # Prepare host configuration cat <>/etc/udev/rules.d/99-tty.rules @@ -377,8 +282,12 @@ chmod 770 ${path}/hooks/pre-start.sh cat <>${path}/fstab /bin bin none ro,bind 0 0 /etc etc none ro,bind 0 0 +${path}/tlm.conf etc/tlm.conf none ro,bind 0 0 +${path}/user-session etc/session.d/user-session none rw,bind 0 0 ${path}/systemd/system etc/systemd/system none ro,bind 0 0 ${path}/systemd/user etc/systemd/user none ro,bind 0 0 +${path}/weston etc/sysconfig/weston none ro,bind 0 0 +${path}/weston.ini etc/xdg/weston/weston.ini none ro,bind 0 0 /lib lib none ro,bind 0 0 /media media none ro,bind 0 0 /mnt mnt none ro,bind 0 0 -- 2.7.4 From 952b04e1a069e0c0e55d393ec2460a3fc24766ff Mon Sep 17 00:00:00 2001 From: Dariusz Michaluk Date: Thu, 8 Jan 2015 11:12:25 +0100 Subject: [PATCH 07/16] Add correct button configuration [Bug/Feature] Add correct button configuration [Cause] N/A [Solution] N/A [Verification] Build, install, run tests, launch vasum Change-Id: I3561823663c3dd9a058249a0467fa704105f19a1 Signed-off-by: Dariusz Michaluk --- packaging/vasum.spec | 1 - server/configs/daemon.conf.in | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packaging/vasum.spec b/packaging/vasum.spec index 887409e..e743a75 100644 --- a/packaging/vasum.spec +++ b/packaging/vasum.spec @@ -158,7 +158,6 @@ Development package including the header files for the client library %package zone-support Summary: Vasum Support Group: Security/Other -Conflicts: vasum %description zone-support Zones support installed inside every zone. diff --git a/server/configs/daemon.conf.in b/server/configs/daemon.conf.in index 648d986..4522bb5 100644 --- a/server/configs/daemon.conf.in +++ b/server/configs/daemon.conf.in @@ -8,10 +8,10 @@ "foregroundId" : "private", "defaultId" : "private", "lxcTemplatePrefix" : "/etc/vasum/lxc-templates", - "inputConfig" : {"enabled" : false, + "inputConfig" : {"enabled" : true, "device" : "gpio_keys.6", - "code" : 139, - "numberOfEvents" : 1, + "code" : 116, + "numberOfEvents" : 2, "timeWindowMs" : 500}, "proxyCallRules" : [] } -- 2.7.4 From 94b00b1c12569f6efeb68b4e086198f2671bff95 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Fri, 9 Jan 2015 10:11:28 +0100 Subject: [PATCH 08/16] IPC: Cleaned up timeouts in tests [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install, run tests, run tests under valgrind Change-Id: Ifee2dffed77a5686fdb2165b2a530f39aef0857a --- common/ipc/internals/processor.cpp | 1 + common/ipc/internals/processor.hpp | 12 ++++++- tests/unit_tests/ipc/ut-ipc.cpp | 67 ++++++++++++++++++++------------------ 3 files changed, 48 insertions(+), 32 deletions(-) diff --git a/common/ipc/internals/processor.cpp b/common/ipc/internals/processor.cpp index 22aa38e..6084718 100644 --- a/common/ipc/internals/processor.cpp +++ b/common/ipc/internals/processor.cpp @@ -588,6 +588,7 @@ bool Processor::onNewPeer() Lock lock(mCallbacksMutex); if (mNewPeerCallback) { // Notify about the new user. + LOGT("Calling NewPeerCallback"); mNewPeerCallback(socketInfo.peerFD); } } diff --git a/common/ipc/internals/processor.hpp b/common/ipc/internals/processor.hpp index 728b8d2..0e8fe8f 100644 --- a/common/ipc/internals/processor.hpp +++ b/common/ipc/internals/processor.hpp @@ -602,11 +602,21 @@ std::shared_ptr Processor::callSync(const MethodID methodID, }; std::unique_lock lock(mutex); + LOGT("Waiting for the response..."); + // TODO: There is a race here. mReturnCallbacks were used to indicate if the return call was served or not, + // but if the timeout occurs before the call is even sent, then this method is broken. if (!cv.wait_for(lock, std::chrono::milliseconds(timeoutMS), isResultInitialized)) { + // Timeout occurred: + // - call isn't sent => delete it + // - call is sent and no reply => throw IPCTimeoutError + // - call is being serviced => wait for it with the same timeout + LOGT("Probably a timeout in callSync. Checking..."); + bool isTimeout = false; { Lock lock(mReturnCallbacksMutex); if (1 == mReturnCallbacks.erase(messageID)) { + // Return callback was present, so there was a timeout isTimeout = true; } } @@ -615,7 +625,7 @@ std::shared_ptr Processor::callSync(const MethodID methodID, LOGE("Function call timeout; methodID: " << methodID); throw IPCTimeoutException("Function call timeout; methodID: " + std::to_string(methodID)); } else { - // Timeout started during the return value processing, so wait for it to finish + //Timeout started during the return value processing, so wait for it to finish cv.wait(lock, isResultInitialized); } } diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp index 18c4785..65ee27e 100644 --- a/tests/unit_tests/ipc/ut-ipc.cpp +++ b/tests/unit_tests/ipc/ut-ipc.cpp @@ -55,6 +55,16 @@ using namespace std::placeholders; namespace fs = boost::filesystem; namespace { + +// Timeout for sending one message +const int TIMEOUT = 1000 /*ms*/; + +// Time that won't cause "TIMEOUT" methods to throw +const int SHORT_OPERATION_TIME = TIMEOUT / 100; + +// Time that will cause "TIMEOUT" methods to throw +const int LONG_OPERATION_TIME = 3 * TIMEOUT; + struct Fixture { std::string socketPath; @@ -133,7 +143,7 @@ std::shared_ptr echoCallback(const FileDescriptor, std::shared_ptr longEchoCallback(const FileDescriptor, std::shared_ptr& data) { - std::this_thread::sleep_for(std::chrono::seconds(1)); + std::this_thread::sleep_for(std::chrono::milliseconds(LONG_OPERATION_TIME)); return data; } @@ -161,7 +171,7 @@ FileDescriptor connect(Service& s, Client& c) std::unique_lock lock(mutex); - BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(3 * TIMEOUT), [&peerFD]() { return peerFD != 0; })); @@ -201,7 +211,7 @@ std::pair connectServiceGSource(Service& s, c.start(); std::unique_lock lock(mutex); - BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(3 * TIMEOUT), [&peerFD]() { return peerFD != 0; })); @@ -236,7 +246,7 @@ std::pair connectClientGSource(Service& s, ipcGSourcePtr->attach(); std::unique_lock lock(mutex); - BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(2000), [&peerFD]() { + BOOST_REQUIRE(cv.wait_for(lock, std::chrono::milliseconds(3 * TIMEOUT), [&peerFD]() { return peerFD != 0; })); @@ -249,14 +259,14 @@ std::pair connectClientGSource(Service& s, void testEcho(Client& c, const MethodID methodID) { std::shared_ptr sentData(new SendData(34)); - std::shared_ptr recvData = c.callSync(methodID, sentData, 1000); + std::shared_ptr recvData = c.callSync(methodID, sentData, TIMEOUT); BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal); } 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, 1000); + std::shared_ptr recvData = s.callSync(methodID, peerFD, sentData, TIMEOUT); BOOST_CHECK_EQUAL(recvData->intVal, sentData->intVal); } @@ -274,7 +284,6 @@ BOOST_AUTO_TEST_CASE(ConstructorDestructor) BOOST_AUTO_TEST_CASE(ServiceAddRemoveMethod) { Service s(socketPath); - s.addMethodHandler(1, returnEmptyCallback); s.addMethodHandler(1, returnDataCallback); @@ -284,7 +293,7 @@ BOOST_AUTO_TEST_CASE(ServiceAddRemoveMethod) s.addMethodHandler(2, returnDataCallback); Client c(socketPath); - c.start(); + connect(s, c); testEcho(c, 1); s.removeMethod(1); @@ -352,9 +361,9 @@ BOOST_AUTO_TEST_CASE(SyncClientToServiceEcho) s.addMethodHandler(1, echoCallback); s.addMethodHandler(2, echoCallback); - s.start(); Client c(socketPath); - c.start(); + connect(s, c); + testEcho(c, 1); testEcho(c, 2); } @@ -421,7 +430,7 @@ BOOST_AUTO_TEST_CASE(AsyncClientToServiceEcho) // Wait for the response std::unique_lock lock(mutex); - BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(100), [&recvData]() { + BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(TIMEOUT), [&recvData]() { return static_cast(recvData); })); @@ -452,7 +461,7 @@ BOOST_AUTO_TEST_CASE(AsyncServiceToClientEcho) // Wait for the response std::unique_lock lock(mutex); - BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(1000), [&recvData]() { + BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(TIMEOUT), [&recvData]() { return recvData.get() != nullptr; })); @@ -465,23 +474,20 @@ BOOST_AUTO_TEST_CASE(SyncTimeout) Service s(socketPath); s.addMethodHandler(1, longEchoCallback); - s.start(); Client c(socketPath); - c.start(); + connect(s, c); std::shared_ptr sentData(new SendData(78)); - - BOOST_CHECK_THROW((c.callSync(1, sentData, 10)), IPCException); //TODO it fails from time to time + BOOST_REQUIRE_THROW((c.callSync(1, sentData, TIMEOUT)), IPCException); } BOOST_AUTO_TEST_CASE(SerializationError) { Service s(socketPath); s.addMethodHandler(1, echoCallback); - s.start(); Client c(socketPath); - c.start(); + connect(s, c); std::shared_ptr throwingData(new ThrowOnAcceptData()); @@ -532,7 +538,7 @@ BOOST_AUTO_TEST_CASE(DisconnectedPeerError) // Wait for the response std::unique_lock lock(mutex); - BOOST_CHECK(cv.wait_for(lock, std::chrono::seconds(100), [&retStatus]() { + BOOST_CHECK(cv.wait_for(lock, std::chrono::milliseconds(TIMEOUT), [&retStatus]() { return retStatus != ipc::Status::UNDEFINED; })); @@ -547,7 +553,7 @@ BOOST_AUTO_TEST_CASE(ReadTimeout) { Service s(socketPath); auto longEchoCallback = [](const FileDescriptor, std::shared_ptr& data) { - return std::shared_ptr(new LongSendData(data->intVal, 4000 /*ms*/)); + return std::shared_ptr(new LongSendData(data->intVal, LONG_OPERATION_TIME)); }; s.addMethodHandler(1, longEchoCallback); @@ -556,7 +562,7 @@ BOOST_AUTO_TEST_CASE(ReadTimeout) // Test timeout on read std::shared_ptr sentData(new SendData(334)); - BOOST_CHECK_THROW((c.callSync(1, sentData, 10)), IPCException); + BOOST_CHECK_THROW((c.callSync(1, sentData, TIMEOUT)), IPCException); } @@ -570,13 +576,13 @@ BOOST_AUTO_TEST_CASE(WriteTimeout) c.start(); // Test echo with a minimal timeout - std::shared_ptr sentDataA(new LongSendData(34, 10 /*ms*/)); - std::shared_ptr recvData = c.callSync(1, sentDataA, 100); + std::shared_ptr sentDataA(new LongSendData(34, SHORT_OPERATION_TIME)); + std::shared_ptr recvData = c.callSync(1, sentDataA, TIMEOUT); BOOST_CHECK_EQUAL(recvData->intVal, sentDataA->intVal); // Test timeout on write - std::shared_ptr sentDataB(new LongSendData(34, 1000 /*ms*/)); - BOOST_CHECK_THROW((c.callSync(1, sentDataB, 100)), IPCTimeoutException); + std::shared_ptr sentDataB(new LongSendData(34, LONG_OPERATION_TIME)); + BOOST_CHECK_THROW((c.callSync(1, sentDataB, TIMEOUT)), IPCTimeoutException); } @@ -604,7 +610,7 @@ BOOST_AUTO_TEST_CASE(AddSignalInRuntime) s.signal(1, data); // Wait for the signals to arrive - std::this_thread::sleep_for(std::chrono::milliseconds(100)); //TODO wait_for + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for BOOST_CHECK(isHandlerACalled && isHandlerBCalled); } @@ -630,13 +636,13 @@ BOOST_AUTO_TEST_CASE(AddSignalOffline) connect(s, c); // Wait for the information about the signals to propagate - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); auto data = std::make_shared(1); s.signal(2, data); s.signal(1, data); // Wait for the signals to arrive - std::this_thread::sleep_for(std::chrono::milliseconds(100)); //TODO wait_for + std::this_thread::sleep_for(std::chrono::milliseconds(2 * TIMEOUT)); BOOST_CHECK(isHandlerACalled && isHandlerBCalled); } @@ -646,7 +652,6 @@ BOOST_AUTO_TEST_CASE(AddSignalOffline) BOOST_AUTO_TEST_CASE(ServiceGSource) { ScopedGlibLoop loop; - std::atomic_bool isSignalCalled(false); auto signalHandler = [&isSignalCalled](const FileDescriptor, std::shared_ptr&) { isSignalCalled = true; @@ -667,7 +672,7 @@ BOOST_AUTO_TEST_CASE(ServiceGSource) auto data = std::make_shared(1); c.signal(2, data); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); //TODO wait_for + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for BOOST_CHECK(isSignalCalled); } @@ -697,7 +702,7 @@ BOOST_AUTO_TEST_CASE(ClientGSource) auto data = std::make_shared(1); s.signal(2, data); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); //TODO wait_for + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for BOOST_CHECK(isSignalCalled); } -- 2.7.4 From 3035b39d565506bc70dce035cd4178e883b7f976 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Fri, 9 Jan 2015 12:34:12 +0100 Subject: [PATCH 09/16] Fixed generation of log_sink file [Bug]i log_sink file is invalid formated xml [Cause] Lxc invoke exit(0) after fork. Exit from child and from parent cause redundant log write [Solution] Add defintion to CMakeLists.txt [Verification] Build, install, build run test with '--log_sink=/tmp/log.xml', run 'xmllint --huge --format /tmp/log.xml' (there should be no parse errors). Note: StartHasStoppedTest can fail. Change-Id: I9a1d4bee9030eb8e3958ff914ef7cf2dc1a953ce --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index a12367f..06a1064 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,6 +73,7 @@ ADD_DEFINITIONS("-pedantic") # Be pedantic ADD_DEFINITIONS("-pedantic-errors") # Make pedantic warnings into errors ADD_DEFINITIONS(-DPROGRAM_VERSION="${VERSION}") ADD_DEFINITIONS(-DPROJECT_SOURCE_DIR="${PROJECT_SOURCE_DIR}") +ADD_DEFINITIONS(-DUSE_EXEC) IF("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") # Warn about documentation problems -- 2.7.4 From a83342a7862458d2a6a35fe8889e80cd9a9dc1bb Mon Sep 17 00:00:00 2001 From: Lukasz Kostyra Date: Wed, 7 Jan 2015 15:39:02 +0100 Subject: [PATCH 10/16] IPC: Convert test synchronization method to Latch [Bug/Feature] Some tests were using sleep_for in order to synchronize threads [Cause] N/A [Solution] Use Latch instead of atomic_bool set and sleep_for to synchronize threads in tests [Verification] Build, install, run tests. Should pass as they did before. Change-Id: I3067ed1f13cdde047f720c0b6d05ce19ec156dbe --- tests/unit_tests/ipc/ut-ipc.cpp | 50 +++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp index 65ee27e..74c18f3 100644 --- a/tests/unit_tests/ipc/ut-ipc.cpp +++ b/tests/unit_tests/ipc/ut-ipc.cpp @@ -36,12 +36,12 @@ #include "ipc/ipc-gsource.hpp" #include "ipc/types.hpp" #include "utils/glib-loop.hpp" +#include "utils/latch.hpp" #include "config/fields.hpp" #include "logger/logger.hpp" #include -#include #include #include #include @@ -592,14 +592,14 @@ BOOST_AUTO_TEST_CASE(AddSignalInRuntime) Client c(socketPath); connect(s, c); - std::atomic_bool isHandlerACalled(false); - auto handlerA = [&isHandlerACalled](const FileDescriptor, std::shared_ptr&) { - isHandlerACalled = true; + utils::Latch latchA; + auto handlerA = [&latchA](const FileDescriptor, std::shared_ptr&) { + latchA.set(); }; - std::atomic_bool isHandlerBCalled(false); - auto handlerB = [&isHandlerBCalled](const FileDescriptor, std::shared_ptr&) { - isHandlerBCalled = true; + utils::Latch latchB; + auto handlerB = [&latchB](const FileDescriptor, std::shared_ptr&) { + latchB.set(); }; c.addSignalHandler(1, handlerA); @@ -610,8 +610,7 @@ BOOST_AUTO_TEST_CASE(AddSignalInRuntime) s.signal(1, data); // Wait for the signals to arrive - std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for - BOOST_CHECK(isHandlerACalled && isHandlerBCalled); + BOOST_CHECK(latchA.wait(TIMEOUT) && latchB.wait(TIMEOUT)); } @@ -620,14 +619,14 @@ BOOST_AUTO_TEST_CASE(AddSignalOffline) Service s(socketPath); Client c(socketPath); - std::atomic_bool isHandlerACalled(false); - auto handlerA = [&isHandlerACalled](const FileDescriptor, std::shared_ptr&) { - isHandlerACalled = true; + utils::Latch latchA; + auto handlerA = [&latchA](const FileDescriptor, std::shared_ptr&) { + latchA.set(); }; - std::atomic_bool isHandlerBCalled(false); - auto handlerB = [&isHandlerBCalled](const FileDescriptor, std::shared_ptr&) { - isHandlerBCalled = true; + utils::Latch latchB; + auto handlerB = [&latchB](const FileDescriptor, std::shared_ptr&) { + latchB.set(); }; c.addSignalHandler(1, handlerA); @@ -642,8 +641,7 @@ BOOST_AUTO_TEST_CASE(AddSignalOffline) s.signal(1, data); // Wait for the signals to arrive - std::this_thread::sleep_for(std::chrono::milliseconds(2 * TIMEOUT)); - BOOST_CHECK(isHandlerACalled && isHandlerBCalled); + BOOST_CHECK(latchA.wait(TIMEOUT) && latchB.wait(TIMEOUT)); } @@ -652,9 +650,9 @@ BOOST_AUTO_TEST_CASE(AddSignalOffline) BOOST_AUTO_TEST_CASE(ServiceGSource) { ScopedGlibLoop loop; - std::atomic_bool isSignalCalled(false); - auto signalHandler = [&isSignalCalled](const FileDescriptor, std::shared_ptr&) { - isSignalCalled = true; + utils::Latch l; + auto signalHandler = [&l](const FileDescriptor, std::shared_ptr&) { + l.set(); }; IPCGSource::Pointer serviceGSource; @@ -672,17 +670,16 @@ BOOST_AUTO_TEST_CASE(ServiceGSource) auto data = std::make_shared(1); c.signal(2, data); - std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for - BOOST_CHECK(isSignalCalled); + BOOST_CHECK(l.wait(TIMEOUT)); } BOOST_AUTO_TEST_CASE(ClientGSource) { ScopedGlibLoop loop; - std::atomic_bool isSignalCalled(false); - auto signalHandler = [&isSignalCalled](const FileDescriptor, std::shared_ptr&) { - isSignalCalled = true; + utils::Latch l; + auto signalHandler = [&l](const FileDescriptor, std::shared_ptr&) { + l.set(); }; Service s(socketPath); @@ -702,8 +699,7 @@ BOOST_AUTO_TEST_CASE(ClientGSource) auto data = std::make_shared(1); s.signal(2, data); - std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT)); //TODO wait_for - BOOST_CHECK(isSignalCalled); + BOOST_CHECK(l.wait(TIMEOUT)); } #endif // GLIB_CHECK_VERSION -- 2.7.4 From e310694aca62b3359f7ca1e9027e4f8650a52ade Mon Sep 17 00:00:00 2001 From: Lukasz Kostyra Date: Wed, 17 Dec 2014 10:42:41 +0100 Subject: [PATCH 11/16] Add test for LoggerScope [Feature] Test for new LoggerScope interface. [Cause] New interface in libLogger. [Solution] Test for new LoggerScope interface. [Verification] Build, install, run Log tests. Change-Id: I43767c1f65d09ca8304046fd9d8586f1077a8b63 --- tests/unit_tests/log/ut-logger.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit_tests/log/ut-logger.cpp b/tests/unit_tests/log/ut-logger.cpp index e574505..74028db 100644 --- a/tests/unit_tests/log/ut-logger.cpp +++ b/tests/unit_tests/log/ut-logger.cpp @@ -26,6 +26,7 @@ #include "config.hpp" #include "ut.hpp" #include "logger/logger.hpp" +#include "logger/logger-scope.hpp" #include "logger/formatter.hpp" #include "logger/backend.hpp" #include "logger/backend-stderr.hpp" @@ -197,6 +198,19 @@ BOOST_AUTO_TEST_CASE(TestLogsTrace) BOOST_CHECK(tf.logContains("[TRACE]") == true); } +BOOST_AUTO_TEST_CASE(TestLoggerScope) +{ + LOGS("Main function scope"); + + { + LOGS("Scope inside function"); + LOGD("Some additional information in-between scoped logs"); + { + LOGS("Additional scope with " << "stringstream" << ' ' << "test" << 3 << ' ' << 3.42); + LOGD("More additional information in-between scoped logs"); + } + } +} BOOST_AUTO_TEST_SUITE_END() -- 2.7.4 From da1be5620b803b7d826376faffe7ecd0083b8222 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Mon, 12 Jan 2015 16:16:47 +0100 Subject: [PATCH 12/16] Added testing wrong or unset union type [Bug/Feature] Added testing wrong or unset union type [Cause] N/A [Solution] Throw ConfigException [Verification] Build, install, run ConfigurationSuite tests Change-Id: I813b52870c4c12a58a9a614821056278a5445a68 --- tests/unit_tests/config/ut-configuration.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit_tests/config/ut-configuration.cpp b/tests/unit_tests/config/ut-configuration.cpp index 8e82905..95fc52a 100644 --- a/tests/unit_tests/config/ut-configuration.cpp +++ b/tests/unit_tests/config/ut-configuration.cpp @@ -95,6 +95,9 @@ BOOST_AUTO_TEST_CASE(ToStringTest) std::string out = saveToString(testConfig); BOOST_CHECK_EQUAL(out, jsonTestString); + + TestConfig::SubConfigOption unionConfig; + BOOST_CHECK_THROW(saveToString(unionConfig), ConfigException); } namespace loadErrorsTest { @@ -111,6 +114,13 @@ DECLARE_CONFIG(BoolConfig, bool) DECLARE_CONFIG(ArrayConfig, std::vector) DECLARE_CONFIG(ObjectConfig, IntConfig) #undef DECLARE_CONFIG +struct UnionConfig { + CONFIG_DECLARE_UNION + ( + int, + bool + ) +}; } // namespace loadErrorsTest @@ -177,6 +187,12 @@ BOOST_AUTO_TEST_CASE(LoadErrorsTest) BOOST_CHECK_THROW(loadFromString("{\"field\": []}", objectConfig), ConfigException); BOOST_CHECK_THROW(loadFromString("{\"field\": {}}", objectConfig), ConfigException); BOOST_CHECK_NO_THROW(loadFromString("{\"field\": {\"field\": 1}}", objectConfig)); + + UnionConfig unionConfig; + BOOST_CHECK_THROW(loadFromString("{\"type\": \"long\", \"value\": 1}", unionConfig), ConfigException); + BOOST_CHECK_THROW(loadFromString("{\"type\": \"int\"}", unionConfig), ConfigException); + BOOST_CHECK_NO_THROW(loadFromString("{\"type\": \"int\", \"value\": 1}", unionConfig)); + BOOST_CHECK_NO_THROW(loadFromString("{\"type\": \"bool\", \"value\": true}", unionConfig)); } namespace hasVisitableTest { -- 2.7.4 From 04f7e267d40ba248d20251dcb13223134996095f Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Wed, 7 Jan 2015 14:29:30 +0100 Subject: [PATCH 13/16] Add get_zone_ids, get_active_zone_id and get_zones_status to cli [Feature] Add get_zone_ids, get_active_zone_id and get_zones_status to cli [Cause] N/A [Solution] N/A [Verification] Build, run appropriate functions (through cli) Change-Id: If6823243d66606d28bf6b45c5b2ece8ab7c06e49 --- cli/command-line-interface.cpp | 89 ++++++++++++++++++++++++++++++++++++++++++ cli/command-line-interface.hpp | 20 ++++++++++ cli/main.cpp | 24 ++++++++++++ 3 files changed, 133 insertions(+) diff --git a/cli/command-line-interface.cpp b/cli/command-line-interface.cpp index 1222a86..06be364 100644 --- a/cli/command-line-interface.cpp +++ b/cli/command-line-interface.cpp @@ -31,7 +31,12 @@ #include #include #include +#include +#include +#include +#include #include +#include using namespace std; @@ -82,6 +87,14 @@ finish: } } +template +string stringAsInStream(const T& value) +{ + std::ostringstream stream; + stream << value; + return stream.str(); +} + ostream& operator<<(ostream& out, const VsmZoneState& state) { const char* name; @@ -113,6 +126,30 @@ ostream& operator<<(ostream& out, const VsmZone& zone) return out; } +typedef vector> Table; + +ostream& operator<<(ostream& out, const Table& table) +{ + vector sizes; + for (const auto& row : table) { + if (sizes.size() < row.size()) { + sizes.resize(row.size()); + } + for (size_t i = 0; i < row.size(); ++i) { + sizes[i] = max(sizes[i], row[i].length()); + } + } + + for (const auto& row : table) { + for (size_t i = 0; i < row.size(); ++i) { + out << left << setw(sizes[i]+2) << row[i]; + } + out << "\n"; + } + + return out; +} + } // namespace void CommandLineInterface::printUsage(std::ostream& out) const @@ -210,6 +247,58 @@ void unlock_zone(int pos, int argc, const char** argv) one_shot(bind(vsm_unlock_zone, _1, argv[pos + 1])); } +void get_zones_status(int /* pos */, int /* argc */, const char** /* argv */) +{ + using namespace std::placeholders; + + VsmArrayString ids; + VsmString activeId; + Table table; + + one_shot(bind(vsm_get_zone_ids, _1, &ids)); + one_shot(bind(vsm_get_active_zone_id, _1, &activeId)); + table.push_back({"Active", "Id", "State", "Terminal", "Root"}); + for (VsmString* id = ids; *id; ++id) { + VsmZone zone; + one_shot(bind(vsm_lookup_zone_by_id, _1, *id, &zone)); + assert(string(zone->id) == string(*id)); + table.push_back({string(zone->id) == string(activeId) ? "*" : "", + zone->id, + stringAsInStream(zone->state), + to_string(zone->terminal), + zone->rootfs_path}); + vsm_zone_free(zone); + } + vsm_string_free(activeId); + vsm_array_string_free(ids); + cout << table << endl; +} + +void get_zone_ids(int /* pos */, int /* argc */, const char** /* argv */) +{ + using namespace std::placeholders; + + VsmArrayString ids; + one_shot(bind(vsm_get_zone_ids, _1, &ids)); + string delim; + for (VsmString* id = ids; *id; ++id) { + cout << delim << *id; + delim = ", "; + } + cout << endl; + vsm_array_string_free(ids); +} + +void get_active_zone_id(int /* pos */, int /* argc */, const char** /* argv */) +{ + using namespace std::placeholders; + + VsmString id; + one_shot(bind(vsm_get_active_zone_id, _1, &id)); + cout << id << endl; + vsm_string_free(id); +} + void lookup_zone_by_id(int pos, int argc, const char** argv) { using namespace std::placeholders; diff --git a/cli/command-line-interface.hpp b/cli/command-line-interface.hpp index 6a655d8..055bb1e 100644 --- a/cli/command-line-interface.hpp +++ b/cli/command-line-interface.hpp @@ -147,6 +147,26 @@ void lock_zone(int pos, int argc, const char** argv); void unlock_zone(int pos, int argc, const char** argv); /** + * Parses command line arguments and prints list of zone with + * some useful informations (id, state, terminal, root path) + */ +void get_zones_status(int pos, int argc, const char** argv); + +/** + * Parses command line arguments and call vsm_get_zone_ids + * + * @see vsm_get_zone_ids + */ +void get_zone_ids(int pos, int argc, const char** argv); + +/** + * Parses command line arguments and call vsm_get_active_zone_id + * + * @see vsm_get_active_zone_id + */ +void get_active_zone_id(int pos, int argc, const char** argv); + +/** * Parses command line arguments and call vsm_lookup_zone_by_id * * @see vsm_lookup_zone_by_id diff --git a/cli/main.cpp b/cli/main.cpp index 1afe134..d003d10 100644 --- a/cli/main.cpp +++ b/cli/main.cpp @@ -92,6 +92,30 @@ std::map commands = { } }, { + "get_zones_status", { + get_zones_status, + "get_zones_status", + "Get list of zone with some useful informations (id, state, terminal, root path)", + {} + } + }, + { + "get_zone_ids", { + get_zone_ids, + "get_zone_ids", + "Get all zone ids", + {} + } + }, + { + "get_active_zone_id", { + get_active_zone_id, + "get_active_zone_id", + "Get active (foreground) zone ids", + {} + } + }, + { "lookup_zone_by_id", { lookup_zone_by_id, "lookup_zone_by_id zone_id", -- 2.7.4 From b8e9e623277194fb1d620de64604dc821a97410a Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Tue, 13 Jan 2015 15:36:23 +0100 Subject: [PATCH 14/16] Fix: Crash when testing ut-zones-manager [Bug/Feature] Crash when testing ut-zones-manager [Cause] BOOST_CHECK is not thread safe [Solution] BOOSR_CHECK was removed from child thread [Verification] Build, install, run test on slp Change-Id: I6cb10f626d8e5ec908e359de15b3fbbed9abb77c --- tests/unit_tests/server/ut-zones-manager.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/unit_tests/server/ut-zones-manager.cpp b/tests/unit_tests/server/ut-zones-manager.cpp index 06af8eb..a733e05 100644 --- a/tests/unit_tests/server/ut-zones-manager.cpp +++ b/tests/unit_tests/server/ut-zones-manager.cpp @@ -313,8 +313,9 @@ public: const VoidResultCallback& result) { auto asyncResult = [result](dbus::AsyncMethodCallResult& asyncMethodCallResult) { - BOOST_CHECK(g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)); - result(); + if (g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)) { + result(); + } }; assert(isHost()); @@ -332,8 +333,9 @@ public: const VoidResultCallback& result) { auto asyncResult = [result](dbus::AsyncMethodCallResult& asyncMethodCallResult) { - BOOST_CHECK(g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)); - result(); + if (g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)) { + result(); + } }; assert(isHost()); @@ -351,8 +353,9 @@ public: const VoidResultCallback& result) { auto asyncResult = [result](dbus::AsyncMethodCallResult& asyncMethodCallResult) { - BOOST_CHECK(g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)); - result(); + if (g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)) { + result(); + } }; assert(isHost()); @@ -370,8 +373,9 @@ public: const VoidResultCallback& result) { auto asyncResult = [result](dbus::AsyncMethodCallResult& asyncMethodCallResult) { - BOOST_CHECK(g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)); - result(); + if (g_variant_is_of_type(asyncMethodCallResult.get(), G_VARIANT_TYPE_UNIT)) { + result(); + } }; assert(isHost()); @@ -923,7 +927,7 @@ BOOST_AUTO_TEST_CASE(ZoneDbusesSignalsTest) cm.startAll(); - BOOST_CHECK(signalLatch.waitForN(TEST_DBUS_CONNECTION_ZONES_COUNT, EVENT_TIMEOUT)); + BOOST_REQUIRE(signalLatch.waitForN(TEST_DBUS_CONNECTION_ZONES_COUNT, EVENT_TIMEOUT)); BOOST_CHECK(signalLatch.empty()); BOOST_CHECK(EXPECTED_DBUSES_STARTED == collectedDbuses); collectedDbuses.clear(); -- 2.7.4 From 808219e7cc0fc49ae2f3edabf26b6d7a1a5ecf69 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Fri, 9 Jan 2015 13:48:05 +0100 Subject: [PATCH 15/16] IPC: Single state mutex in Processor [Bug/Feature] Fixed a bug in callSync. Replaced all mutexes in Processor with only one. Added LOGS loggs [Cause] N/A [Solution] N/A [Verification] Build, install, run tests, run tests under valgrind Change-Id: I6b6ec26df5f5d7ba8b930e4321f766146fa556f0 --- common/ipc/client.cpp | 19 +- common/ipc/client.hpp | 14 +- common/ipc/internals/call-queue.cpp | 17 +- common/ipc/internals/call-queue.hpp | 21 +- common/ipc/internals/processor.cpp | 377 +++++++++++++++++------------------- common/ipc/internals/processor.hpp | 139 ++++++------- common/ipc/ipc-gsource.cpp | 16 +- common/ipc/service.cpp | 17 +- common/ipc/service.hpp | 16 +- tests/unit_tests/ipc/ut-ipc.cpp | 7 +- 10 files changed, 304 insertions(+), 339 deletions(-) diff --git a/common/ipc/client.cpp b/common/ipc/client.cpp index 3187f15..8455b19 100644 --- a/common/ipc/client.cpp +++ b/common/ipc/client.cpp @@ -34,18 +34,17 @@ namespace ipc { Client::Client(const std::string& socketPath) : mSocketPath(socketPath) { - LOGD("Creating client"); + LOGS("Client Constructor"); } Client::~Client() { - LOGD("Destroying client..."); + LOGS("Client Destructor"); try { stop(); } catch (IPCException& e) { LOGE("Error in Client's destructor: " << e.what()); } - LOGD("Destroyed client"); } void Client::connect() @@ -58,14 +57,9 @@ void Client::connect() void Client::start() { - LOGD("Starting client..."); - + LOGS("Client start"); connect(); - - // Start polling thread mProcessor.start(); - - LOGD("Started client"); } bool Client::isStarted() @@ -75,9 +69,8 @@ bool Client::isStarted() void Client::stop() { - LOGD("Stopping client..."); + LOGS("Client Destructor"); mProcessor.stop(); - LOGD("Stopped"); } std::vector Client::getFDs() @@ -107,17 +100,19 @@ void Client::handle(const FileDescriptor fd, const short pollEvent) void Client::setNewPeerCallback(const PeerCallback& newPeerCallback) { + LOGS("Client setNewPeerCallback"); mProcessor.setNewPeerCallback(newPeerCallback); } void Client::setRemovedPeerCallback(const PeerCallback& removedPeerCallback) { + LOGS("Client setRemovedPeerCallback"); mProcessor.setRemovedPeerCallback(removedPeerCallback); } void Client::removeMethod(const MethodID methodID) { - LOGD("Removing method id: " << methodID); + LOGS("Client removeMethod methodID: " << methodID); mProcessor.removeMethod(methodID); } diff --git a/common/ipc/client.hpp b/common/ipc/client.hpp index b5b00e5..de847a9 100644 --- a/common/ipc/client.hpp +++ b/common/ipc/client.hpp @@ -189,18 +189,16 @@ template void Client::addMethodHandler(const MethodID methodID, const typename MethodHandler::type& method) { - LOGD("Adding method with id " << methodID); + LOGS("Client addMethodHandler, methodID: " << methodID); mProcessor.addMethodHandler(methodID, method); - LOGD("Added method with id " << methodID); } template void Client::addSignalHandler(const MethodID methodID, const typename SignalHandler::type& handler) { - LOGD("Adding signal with id " << methodID); + LOGS("Client addSignalHandler, methodID: " << methodID); mProcessor.addSignalHandler(methodID, handler); - LOGD("Added signal with id " << methodID); } template @@ -208,7 +206,7 @@ std::shared_ptr Client::callSync(const MethodID methodID, const std::shared_ptr& data, unsigned int timeoutMS) { - LOGD("Sync calling method: " << methodID); + LOGS("Client callSync, methodID: " << methodID << ", timeoutMS: " << timeoutMS); return mProcessor.callSync(methodID, mServiceFD, data, timeoutMS); } @@ -217,22 +215,20 @@ void Client::callAsync(const MethodID methodID, const std::shared_ptr& data, const typename ResultHandler::type& resultCallback) { - LOGD("Async calling method: " << methodID); + LOGS("Client callAsync, methodID: " << methodID); mProcessor.callAsync(methodID, mServiceFD, data, resultCallback); - LOGD("Async called method: " << methodID); } template void Client::signal(const MethodID methodID, const std::shared_ptr& data) { - LOGD("Signaling: " << methodID); + LOGS("Client signal, methodID: " << methodID); mProcessor.signal(methodID, data); - LOGD("Signaled: " << methodID); } } // namespace ipc diff --git a/common/ipc/internals/call-queue.cpp b/common/ipc/internals/call-queue.cpp index df70f11..70871e5 100644 --- a/common/ipc/internals/call-queue.cpp +++ b/common/ipc/internals/call-queue.cpp @@ -27,6 +27,7 @@ #include "ipc/internals/call-queue.hpp" #include "ipc/exception.hpp" #include "logger/logger.hpp" +#include namespace vasum { namespace ipc { @@ -51,6 +52,20 @@ MessageID CallQueue::getNextMessageID() return ++mMessageIDCounter; } +bool CallQueue::erase(const MessageID messageID) +{ + LOGT("Erase messgeID: " << messageID); + auto it = std::find(mCalls.begin(), mCalls.end(), messageID); + if (it == mCalls.end()) { + LOGT("No such messgeID"); + return false; + } + + mCalls.erase(it); + LOGT("Erased"); + return true; +} + CallQueue::Call CallQueue::pop() { if (isEmpty()) { @@ -58,7 +73,7 @@ CallQueue::Call CallQueue::pop() throw IPCException("CallQueue is empty"); } Call call = std::move(mCalls.front()); - mCalls.pop(); + mCalls.pop_front(); return call; } diff --git a/common/ipc/internals/call-queue.hpp b/common/ipc/internals/call-queue.hpp index 03adfc8..a6e45ed 100644 --- a/common/ipc/internals/call-queue.hpp +++ b/common/ipc/internals/call-queue.hpp @@ -27,9 +27,10 @@ #include "ipc/types.hpp" #include "config/manager.hpp" +#include "logger/logger-scope.hpp" #include -#include +#include namespace vasum { namespace ipc { @@ -45,9 +46,15 @@ public: struct Call { Call(const Call& other) = delete; Call& operator=(const Call&) = delete; + Call& operator=(Call&&) = default; Call() = default; Call(Call&&) = default; + bool operator==(const MessageID m) + { + return m == messageID; + } + FileDescriptor peerFD; MethodID methodID; MessageID messageID; @@ -78,10 +85,12 @@ public: Call pop(); + bool erase(const MessageID messageID); + bool isEmpty() const; private: - std::queue mCalls; + std::list mCalls; std::atomic mMessageIDCounter; MessageID getNextMessageID(); @@ -103,21 +112,24 @@ MessageID CallQueue::push(const MethodID methodID, call.messageID = messageID; call.serialize = [](const int fd, std::shared_ptr& data)->void { + LOGS("Method serialize, peerFD: " << fd); config::saveToFD(fd, *std::static_pointer_cast(data)); }; call.parse = [](const int fd)->std::shared_ptr { + LOGS("Method parse, peerFD: " << fd); std::shared_ptr data(new ReceivedDataType()); config::loadFromFD(fd, *data); return data; }; call.process = [process](Status status, std::shared_ptr& data)->void { + LOGS("Method process, status: " << toString(status)); std::shared_ptr tmpData = std::static_pointer_cast(data); return process(status, tmpData); }; - mCalls.push(std::move(call)); + mCalls.push_back(std::move(call)); return messageID; } @@ -136,10 +148,11 @@ MessageID CallQueue::push(const MethodID methodID, call.messageID = messageID; call.serialize = [](const int fd, std::shared_ptr& data)->void { + LOGS("Signal serialize, peerFD: " << fd); config::saveToFD(fd, *std::static_pointer_cast(data)); }; - mCalls.push(std::move(call)); + mCalls.push_back(std::move(call)); return messageID; } diff --git a/common/ipc/internals/processor.cpp b/common/ipc/internals/processor.cpp index 6084718..d1e829e 100644 --- a/common/ipc/internals/processor.cpp +++ b/common/ipc/internals/processor.cpp @@ -59,25 +59,22 @@ Processor::Processor(const PeerCallback& newPeerCallback, mRemovedPeerCallback(removedPeerCallback), mMaxNumberOfPeers(maxNumberOfPeers) { - LOGT("Creating Processor"); + LOGS("Processor Constructor"); utils::signalBlock(SIGPIPE); using namespace std::placeholders; addMethodHandlerInternal(REGISTER_SIGNAL_METHOD_ID, std::bind(&Processor::onNewSignals, this, _1, _2)); - } Processor::~Processor() { - LOGT("Destroying Processor"); + LOGS("Processor Destructor"); try { stop(); } catch (IPCException& e) { LOGE("Error in Processor's destructor: " << e.what()); } - - LOGT("Destroyed Processor"); } bool Processor::isStarted() @@ -87,60 +84,60 @@ bool Processor::isStarted() void Processor::start() { - LOGT("Starting Processor"); + LOGS("Processor start"); + if (!isStarted()) { mThread = std::thread(&Processor::run, this); } - LOGT("Started Processor"); } void Processor::stop() { - LOGT("Stopping Processor"); + LOGS("Processor stop"); if (isStarted()) { - mEventQueue.send(Event::FINISH); + { + Lock lock(mStateMutex); + mEventQueue.send(Event::FINISH); + } + LOGT("Waiting for the Processor to stop"); mThread.join(); } - - LOGT("Stopped Processor"); } void Processor::setNewPeerCallback(const PeerCallback& newPeerCallback) { - Lock lock(mCallbacksMutex); + Lock lock(mStateMutex); mNewPeerCallback = newPeerCallback; } void Processor::setRemovedPeerCallback(const PeerCallback& removedPeerCallback) { - Lock lock(mCallbacksMutex); + Lock lock(mStateMutex); mRemovedPeerCallback = removedPeerCallback; } FileDescriptor Processor::getEventFD() { + Lock lock(mStateMutex); return mEventQueue.getFD(); } void Processor::removeMethod(const MethodID methodID) { - LOGT("Removing method " << methodID); - Lock lock(mCallsMutex); + Lock lock(mStateMutex); mMethodsCallbacks.erase(methodID); } FileDescriptor Processor::addPeer(const std::shared_ptr& socketPtr) { - LOGT("Adding socket"); - FileDescriptor peerFD; - { - Lock lock(mSocketsMutex); - peerFD = socketPtr->getFD(); - SocketInfo socketInfo(peerFD, std::move(socketPtr)); - mNewSockets.push(std::move(socketInfo)); - mEventQueue.send(Event::ADD_PEER); - } + LOGS("Processor addPeer"); + Lock lock(mStateMutex); + FileDescriptor peerFD = socketPtr->getFD(); + SocketInfo socketInfo(peerFD, std::move(socketPtr)); + mNewSockets.push(std::move(socketInfo)); + mEventQueue.send(Event::ADD_PEER); + LOGI("New peer added. Id: " << peerFD); return peerFD; @@ -148,18 +145,22 @@ FileDescriptor Processor::addPeer(const std::shared_ptr& socketPtr) void Processor::removePeer(const FileDescriptor peerFD) { - std::shared_ptr conditionPtr(new std::condition_variable()); + LOGS("Processor removePeer peerFD: " << peerFD); + + // TODO: Remove ADD_PEER event if it's not processed + + // Remove peer and wait till he's gone + std::shared_ptr conditionPtr(new std::condition_variable()); { - Lock lock(mSocketsMutex); + Lock lock(mStateMutex); RemovePeerRequest request(peerFD, conditionPtr); mPeersToDelete.push(std::move(request)); mEventQueue.send(Event::REMOVE_PEER); } - auto isPeerDeleted = [&peerFD, this]()->bool { - Lock lock(mSocketsMutex); + Lock lock(mStateMutex); return mSockets.count(peerFD) == 0; }; @@ -170,49 +171,41 @@ void Processor::removePeer(const FileDescriptor peerFD) void Processor::removePeerInternal(const FileDescriptor peerFD, Status status) { - LOGW("Removing peer. ID: " << peerFD); - { - Lock lock(mSocketsMutex); - if (!mSockets.erase(peerFD)) { - LOGW("No such peer. Another thread called removePeerInternal"); - return; - } + LOGS("Processor removePeerInternal peerFD: " << peerFD); + LOGI("Removing peer. peerFD: " << peerFD); - // Remove from signal addressees - for (auto it = mSignalsPeers.begin(); it != mSignalsPeers.end();) { - it->second.remove(peerFD); - if (it->second.empty()) { - it = mSignalsPeers.erase(it); - } else { - ++it; - } - } + if (!mSockets.erase(peerFD)) { + LOGW("No such peer. Another thread called removePeerInternal"); + return; } - { - // Erase associated return value callbacks - Lock lock(mReturnCallbacksMutex); - - std::shared_ptr data; - for (auto it = mReturnCallbacks.begin(); it != mReturnCallbacks.end();) { - if (it->second.peerFD == peerFD) { - IGNORE_EXCEPTIONS(it->second.process(status, data)); - it = mReturnCallbacks.erase(it); - } else { - ++it; - } + // Remove from signal addressees + for (auto it = mSignalsPeers.begin(); it != mSignalsPeers.end();) { + it->second.remove(peerFD); + if (it->second.empty()) { + it = mSignalsPeers.erase(it); + } else { + ++it; } } - - { - Lock lock(mCallbacksMutex); - if (mRemovedPeerCallback) { - // Notify about the deletion - mRemovedPeerCallback(peerFD); + // Erase associated return value callbacks + std::shared_ptr data; + for (auto it = mReturnCallbacks.begin(); it != mReturnCallbacks.end();) { + if (it->second.peerFD == peerFD) { + IGNORE_EXCEPTIONS(it->second.process(status, data)); + it = mReturnCallbacks.erase(it); + } else { + ++it; } } + if (mRemovedPeerCallback) { + // Notify about the deletion + mRemovedPeerCallback(peerFD); + } + + resetPolling(); } @@ -222,25 +215,29 @@ void Processor::resetPolling() return; } - LOGI("Resetting polling"); - // Setup polling on eventfd and sockets - Lock lock(mSocketsMutex); - mFDs.resize(mSockets.size() + 1); + { + Lock lock(mStateMutex); - mFDs[0].fd = mEventQueue.getFD(); - mFDs[0].events = POLLIN; + // Setup polling on eventfd and sockets + mFDs.resize(mSockets.size() + 1); - auto socketIt = mSockets.begin(); - for (unsigned int i = 1; i < mFDs.size(); ++i) { - mFDs[i].fd = socketIt->second->getFD(); - mFDs[i].events = POLLIN | POLLHUP; // Listen for input events - ++socketIt; - // TODO: It's possible to block on writing to fd. Maybe listen for POLLOUT too? + mFDs[0].fd = mEventQueue.getFD(); + mFDs[0].events = POLLIN; + + auto socketIt = mSockets.begin(); + for (unsigned int i = 1; i < mFDs.size(); ++i) { + mFDs[i].fd = socketIt->second->getFD(); + mFDs[i].events = POLLIN | POLLHUP; // Listen for input events + ++socketIt; + // TODO: It's possible to block on writing to fd. Maybe listen for POLLOUT too? + } } } void Processor::run() { + LOGS("Processor run"); + resetPolling(); mIsRunning = true; @@ -282,60 +279,53 @@ void Processor::run() bool Processor::handleLostConnections() { - std::vector peersToRemove; + Lock lock(mStateMutex); + + bool isPeerRemoved = false; { - Lock lock(mSocketsMutex); for (unsigned int i = 1; i < mFDs.size(); ++i) { if (mFDs[i].revents & POLLHUP) { LOGI("Lost connection to peer: " << mFDs[i].fd); mFDs[i].revents &= ~(POLLHUP); - peersToRemove.push_back(mFDs[i].fd); + removePeerInternal(mFDs[i].fd, Status::PEER_DISCONNECTED); + isPeerRemoved = true; } } } - for (const FileDescriptor peerFD : peersToRemove) { - removePeerInternal(peerFD, Status::PEER_DISCONNECTED); - } - - return !peersToRemove.empty(); + return isPeerRemoved; } bool Processor::handleLostConnection(const FileDescriptor peerFD) { + Lock lock(mStateMutex); removePeerInternal(peerFD, Status::PEER_DISCONNECTED); return true; } bool Processor::handleInputs() { - std::vector peersWithInput; - { - Lock lock(mSocketsMutex); - for (unsigned int i = 1; i < mFDs.size(); ++i) { - if (mFDs[i].revents & POLLIN) { - mFDs[i].revents &= ~(POLLIN); - peersWithInput.push_back(mFDs[i].fd); - } - } - } + Lock lock(mStateMutex); bool pollChanged = false; - // Handle input outside the critical section - for (const FileDescriptor peerFD : peersWithInput) { - pollChanged = pollChanged || handleInput(peerFD); + for (unsigned int i = 1; i < mFDs.size(); ++i) { + if (mFDs[i].revents & POLLIN) { + mFDs[i].revents &= ~(POLLIN); + pollChanged = pollChanged || handleInput(mFDs[i].fd); + } } + return pollChanged; } bool Processor::handleInput(const FileDescriptor peerFD) { - LOGT("Handle incoming data"); + LOGS("Processor handleInput peerFD: " << peerFD); + Lock lock(mStateMutex); std::shared_ptr socketPtr; try { // Get the peer's socket - Lock lock(mSocketsMutex); socketPtr = mSockets.at(peerFD); } catch (const std::out_of_range&) { LOGE("No such peer: " << peerFD); @@ -360,22 +350,18 @@ bool Processor::handleInput(const FileDescriptor peerFD) return onReturnValue(*socketPtr, messageID); } else { - Lock lock(mCallsMutex); if (mMethodsCallbacks.count(methodID)) { // Method std::shared_ptr methodCallbacks = mMethodsCallbacks.at(methodID); - lock.unlock(); return onRemoteCall(*socketPtr, methodID, messageID, methodCallbacks); } else if (mSignalsCallbacks.count(methodID)) { // Signal std::shared_ptr signalCallbacks = mSignalsCallbacks.at(methodID); - lock.unlock(); return onRemoteSignal(*socketPtr, methodID, messageID, signalCallbacks); } else { // Nothing - lock.unlock(); LOGW("No method or signal callback for methodID: " << methodID); removePeerInternal(socketPtr->getFD(), Status::NAUGHTY_PEER); return true; @@ -387,9 +373,9 @@ bool Processor::handleInput(const FileDescriptor peerFD) std::shared_ptr Processor::onNewSignals(const FileDescriptor peerFD, std::shared_ptr& data) { - LOGD("New signals for peer: " << peerFD); - Lock lock(mSocketsMutex); - for (MethodID methodID : data->ids) { + LOGS("Processor onNewSignals peerFD: " << peerFD); + + for (const MethodID methodID : data->ids) { mSignalsPeers[methodID].push_back(peerFD); } @@ -399,10 +385,11 @@ std::shared_ptr Processor::onNewSignals(const FileDescript bool Processor::onReturnValue(const Socket& socket, const MessageID messageID) { - LOGI("Return value for messageID: " << messageID); + LOGS("Processor onReturnValue messageID: " << messageID); + + // LOGI("Return value for messageID: " << messageID); ReturnCallbacks returnCallbacks; try { - Lock lock(mReturnCallbacksMutex); LOGT("Getting the return callback"); returnCallbacks = std::move(mReturnCallbacks.at(messageID)); mReturnCallbacks.erase(messageID); @@ -423,9 +410,10 @@ bool Processor::onReturnValue(const Socket& socket, return true; } - LOGT("Process return value callback for messageID: " << messageID); + // LOGT("Process return value callback for messageID: " << messageID); IGNORE_EXCEPTIONS(returnCallbacks.process(Status::OK, data)); + // LOGT("Return value for messageID: " << messageID << " processed"); return false; } @@ -434,7 +422,9 @@ bool Processor::onRemoteSignal(const Socket& socket, const MessageID messageID, std::shared_ptr signalCallbacks) { - LOGI("Remote signal; methodID: " << methodID << " messageID: " << messageID); + LOGS("Processor onRemoteSignal; methodID: " << methodID << " messageID: " << messageID); + + // LOGI("Processor onRemoteSignal; methodID: " << methodID << " messageID: " << messageID); std::shared_ptr data; try { @@ -446,7 +436,7 @@ bool Processor::onRemoteSignal(const Socket& socket, return true; } - LOGT("Signal callback for methodID: " << methodID << "; messageID: " << messageID); + // LOGT("Signal callback for methodID: " << methodID << "; messageID: " << messageID); try { signalCallbacks->signal(socket.getFD(), data); } catch (const std::exception& e) { @@ -463,7 +453,8 @@ bool Processor::onRemoteCall(const Socket& socket, const MessageID messageID, std::shared_ptr methodCallbacks) { - LOGI("Remote call; methodID: " << methodID << " messageID: " << messageID); + LOGS("Processor onRemoteCall; methodID: " << methodID << " messageID: " << messageID); + // LOGI("Remote call; methodID: " << methodID << " messageID: " << messageID); std::shared_ptr data; try { @@ -503,13 +494,16 @@ bool Processor::onRemoteCall(const Socket& socket, bool Processor::handleEvent() { + LOGS("Processor handleEvent"); + + Lock lock(mStateMutex); + switch (mEventQueue.receive()) { + case Event::FINISH: { LOGD("Event FINISH"); - mIsRunning = false; cleanCommunication(); - return false; } @@ -534,91 +528,69 @@ bool Processor::handleEvent() bool Processor::onNewPeer() { - SocketInfo socketInfo; - { - Lock lock(mSocketsMutex); + LOGS("Processor onNewPeer"); - socketInfo = std::move(mNewSockets.front()); - mNewSockets.pop(); - - if (mSockets.size() > mMaxNumberOfPeers) { - LOGE("There are too many peers. I don't accept the connection with " << socketInfo.peerFD); - return false; - } - if (mSockets.count(socketInfo.peerFD) != 0) { - LOGE("There already was a socket for peerFD: " << socketInfo.peerFD); - return false; - } + // TODO: What if there is no newSocket? (request removed in the mean time) + // Add new socket of the peer + SocketInfo socketInfo = std::move(mNewSockets.front()); + mNewSockets.pop(); - mSockets[socketInfo.peerFD] = std::move(socketInfo.socketPtr); + if (mSockets.size() > mMaxNumberOfPeers) { + LOGE("There are too many peers. I don't accept the connection with " << socketInfo.peerFD); + return false; + } + if (mSockets.count(socketInfo.peerFD) != 0) { + LOGE("There already was a socket for peerFD: " << socketInfo.peerFD); + return false; } + mSockets[socketInfo.peerFD] = std::move(socketInfo.socketPtr); - // Broadcast the new signal to peers - LOGW("Sending handled signals"); - std::list peersFDs; - { - Lock lock(mSocketsMutex); - for (const auto kv : mSockets) { - peersFDs.push_back(kv.first); - } - } + // LOGW("Sending handled signals"); std::vector ids; - { - Lock lock(mSocketsMutex); - for (const auto kv : mSignalsCallbacks) { - ids.push_back(kv.first); - } + for (const auto kv : mSignalsCallbacks) { + ids.push_back(kv.first); } auto data = std::make_shared(ids); - - for (const FileDescriptor peerFD : peersFDs) { - callInternal(REGISTER_SIGNAL_METHOD_ID, - peerFD, - data, - discardResultHandler); - } - LOGW("Sent handled signals"); - + callAsync(REGISTER_SIGNAL_METHOD_ID, + socketInfo.peerFD, + data, + discardResultHandler); + // LOGW("Sent handled signals"); resetPolling(); - { - Lock lock(mCallbacksMutex); - if (mNewPeerCallback) { - // Notify about the new user. - LOGT("Calling NewPeerCallback"); - mNewPeerCallback(socketInfo.peerFD); - } + if (mNewPeerCallback) { + // Notify about the new user. + LOGT("Calling NewPeerCallback"); + mNewPeerCallback(socketInfo.peerFD); } + return true; } bool Processor::onRemovePeer() { - RemovePeerRequest request; - { - Lock lock(mSocketsMutex); - request = std::move(mPeersToDelete.front()); - mPeersToDelete.pop(); - } + LOGS("Processor onRemovePeer"); - removePeerInternal(request.peerFD, Status::REMOVED_PEER); - request.conditionPtr->notify_all(); - return true; -} + removePeerInternal(mPeersToDelete.front().peerFD, Status::REMOVED_PEER); -CallQueue::Call Processor::getCall() -{ - Lock lock(mCallsMutex); - return mCalls.pop(); + mPeersToDelete.front().conditionPtr->notify_all(); + mPeersToDelete.pop(); + return true; } bool Processor::onCall() { - LOGT("Handle call (from another thread) to send a message."); - CallQueue::Call call = getCall(); + LOGS("Processor onCall"); + CallQueue::Call call; + try { + call = std::move(mCalls.pop()); + } catch (const IPCException&) { + LOGE("No calls to serve, but got an EVENT::CALL. Event got removed before serving"); + return false; + } if (call.parse && call.process) { return onMethodCall(call); @@ -629,10 +601,11 @@ bool Processor::onCall() bool Processor::onSignalCall(CallQueue::Call& call) { + LOGS("Processor onSignalCall"); + std::shared_ptr socketPtr; try { // Get the peer's socket - Lock lock(mSocketsMutex); socketPtr = mSockets.at(call.peerFD); } catch (const std::out_of_range&) { LOGE("Peer disconnected. No socket with a peerFD: " << call.peerFD); @@ -653,15 +626,16 @@ bool Processor::onSignalCall(CallQueue::Call& call) } return false; - } bool Processor::onMethodCall(CallQueue::Call& call) { + LOGS("Processor onMethodCall"); std::shared_ptr socketPtr; + + try { // Get the peer's socket - Lock lock(mSocketsMutex); socketPtr = mSockets.at(call.peerFD); } catch (const std::out_of_range&) { LOGE("Peer disconnected. No socket with a peerFD: " << call.peerFD); @@ -672,22 +646,19 @@ bool Processor::onMethodCall(CallQueue::Call& call) return false; } - { - // Set what to do with the return message, but only if needed - Lock lock(mReturnCallbacksMutex); - if (mReturnCallbacks.count(call.messageID) != 0) { - LOGE("There already was a return callback for messageID: " << call.messageID); - } - mReturnCallbacks[call.messageID] = std::move(ReturnCallbacks(call.peerFD, - std::move(call.parse), - std::move(call.process))); + if (mReturnCallbacks.count(call.messageID) != 0) { + LOGE("There already was a return callback for messageID: " << call.messageID); } + mReturnCallbacks[call.messageID] = std::move(ReturnCallbacks(call.peerFD, + std::move(call.parse), + std::move(call.process))); try { // Send the call with the socket Socket::Guard guard = socketPtr->getGuard(); socketPtr->write(&call.methodID, sizeof(call.methodID)); socketPtr->write(&call.messageID, sizeof(call.messageID)); + LOGT("Serializing the message"); call.serialize(socketPtr->getFD(), call.data); } catch (const std::exception& e) { LOGE("Error during sending a method: " << e.what()); @@ -695,13 +666,12 @@ bool Processor::onMethodCall(CallQueue::Call& call) // Inform about the error, IGNORE_EXCEPTIONS(mReturnCallbacks[call.messageID].process(Status::SERIALIZATION_ERROR, call.data)); - { - Lock lock(mReturnCallbacksMutex); - mReturnCallbacks.erase(call.messageID); - } + mReturnCallbacks.erase(call.messageID); removePeerInternal(call.peerFD, Status::SERIALIZATION_ERROR); + return true; + } return false; @@ -709,35 +679,36 @@ bool Processor::onMethodCall(CallQueue::Call& call) void Processor::cleanCommunication() { + LOGS("Processor cleanCommunication"); + while (!mEventQueue.isEmpty()) { switch (mEventQueue.receive()) { case Event::FINISH: { - LOGD("Event FINISH after FINISH"); + LOGE("Event FINISH after FINISH"); break; } case Event::CALL: { - LOGD("Event CALL after FINISH"); - CallQueue::Call call = getCall(); - if (call.process) { - IGNORE_EXCEPTIONS(call.process(Status::CLOSING, call.data)); + LOGW("Event CALL after FINISH"); + try { + CallQueue::Call call = mCalls.pop(); + if (call.process) { + IGNORE_EXCEPTIONS(call.process(Status::CLOSING, call.data)); + } + } catch (const IPCException&) { + // No more calls } break; } case Event::ADD_PEER: { - LOGD("Event ADD_PEER after FINISH"); + LOGW("Event ADD_PEER after FINISH"); break; } case Event::REMOVE_PEER: { - LOGD("Event REMOVE_PEER after FINISH"); - RemovePeerRequest request; - { - Lock lock(mSocketsMutex); - request = std::move(mPeersToDelete.front()); - mPeersToDelete.pop(); - } - request.conditionPtr->notify_all(); + LOGW("Event REMOVE_PEER after FINISH"); + mPeersToDelete.front().conditionPtr->notify_all(); + mPeersToDelete.pop(); break; } } diff --git a/common/ipc/internals/processor.hpp b/common/ipc/internals/processor.hpp index 0e8fe8f..b0f7ea0 100644 --- a/common/ipc/internals/processor.hpp +++ b/common/ipc/internals/processor.hpp @@ -33,6 +33,7 @@ #include "config/manager.hpp" #include "config/fields.hpp" #include "logger/logger.hpp" +#include "logger/logger-scope.hpp" #include #include @@ -75,13 +76,13 @@ const unsigned int DEFAULT_METHOD_TIMEOUT = 1000; * - new way to generate UIDs * - callbacks for serialization/parsing * - store Sockets in a vector, maybe SocketStore? -* - fix valgrind tests * - poll loop outside. * - waiting till the EventQueue is empty before leaving stop() * - no new events added after stop() called * - when using IPCGSource: addFD and removeFD can be called from addPeer removePeer callbacks, but * there is no mechanism to ensure the IPCSource exists.. therefore SIGSEGV :) -* +* - EventQueue should store std::shared_ptr and it should be the only queue to the Processor thread. +* It should have an API for removing events from the middle of the queue * */ class Processor { @@ -279,7 +280,7 @@ public: private: typedef std::function& data)> SerializeCallback; typedef std::function(int fd)> ParseCallback; - typedef std::unique_lock Lock; + typedef std::unique_lock Lock; struct EmptyData { CONFIG_REGISTER_EMPTY @@ -376,27 +377,22 @@ private: bool mIsRunning; - // Mutex for the Calls queue and the map of methods. - std::mutex mCallsMutex; + CallQueue mCalls; std::unordered_map> mMethodsCallbacks; std::unordered_map> mSignalsCallbacks; std::unordered_map> mSignalsPeers; - // Mutex for changing mSockets map. - // Shouldn't be locked on any read/write, that could block. Just copy the ptr. - std::mutex mSocketsMutex; std::unordered_map > mSockets; std::vector mFDs; std::queue mNewSockets; std::queue mPeersToDelete; - // Mutex for modifying the map with return callbacks - std::mutex mReturnCallbacksMutex; std::unordered_map mReturnCallbacks; - // Mutex for setting callbacks - std::mutex mCallbacksMutex; + // Mutex for modifying any internal data + std::recursive_mutex mStateMutex; + PeerCallback mNewPeerCallback; PeerCallback mRemovedPeerCallback; @@ -408,12 +404,6 @@ private: void addMethodHandlerInternal(const MethodID methodID, const typename MethodHandler::type& process); - template - MessageID callInternal(const MethodID methodID, - const FileDescriptor peerFD, - const std::shared_ptr& data, - const typename ResultHandler::type& process); - template static void discardResultHandler(Status, std::shared_ptr&) {} @@ -438,7 +428,6 @@ private: std::shared_ptr signalCallbacks); void resetPolling(); FileDescriptor getNextFileDescriptor(); - CallQueue::Call getCall(); void removePeerInternal(const FileDescriptor peerFD, Status status); std::shared_ptr onNewSignals(const FileDescriptor peerFD, @@ -470,7 +459,7 @@ void Processor::addMethodHandlerInternal(const MethodID methodID, }; { - Lock lock(mCallsMutex); + Lock lock(mStateMutex); mMethodsCallbacks[methodID] = std::make_shared(std::move(methodCall)); } } @@ -485,14 +474,16 @@ void Processor::addMethodHandler(const MethodID methodID, } { - Lock lock(mCallsMutex); + Lock lock(mStateMutex); + if (mSignalsCallbacks.count(methodID)) { LOGE("MethodID used by a signal: " << methodID); throw IPCException("MethodID used by a signal: " + std::to_string(methodID)); } + + addMethodHandlerInternal(methodID, method); } - addMethodHandlerInternal(methodID, method); } template @@ -504,64 +495,49 @@ void Processor::addSignalHandler(const MethodID methodID, throw IPCException("Forbidden methodID: " + std::to_string(methodID)); } + std::shared_ptr data; + std::vector peerFDs; { - Lock lock(mCallsMutex); + Lock lock(mStateMutex); + + // Andd the signal handler: if (mMethodsCallbacks.count(methodID)) { LOGE("MethodID used by a method: " << methodID); throw IPCException("MethodID used by a method: " + std::to_string(methodID)); } - } - SignalHandlers signalCall; + SignalHandlers signalCall; - signalCall.parse = [](const int fd)->std::shared_ptr { - std::shared_ptr data(new ReceivedDataType()); - config::loadFromFD(fd, *data); - return data; - }; + signalCall.parse = [](const int fd)->std::shared_ptr { + std::shared_ptr data(new ReceivedDataType()); + config::loadFromFD(fd, *data); + return data; + }; - signalCall.signal = [handler](const FileDescriptor peerFD, std::shared_ptr& data) { - std::shared_ptr tmpData = std::static_pointer_cast(data); - handler(peerFD, tmpData); - }; + signalCall.signal = [handler](const FileDescriptor peerFD, std::shared_ptr& data) { + std::shared_ptr tmpData = std::static_pointer_cast(data); + handler(peerFD, tmpData); + }; - { - Lock lock(mCallsMutex); mSignalsCallbacks[methodID] = std::make_shared(std::move(signalCall)); - } - std::vector ids {methodID}; - auto data = std::make_shared(ids); + // Broadcast the new signal: + std::vector ids {methodID}; + data = std::make_shared(ids); - std::list peersFDs; - { - Lock lock(mSocketsMutex); for (const auto kv : mSockets) { - peersFDs.push_back(kv.first); + peerFDs.push_back(kv.first); } } - for (const FileDescriptor peerFD : peersFDs) { + for (const auto peerFD : peerFDs) { callSync(REGISTER_SIGNAL_METHOD_ID, peerFD, data, DEFAULT_METHOD_TIMEOUT); } - } -template -MessageID Processor::callInternal(const MethodID methodID, - const FileDescriptor peerFD, - const std::shared_ptr& data, - const typename ResultHandler::type& process) -{ - Lock lock(mCallsMutex); - MessageID messageID = mCalls.push(methodID, peerFD, data, process); - mEventQueue.send(Event::CALL); - - return messageID; -} template MessageID Processor::callAsync(const MethodID methodID, @@ -569,7 +545,11 @@ MessageID Processor::callAsync(const MethodID methodID, const std::shared_ptr& data, const typename ResultHandler::type& process) { - return callInternal(methodID, peerFD, data, process); + Lock lock(mStateMutex); + MessageID messageID = mCalls.push(methodID, peerFD, data, process); + mEventQueue.send(Event::CALL); + + return messageID; } @@ -586,7 +566,6 @@ std::shared_ptr Processor::callSync(const MethodID methodID, Status returnStatus = ipc::Status::UNDEFINED; auto process = [&result, &mutex, &cv, &returnStatus](Status status, std::shared_ptr returnedData) { - std::unique_lock lock(mutex); returnStatus = status; result = returnedData; cv.notify_all(); @@ -603,30 +582,25 @@ std::shared_ptr Processor::callSync(const MethodID methodID, std::unique_lock lock(mutex); LOGT("Waiting for the response..."); - // TODO: There is a race here. mReturnCallbacks were used to indicate if the return call was served or not, - // but if the timeout occurs before the call is even sent, then this method is broken. if (!cv.wait_for(lock, std::chrono::milliseconds(timeoutMS), isResultInitialized)) { - // Timeout occurred: - // - call isn't sent => delete it - // - call is sent and no reply => throw IPCTimeoutError - // - call is being serviced => wait for it with the same timeout - LOGT("Probably a timeout in callSync. Checking..."); - - bool isTimeout = false; + LOGW("Probably a timeout in callSync. Checking..."); + bool isTimeout; { - Lock lock(mReturnCallbacksMutex); - if (1 == mReturnCallbacks.erase(messageID)) { - // Return callback was present, so there was a timeout - isTimeout = true; - } + Lock lock(mStateMutex); + // Call isn't sent or call is sent but there is no reply + isTimeout = mCalls.erase(messageID) || 1 == mReturnCallbacks.erase(messageID); } + if (isTimeout) { - removePeer(peerFD); LOGE("Function call timeout; methodID: " << methodID); + removePeer(peerFD); throw IPCTimeoutException("Function call timeout; methodID: " + std::to_string(methodID)); } else { - //Timeout started during the return value processing, so wait for it to finish - cv.wait(lock, isResultInitialized); + LOGW("Timeout started during the return value processing, so wait for it to finish"); + if (!cv.wait_for(lock, std::chrono::milliseconds(timeoutMS), isResultInitialized)) { + LOGE("Function call timeout; methodID: " << methodID); + throw IPCTimeoutException("Function call timeout; methodID: " + std::to_string(methodID)); + } } } @@ -639,14 +613,13 @@ template void Processor::signal(const MethodID methodID, const std::shared_ptr& data) { - std::list peersFDs; - { - Lock lock(mSocketsMutex); - peersFDs = mSignalsPeers[methodID]; + Lock lock(mStateMutex); + const auto it = mSignalsPeers.find(methodID); + if (it == mSignalsPeers.end()) { + LOGW("No peer is handling signal with methodID: " << methodID); + return; } - - for (const FileDescriptor peerFD : peersFDs) { - Lock lock(mCallsMutex); + for (const FileDescriptor peerFD : it->second) { mCalls.push(methodID, peerFD, data); mEventQueue.send(Event::CALL); } diff --git a/common/ipc/ipc-gsource.cpp b/common/ipc/ipc-gsource.cpp index 4c098d9..5a4e137 100644 --- a/common/ipc/ipc-gsource.cpp +++ b/common/ipc/ipc-gsource.cpp @@ -48,7 +48,8 @@ IPCGSource::IPCGSource(const std::vector fds, const HandlerCallback& handlerCallback) : mHandlerCallback(handlerCallback) { - LOGD("Constructing IPCGSource"); + LOGS("IPCGSource constructor"); + for (const FileDescriptor fd : fds) { addFD(fd); } @@ -56,13 +57,13 @@ IPCGSource::IPCGSource(const std::vector fds, IPCGSource::~IPCGSource() { - LOGD("Destroying IPCGSource"); + LOGS("~IPCGSource"); } IPCGSource::Pointer IPCGSource::create(const std::vector& fds, const HandlerCallback& handlerCallback) { - LOGD("Creating IPCGSource"); + LOGS("Creating IPCGSource"); static GSourceFuncs funcs = { &IPCGSource::prepare, &IPCGSource::check, @@ -94,12 +95,13 @@ IPCGSource::Pointer IPCGSource::create(const std::vector& fds, void IPCGSource::addFD(const FileDescriptor fd) { + if (!&mGSource) { // In case it's called as a callback but the IPCGSource is destroyed return; } + LOGS("Adding fd to glib"); - LOGD("Adding fd to glib"); gpointer tag = g_source_add_unix_fd(&mGSource, fd, conditions); @@ -114,7 +116,7 @@ void IPCGSource::removeFD(const FileDescriptor fd) return; } - LOGD("Removing fd from glib"); + LOGS("Removing fd from glib"); auto it = std::find(mFDInfos.begin(), mFDInfos.end(), fd); if (it == mFDInfos.end()) { LOGE("No such fd"); @@ -126,7 +128,7 @@ void IPCGSource::removeFD(const FileDescriptor fd) guint IPCGSource::attach(GMainContext* context) { - LOGD("Attaching to GMainContext"); + LOGS("Attaching to GMainContext"); guint ret = g_source_attach(&mGSource, context); g_source_unref(&mGSource); return ret; @@ -177,6 +179,8 @@ gboolean IPCGSource::dispatch(GSource* gSource, void IPCGSource::finalize(GSource* gSource) { + LOGS("IPCGSource Finalize"); + if (gSource) { IPCGSource* source = reinterpret_cast(gSource); source->~IPCGSource(); diff --git a/common/ipc/service.cpp b/common/ipc/service.cpp index 5e720d6..ef46346 100644 --- a/common/ipc/service.cpp +++ b/common/ipc/service.cpp @@ -40,30 +40,27 @@ Service::Service(const std::string& socketPath, mAcceptor(socketPath, std::bind(&Processor::addPeer, &mProcessor, _1)) { - LOGD("Creating server"); + LOGS("Service Constructor"); } Service::~Service() { - LOGD("Destroying server..."); + LOGS("Service Destructor"); try { stop(); } catch (IPCException& e) { LOGE("Error in Service's destructor: " << e.what()); } - LOGD("Destroyed"); } void Service::start() { - LOGD("Starting server"); + LOGS("Service start"); mProcessor.start(); // There can be an incoming connection from mAcceptor before mProcessor is listening, // but it's OK. It will handle the connection when ready. So no need to wait for mProcessor. mAcceptor.start(); - - LOGD("Started server"); } bool Service::isStarted() @@ -73,10 +70,9 @@ bool Service::isStarted() void Service::stop() { - LOGD("Stopping server.."); + LOGS("Service stop"); mAcceptor.stop(); mProcessor.stop(); - LOGD("Stopped"); } std::vector Service::getFDs() @@ -116,19 +112,20 @@ void Service::handle(const FileDescriptor fd, const short pollEvent) void Service::setNewPeerCallback(const PeerCallback& newPeerCallback) { + LOGS("Service setNewPeerCallback"); mProcessor.setNewPeerCallback(newPeerCallback); } void Service::setRemovedPeerCallback(const PeerCallback& removedPeerCallback) { + LOGS("Service setRemovedPeerCallback"); mProcessor.setRemovedPeerCallback(removedPeerCallback); } void Service::removeMethod(const MethodID methodID) { - LOGD("Removing method " << methodID); + LOGS("Service removeMethod methodID: " << methodID); mProcessor.removeMethod(methodID); - LOGD("Removed " << methodID); } diff --git a/common/ipc/service.hpp b/common/ipc/service.hpp index ed83606..fa12e30 100644 --- a/common/ipc/service.hpp +++ b/common/ipc/service.hpp @@ -188,18 +188,16 @@ template void Service::addMethodHandler(const MethodID methodID, const typename MethodHandler::type& method) { - LOGD("Adding method with id " << methodID); + LOGS("Service addMethodHandler, methodID " << methodID); mProcessor.addMethodHandler(methodID, method); - LOGD("Added method with id " << methodID); } template void Service::addSignalHandler(const MethodID methodID, const typename SignalHandler::type& handler) { - LOGD("Adding signal with id " << methodID); + LOGS("Service addSignalHandler, methodID " << methodID); mProcessor.addSignalHandler(methodID, handler); - LOGD("Added signal with id " << methodID); } template @@ -208,7 +206,9 @@ std::shared_ptr Service::callSync(const MethodID methodID, const std::shared_ptr& data, unsigned int timeoutMS) { - LOGD("Sync calling method: " << methodID << " for user: " << peerFD); + LOGS("Service callSync, methodID: " << methodID + << ", peerFD: " << peerFD + << ", timeoutMS: " << timeoutMS); return mProcessor.callSync(methodID, peerFD, data, timeoutMS); } @@ -218,22 +218,20 @@ void Service::callAsync(const MethodID methodID, const std::shared_ptr& data, const typename ResultHandler::type& resultCallback) { - LOGD("Async calling method: " << methodID << " for user: " << peerFD); + LOGS("Service callAsync, methodID: " << methodID << ", peerFD: " << peerFD); mProcessor.callAsync(methodID, peerFD, data, resultCallback); - LOGD("Async called method: " << methodID << "for user: " << peerFD); } template void Service::signal(const MethodID methodID, const std::shared_ptr& data) { - LOGD("Signaling: " << methodID); + LOGS("Service signal, methodID: " << methodID); mProcessor.signal(methodID, data); - LOGD("Signaled: " << methodID); } } // namespace ipc diff --git a/tests/unit_tests/ipc/ut-ipc.cpp b/tests/unit_tests/ipc/ut-ipc.cpp index 74c18f3..7c9df6c 100644 --- a/tests/unit_tests/ipc/ut-ipc.cpp +++ b/tests/unit_tests/ipc/ut-ipc.cpp @@ -63,7 +63,7 @@ const int TIMEOUT = 1000 /*ms*/; const int SHORT_OPERATION_TIME = TIMEOUT / 100; // Time that will cause "TIMEOUT" methods to throw -const int LONG_OPERATION_TIME = 3 * TIMEOUT; +const int LONG_OPERATION_TIME = 500 + TIMEOUT; struct Fixture { std::string socketPath; @@ -89,7 +89,7 @@ struct SendData { }; struct LongSendData { - LongSendData(int i = 0, int waitTime = 1000): mSendData(i), mWaitTime(waitTime), intVal(i) {} + LongSendData(int i, int waitTime): mSendData(i), mWaitTime(waitTime), intVal(i) {} template void accept(Visitor visitor) @@ -605,6 +605,9 @@ BOOST_AUTO_TEST_CASE(AddSignalInRuntime) c.addSignalHandler(1, handlerA); c.addSignalHandler(2, handlerB); + // Wait for the signals to propagate to the Service + std::this_thread::sleep_for(std::chrono::milliseconds(2 * TIMEOUT)); + auto data = std::make_shared(1); s.signal(2, data); s.signal(1, data); -- 2.7.4 From e649fc7da6a8db91320c1e9f2aed653fba8dce69 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Wed, 14 Jan 2015 15:28:01 +0100 Subject: [PATCH 16/16] Fix: Drop exception thrown from async callback [Bug/Feature] Crash when exception are thrown from async callback [Cause] Exception from async callback [Solution] Drop exception [Verification] Build, install, run test on slp Change-Id: I00fad962beb488508d25a21a68ee7e487f317ba0 --- tests/unit_tests/server/ut-zones-manager.cpp | 33 +++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/server/ut-zones-manager.cpp b/tests/unit_tests/server/ut-zones-manager.cpp index a733e05..5b29340 100644 --- a/tests/unit_tests/server/ut-zones-manager.cpp +++ b/tests/unit_tests/server/ut-zones-manager.cpp @@ -42,6 +42,7 @@ #include "utils/fs.hpp" #include "utils/img.hpp" #include "utils/scoped-dir.hpp" +#include "logger/logger.hpp" #include #include @@ -79,6 +80,22 @@ const std::string FILE_CONTENT = "File content\n" const std::string NON_EXISTANT_ZONE_ID = "NON_EXISTANT_ZONE_ID"; const std::string ZONES_PATH = "/tmp/ut-zones"; // the same as in daemon.conf +/** + * Currently there is no way to propagate an error from async call + * dropException are used to prevent system crash + **/ +DbusConnection::AsyncMethodCallCallback +dropException(const DbusConnection::AsyncMethodCallCallback& fun) +{ + return [fun](dbus::AsyncMethodCallResult& arg) -> void { + try { + fun(arg); + } catch (const std::exception& ex) { + LOGE("Got exception: " << ex.what()); + } + }; +} + class DbusAccessory { public: static const int HOST_ID = 0; @@ -326,7 +343,7 @@ public: api::host::METHOD_CREATE_ZONE, parameters, "()", - asyncResult); + dropException(asyncResult)); } void callAsyncMethodDestroyZone(const std::string& id, @@ -346,7 +363,7 @@ public: api::host::METHOD_DESTROY_ZONE, parameters, "()", - asyncResult); + dropException(asyncResult)); } void callAsyncMethodShutdownZone(const std::string& id, @@ -366,7 +383,7 @@ public: api::host::METHOD_SHUTDOWN_ZONE, parameters, "()", - asyncResult); + dropException(asyncResult)); } void callAsyncMethodStartZone(const std::string& id, @@ -386,7 +403,7 @@ public: api::host::METHOD_START_ZONE, parameters, "()", - asyncResult); + dropException(asyncResult)); } void callMethodLockZone(const std::string& id) @@ -576,8 +593,8 @@ BOOST_AUTO_TEST_CASE(NotifyActiveZoneTest) dbus.second->callMethodNotify(); } - BOOST_CHECK(signalReceivedLatch.waitForN(dbuses.size() - 1u, EVENT_TIMEOUT)); - BOOST_CHECK(signalReceivedLatch.empty()); + BOOST_REQUIRE(signalReceivedLatch.waitForN(dbuses.size() - 1u, EVENT_TIMEOUT)); + BOOST_REQUIRE(signalReceivedLatch.empty()); //check if there are no signals that was received more than once for (const auto& source : signalReceivedSourcesMap[1]) { @@ -718,8 +735,8 @@ BOOST_AUTO_TEST_CASE(MoveFileTest) BOOST_CHECK_EQUAL(dbuses.at(1)->callMethodMove(ZONE2, TMP + "/file"), api::zone::FILE_MOVE_SUCCEEDED); - BOOST_CHECK(notificationLatch.wait(EVENT_TIMEOUT)); - BOOST_CHECK(notificationLatch.empty()); + BOOST_REQUIRE(notificationLatch.wait(EVENT_TIMEOUT)); + BOOST_REQUIRE(notificationLatch.empty()); BOOST_CHECK_EQUAL(notificationSource, ZONE1); BOOST_CHECK_EQUAL(notificationPath, TMP + "/file"); BOOST_CHECK_EQUAL(notificationRetcode, api::zone::FILE_MOVE_SUCCEEDED); -- 2.7.4