From 94b00b1c12569f6efeb68b4e086198f2671bff95 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Fri, 9 Jan 2015 10:11:28 +0100 Subject: [PATCH] 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