diff options
author | Aleksandar Donchev <Aleksander.Donchev@partner.bmw.de> | 2017-04-24 17:14:32 +0200 |
---|---|---|
committer | Christian Linke <christian.linke@bmw.de> | 2017-05-02 06:25:51 -0700 |
commit | 375dc7b81795da9bfd8372f629d7fcef704a6d58 (patch) | |
tree | c16cae788798d7924ba74ca036c72ed3531fd208 /AudioManagerUtilities | |
parent | a737433a4362e8244801491857d28502adf4c76c (diff) | |
download | audiomanager-375dc7b81795da9bfd8372f629d7fcef704a6d58.tar.gz |
Fix for bug in CAmSocketHandler causing invalid pointers and crash.
Signed-off-by: Christian Linke <christian.linke@bmw.de>
Change-Id: I8d3d2b436ac9fcd61c73a28145a731f327cab1e2
Diffstat (limited to 'AudioManagerUtilities')
-rw-r--r-- | AudioManagerUtilities/include/CAmSerializer.h | 31 | ||||
-rw-r--r-- | AudioManagerUtilities/include/CAmSocketHandler.h | 11 | ||||
-rw-r--r-- | AudioManagerUtilities/src/CAmSocketHandler.cpp | 75 | ||||
-rw-r--r-- | AudioManagerUtilities/test/AmSerializerTest/CAmSerializerTest.cpp | 69 |
4 files changed, 124 insertions, 62 deletions
diff --git a/AudioManagerUtilities/include/CAmSerializer.h b/AudioManagerUtilities/include/CAmSerializer.h index 18f503d..6bd6143 100644 --- a/AudioManagerUtilities/include/CAmSerializer.h +++ b/AudioManagerUtilities/include/CAmSerializer.h @@ -77,6 +77,12 @@ namespace am * PluginRoutingInterfaceAsync. * */ + + /** + * \defgroup Deprecated Obsolete class! + * @{ + */ + namespace V1 { class CAmSerializer @@ -842,7 +848,9 @@ namespace am } }; - } + } /* namespace V1 */ + + /**@}*/ namespace V2 { @@ -967,16 +975,16 @@ namespace am int mReturnPipe[2]; //!< pipe handling returns sh_pollHandle_t mHandle; CAmSocketHandler* mpSocketHandler; - std::deque<CAmDelegagePtr> mListDelegatePoiters; //!< intermediate queue to store the pipe results + std::deque<CAmDelegagePtr> mListDelegatePointers; //!< intermediate queue to store the pipe results public: /** * get the size of delegate pointers */ - int getListDelegatePoiters() + size_t getListDelegatePointers() { - return mListDelegatePoiters.size(); + return mListDelegatePointers.size(); } /** @@ -1057,7 +1065,7 @@ namespace am /** * calls a function with variadic arguments threadsafe - * @param invocation is a type is produced by std::bind + * @param invocation is a type produced by std::bind * \section ex Example: * @code * CAmSerializer serial(&Sockethandler); @@ -1125,7 +1133,7 @@ namespace am logError("CAmSerializer::receiverCallback could not read pipe!"); throw std::runtime_error("CAmSerializer Could not read pipe!"); } - mListDelegatePoiters.assign(listPointers, listPointers + (numReads / sizeof(CAmDelegagePtr))); + mListDelegatePointers.assign(listPointers, listPointers + (numReads / sizeof(CAmDelegagePtr))); } /** @@ -1135,7 +1143,7 @@ namespace am { (void) handle; (void) userData; - if (mListDelegatePoiters.empty()) + if (mListDelegatePointers.empty()) return (false); return (true); } @@ -1147,11 +1155,11 @@ namespace am { (void) handle; (void) userData; - CAmDelegagePtr delegatePoiter = mListDelegatePoiters.front(); - mListDelegatePoiters.pop_front(); + CAmDelegagePtr delegatePoiter = mListDelegatePointers.front(); + mListDelegatePointers.pop_front(); if (delegatePoiter->call(mReturnPipe)) delete delegatePoiter; - if (mListDelegatePoiters.empty()) + if (mListDelegatePointers.empty()) return (false); return (true); } @@ -1169,7 +1177,7 @@ namespace am mReturnPipe(), // mHandle(), mpSocketHandler(iSocketHandler), - mListDelegatePoiters(), // + mListDelegatePointers(), // receiverCallbackT(this, &CAmSerializer::receiverCallback), // dispatcherCallbackT(this, &CAmSerializer::dispatcherCallback), // checkerCallbackT(this, &CAmSerializer::checkerCallback) @@ -1205,5 +1213,6 @@ namespace am } /* namespace V2 */ typedef V1::CAmSerializer CAmSerializer DEPRECATED("You should use V2::CAmSerializer instead!"); + } /* namespace am */ #endif /* CAMSERIALIZER_H_ */ diff --git a/AudioManagerUtilities/include/CAmSocketHandler.h b/AudioManagerUtilities/include/CAmSocketHandler.h index c2a1aa3..0f9b710 100644 --- a/AudioManagerUtilities/include/CAmSocketHandler.h +++ b/AudioManagerUtilities/include/CAmSocketHandler.h @@ -321,7 +321,6 @@ namespace am int mPipe[2]; int mDispatchDone; //this starts / stops the mainloop - VectorListPollfd_t mfdPollingArray; //!<the polling array for ppoll sh_identifier_s mSetPollKeys; //!A set of all used ppoll keys VectorListPoll_t mListPoll; //!<list that holds all information for the ppoll sh_identifier_s mSetTimerKeys; //!A set of all used timer keys @@ -435,14 +434,14 @@ namespace am * @param a * @return */ - inline static void prepare(am::CAmSocketHandler::sh_poll_s& row); - + inline static void prepare(sh_poll_s& row); + /** * functor to return all fired events * @param a * @return */ - inline static void fire(sh_poll_s* a); + inline static void fire(sh_poll_s& a); /** * functor to return all fired events @@ -456,14 +455,14 @@ namespace am * @param a * @return */ - inline static bool noDispatching(const sh_poll_s* a); + inline static bool noDispatching(const sh_poll_s& a); /** * checks if dispatching is already finished * @param a * @return */ - inline static bool dispatchingFinished(const sh_poll_s* a); + inline static bool dispatchingFinished(const sh_poll_s& a); /** * timer fire callback diff --git a/AudioManagerUtilities/src/CAmSocketHandler.cpp b/AudioManagerUtilities/src/CAmSocketHandler.cpp index 7c6fb9f..fe7cf58 100644 --- a/AudioManagerUtilities/src/CAmSocketHandler.cpp +++ b/AudioManagerUtilities/src/CAmSocketHandler.cpp @@ -45,7 +45,6 @@ namespace am CAmSocketHandler::CAmSocketHandler() : mPipe(), // mDispatchDone(1), // - mfdPollingArray(), // mSetPollKeys(MAX_POLLHANDLE), // mListPoll(), // mSetTimerKeys(MAX_TIMERHANDLE), @@ -108,39 +107,44 @@ namespace am clock_gettime(CLOCK_MONOTONIC, &mStartTime); #endif timespec buffertime; - //freeze mListPoll by copying it - otherwise we get problems when we want to manipulate it during the next lines - std::list<sh_poll_s*> listPoll; + + std::list<sh_poll_s> listPoll; + VectorListPoll_t cloneListPoll; VectorListPoll_t::iterator listmPollIt; VectorListPollfd_t::iterator itMfdPollingArray; - + VectorListPollfd_t fdPollingArray; //!<the polling array for ppoll + auto preparePollfd = [&](const sh_poll_s& row) { CAmSocketHandler::prepare((sh_poll_s&)row); pollfd temp = row.pollfdValue; temp.revents = 0; - mfdPollingArray.push_back(temp); + fdPollingArray.push_back(temp); }; while (!mDispatchDone) { if (mRecreatePollfds) { - mfdPollingArray.clear(); + fdPollingArray.clear(); + //freeze mListPoll by copying it - otherwise we get problems when we want to manipulate it during the next lines + cloneListPoll = mListPoll; //there was a change in the setup, so we need to recreate the fdarray from the list - std::for_each(mListPoll.begin(), mListPoll.end(), preparePollfd); + std::for_each(cloneListPoll.begin(), cloneListPoll.end(), preparePollfd); mRecreatePollfds = false; } else { //first we go through the registered filedescriptors and check if someone needs preparation: - std::for_each(mListPoll.begin(), mListPoll.end(), CAmSocketHandler::prepare); + std::for_each(cloneListPoll.begin(), cloneListPoll.end(), CAmSocketHandler::prepare); } + #ifndef WITH_TIMERFD timerCorrection(); #endif //block until something is on a filedescriptor - if ((pollStatus = ppoll(&mfdPollingArray[0], mfdPollingArray.size(), insertTime(buffertime), &sigmask)) < 0) + if ((pollStatus = ppoll(&fdPollingArray[0], fdPollingArray.size(), insertTime(buffertime), &sigmask)) < 0) { if (errno == EINTR) { @@ -159,24 +163,25 @@ namespace am //todo: here could be a timer that makes sure naughty plugins return! listPoll.clear(); //stage 0+1, call firedCB - for (itMfdPollingArray = mfdPollingArray.begin(); itMfdPollingArray != mfdPollingArray.end(); itMfdPollingArray++) + for (itMfdPollingArray = fdPollingArray.begin(); itMfdPollingArray != fdPollingArray.end(); itMfdPollingArray++) { if (CAmSocketHandler::eventFired(*itMfdPollingArray)) - { - listmPollIt = mListPoll.begin() + (itMfdPollingArray - mfdPollingArray.begin()); - am::CAmSocketHandler::sh_poll_s * pollItem = &((am::CAmSocketHandler::sh_poll_s&) (*listmPollIt)); - listPoll.push_back(pollItem); - CAmSocketHandler::fire(pollItem); + { + listmPollIt = cloneListPoll.begin(); + std::advance(listmPollIt, std::distance(fdPollingArray.begin(), itMfdPollingArray)); + + listPoll.push_back(*listmPollIt); + CAmSocketHandler::fire(*listmPollIt); } } - + //stage 2, lets ask around if some dispatching is necessary, the ones who need stay on the list - listPoll.erase(std::remove_if(listPoll.begin(), listPoll.end(), CAmSocketHandler::noDispatching), listPoll.end()); + listPoll.remove_if(CAmSocketHandler::noDispatching); //stage 3, the ones left need to dispatch, we do this as long as there is something to dispatch.. do { - listPoll.erase(std::remove_if(listPoll.begin(), listPoll.end(), CAmSocketHandler::dispatchingFinished), listPoll.end()); + listPoll.remove_if(CAmSocketHandler::dispatchingFinished); } while (!listPoll.empty()); } @@ -915,45 +920,45 @@ namespace am /** * fire callback */ - void CAmSocketHandler::fire(sh_poll_s* a) + void CAmSocketHandler::fire(sh_poll_s& a) { try { - a->firedCB(a->pollfdValue, a->handle, a->userData); + a.firedCB(a.pollfdValue, a.handle, a.userData); } catch (std::exception& e) { - logError("Sockethandler: Exception in FireCallback,caught", e.what()); + logError("Sockethandler: Exception in Preparecallback,caught", e.what()); } } /** - * event triggered - */ - bool CAmSocketHandler::eventFired(const pollfd& a) - { - return (a.revents == 0 ? false : true); - } - - /** * should disptach */ - bool CAmSocketHandler::noDispatching(const sh_poll_s* a) + bool CAmSocketHandler::noDispatching(const sh_poll_s& a) { //remove from list of there is no checkCB - if (!a->checkCB) + if (nullptr == a.checkCB) return (true); - return (!a->checkCB(a->handle, a->userData)); + return (!a.checkCB(a.handle, a.userData)); } /** * disptach */ - bool CAmSocketHandler::dispatchingFinished(const sh_poll_s* a) + bool CAmSocketHandler::dispatchingFinished(const sh_poll_s& a) { //remove from list of there is no dispatchCB - if (!a->dispatchCB) + if (nullptr == a.dispatchCB) return (true); - return (!a->dispatchCB(a->handle, a->userData)); + return (!a.dispatchCB(a.handle, a.userData)); + } + + /** + * event triggered + */ + bool CAmSocketHandler::eventFired(const pollfd& a) + { + return (a.revents == 0 ? false : true); } /** diff --git a/AudioManagerUtilities/test/AmSerializerTest/CAmSerializerTest.cpp b/AudioManagerUtilities/test/AmSerializerTest/CAmSerializerTest.cpp index 508ed45..49c6738 100644 --- a/AudioManagerUtilities/test/AmSerializerTest/CAmSerializerTest.cpp +++ b/AudioManagerUtilities/test/AmSerializerTest/CAmSerializerTest.cpp @@ -79,7 +79,7 @@ struct SerializerData V2::CAmSerializer *pSerializer; }; -void* ptSerializer(void* data) +void* ptSerializerSync(void* data) { SerializerData *pData = (SerializerData*) data; std::string testStr(pData->testStr); @@ -91,11 +91,27 @@ void* ptSerializer(void* data) #pragma GCC diagnostic ignored "-Wdeprecated-declarations" pData->pSerializer->syncCall(pData->pSerCb, &MockIAmSerializerCb::check); pData->pSerializer->syncCall(pData->pSerCb, &MockIAmSerializerCb::checkInt, pData->result); - pData->pSerializer->syncCall(pData->pSerCb, &MockIAmSerializerCb::dispatchData, result, ten, pData->testStr); + pData->pSerializer->syncCall(pData->pSerCb, &MockIAmSerializerCb::dispatchData, result, ten, pData->testStr); +#pragma GCC diagnostic pop + return (NULL); +} + +void* ptSerializerASync(void* data) +{ + SerializerData *pData = (SerializerData*) data; + std::string testStr; + bool result = false; + int r = 0; + const uint32_t ten = 10; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" for (uint32_t i = 0; i < 5; i++) + { + testStr = pData->testStr; pData->pSerializer->asyncCall(pData->pSerCb, &MockIAmSerializerCb::dispatchData, i, testStr); - + } + pData->testStr = testStr; pData->pSerializer->asyncInvocation(std::bind([]()->bool { return 1;})); pData->pSerializer->asyncInvocation(std::bind([](const int i, int & result) @@ -105,6 +121,7 @@ void* ptSerializer(void* data) pData->pSerializer->asyncCall(pData->pSerCb, &MockIAmSerializerCb::check); pData->pSerializer->asyncCall(pData->pSerCb, &MockIAmSerializerCb::checkInt); + #pragma GCC diagnostic pop return (NULL); } @@ -113,7 +130,7 @@ ACTION(ActionDispatchData){ arg1="DispatchData"; } -TEST(CAmSerializerTest, serializerTest) +TEST(CAmSerializerTest, syncTest) { pthread_t serThread; @@ -135,14 +152,12 @@ TEST(CAmSerializerTest, serializerTest) serializerData.pSerCb = &serCb; serializerData.pSocketHandler = &myHandler; serializerData.pSerializer = &serializer; - pthread_create(&serThread, NULL, ptSerializer, &serializerData); + pthread_create(&serThread, NULL, ptSerializerSync, &serializerData); - EXPECT_CALL(serCb,check()).Times(3); - EXPECT_CALL(serCb,checkInt()).Times(2).WillRepeatedly(Return(100)); + EXPECT_CALL(serCb,check()).Times(1); + EXPECT_CALL(serCb,checkInt()).Times(1).WillRepeatedly(Return(100)); + EXPECT_CALL(serCb,dispatchData(10,testStr)).Times(1).WillRepeatedly(DoAll(ActionDispatchData(), Return(true))); - EXPECT_CALL(serCb,dispatchData(10,testStr)).WillOnce(DoAll(ActionDispatchData(), Return(true))); - for (int i = 0; i < 5; i++) - EXPECT_CALL(serCb,dispatchData(i,testStr)).WillOnce(DoAll(ActionDispatchData(), Return(true))); myHandler.start_listenting(); pthread_join(serThread, NULL); @@ -150,6 +165,40 @@ TEST(CAmSerializerTest, serializerTest) ASSERT_TRUE(serializerData.result == 100); } +TEST(CAmSerializerTest, asyncTest) +{ + pthread_t serThread; + + MockIAmSerializerCb serCb; + CAmSocketHandler myHandler; + std::string testStr("testStr"); + V2::CAmSerializer serializer(&myHandler); + sh_timerHandle_t handle; + timespec timeout4; + timeout4.tv_nsec = 0; + timeout4.tv_sec = 3; + CAmTimerSockethandlerController testCallback4(&myHandler, timeout4); + myHandler.addTimer(timeout4, &testCallback4.pTimerCallback, handle, NULL); + EXPECT_CALL(testCallback4,timerCallback(handle,NULL)).Times(1); + + SerializerData serializerData; + serializerData.result = 0; + serializerData.testStr = testStr; + serializerData.pSerCb = &serCb; + serializerData.pSocketHandler = &myHandler; + serializerData.pSerializer = &serializer; + pthread_create(&serThread, NULL, ptSerializerASync, &serializerData); + + EXPECT_CALL(serCb,check()).Times(2); + EXPECT_CALL(serCb,checkInt()).Times(1).WillRepeatedly(Return(100)); + for (int i = 0; i < 5; i++) + EXPECT_CALL(serCb,dispatchData(i,testStr)).WillOnce(DoAll(ActionDispatchData(), Return(true))); + + myHandler.start_listenting(); + + pthread_join(serThread, NULL); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); |