From bc33226f59910a960f62d419ba10d4ea761e3724 Mon Sep 17 00:00:00 2001
From: Aleksandar Donchev <Aleksander.Donchev@partner.bmw.de>
Date: Tue, 26 Sep 2017 16:07:00 +0200
Subject: A filedescriptor removal will set an invalidation flag which will
 prevent calls on the invalidated objects in the current iteration.

Signed-off-by: Christian Linke <christian.linke@bmw.de>

Change-Id: I9d5d3c434ac9fad62c76a76145c731b538aeb1e3

# Conflicts:
#	AudioManagerUtilities/src/CAmSocketHandler.cpp
---
 AudioManagerUtilities/include/CAmSocketHandler.h   | 11 ++-
 AudioManagerUtilities/src/CAmSocketHandler.cpp     | 85 ++++++++++++----------
 .../AmSocketHandlerTest/CAmSocketHandlerTest.cpp   |  9 ++-
 3 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/AudioManagerUtilities/include/CAmSocketHandler.h b/AudioManagerUtilities/include/CAmSocketHandler.h
index 7baa496..d14c1a3 100644
--- a/AudioManagerUtilities/include/CAmSocketHandler.h
+++ b/AudioManagerUtilities/include/CAmSocketHandler.h
@@ -217,6 +217,7 @@ class CAmSocketHandler
 {
     struct sh_poll_s //!<struct that holds information about polls
     {
+        bool isValid;
         sh_pollHandle_t handle; //!<handle to uniquely adress a filedesriptor
         pollfd pollfdValue; //!<the array for polling the filedescriptors
         std::function<void(const sh_pollHandle_t handle, void* userData)> prepareCB; //preperation callback
@@ -226,7 +227,7 @@ class CAmSocketHandler
         void* userData;
 
         sh_poll_s() :
-                handle(0), pollfdValue(), prepareCB(), firedCB(), checkCB(), dispatchCB(), userData(0)
+                isValid(true), handle(0), pollfdValue(), prepareCB(), firedCB(), checkCB(), dispatchCB(), userData(0)
         {}
     };
 
@@ -302,9 +303,11 @@ class CAmSocketHandler
     bool mRecreatePollfds; //!<when this is true, the poll list needs to be recreated
     internal_codes_t mInternalCodes;
     sh_pollHandle_t mSignalFdHandle;
+    VectorListPoll_t mListActivePolls;
 #ifndef WITH_TIMERFD
     timespec mStartTime; //!<here the actual time is saved for timecorrection
 #endif
+    
 private:
     bool fdIsValid(const int fd) const;
 
@@ -413,7 +416,7 @@ private:
       * @param a
       * @return
       */
-    inline static void fire(sh_poll_s& a);
+    inline static void fire(const sh_poll_s* a);
 
     /**
       * functor to return all fired events
@@ -427,14 +430,14 @@ private:
       * @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 943a80c..54ba4ef 100644
--- a/AudioManagerUtilities/src/CAmSocketHandler.cpp
+++ b/AudioManagerUtilities/src/CAmSocketHandler.cpp
@@ -58,7 +58,8 @@ CAmSocketHandler::CAmSocketHandler() :
         mSignalHandlers(), //
         mRecreatePollfds(true),
         mInternalCodes(internal_codes_e::NO_ERROR),
-        mSignalFdHandle(0)
+        mSignalFdHandle(0),
+        mListActivePolls()
         #ifndef WITH_TIMERFD
         ,mStartTime() //
         #endif
@@ -73,13 +74,10 @@ CAmSocketHandler::CAmSocketHandler() :
     short event = 0;
     sh_pollHandle_t handle;
     event |= POLLIN;
-    if (addFDPoll(mPipe[0], event, NULL,
-                [](const pollfd, const sh_pollHandle_t, void*){},
-                [](const sh_pollHandle_t, void*) { return (false); },
-            NULL, NULL, handle) != E_OK)
-    {
+    if (addFDPoll(mPipe[0], event, NULL, [](const pollfd pollfd, const sh_pollHandle_t, void*)
+    {}, [](const sh_pollHandle_t, void*)
+    {   return (false);}, NULL, NULL, handle) != E_OK)
         mInternalCodes |= internal_codes_e::FD_ERROR;
-    }
 }
 
 CAmSocketHandler::~CAmSocketHandler()
@@ -109,8 +107,9 @@ void CAmSocketHandler::start_listenting()
 #endif    
     timespec buffertime;
     
-    VectorListPoll_t cloneListPoll;
+    std::list<sh_poll_s*> listPoll;
     VectorListPoll_t::iterator listmPollIt;
+    VectorListPollfd_t::iterator itMfdPollingArray;
     VectorListPollfd_t fdPollingArray; //!<the polling array for ppoll
     
     auto preparePollfd = [&](const sh_poll_s& row)
@@ -130,15 +129,15 @@ void CAmSocketHandler::start_listenting()
             #endif
             fdPollingArray.clear();
             //freeze mListPoll by copying it - otherwise we get problems when we want to manipulate it during the next lines
-            cloneListPoll = mListPoll;
+            mListActivePolls = mListPoll;
             //there was a change in the setup, so we need to recreate the fdarray from the list
-            std::for_each(cloneListPoll.begin(), cloneListPoll.end(), preparePollfd);
+            std::for_each(mListActivePolls.begin(), mListActivePolls.end(), preparePollfd);
             mRecreatePollfds = false;
         }
         else
         {
             //first we go through the registered filedescriptors and check if someone needs preparation:
-            std::for_each(cloneListPoll.begin(), cloneListPoll.end(), CAmSocketHandler::prepare);
+            std::for_each(mListActivePolls.begin(), mListActivePolls.end(), CAmSocketHandler::prepare);
         }
 
 #ifndef WITH_TIMERFD
@@ -162,25 +161,23 @@ void CAmSocketHandler::start_listenting()
 
         if (pollStatus != 0) //only check filedescriptors if there was a change
         {
-            std::list<sh_poll_s> listPoll;
             //todo: here could be a timer that makes sure naughty plugins return!
+            listPoll.clear();
             //stage 0+1, call firedCB
-            listmPollIt = cloneListPoll.begin();
-            for (auto it : fdPollingArray)
+            for (itMfdPollingArray = fdPollingArray.begin(); itMfdPollingArray != fdPollingArray.end(); itMfdPollingArray++)
             {
-                if (CAmSocketHandler::eventFired(it))
-                {
-                    listmPollIt->pollfdValue.revents = it.revents;
-                    listPoll.push_back(*listmPollIt);
-                    CAmSocketHandler::fire(*listmPollIt);
-                }
-                else
-                {
-                    listmPollIt->pollfdValue.revents = 0;
+                if (CAmSocketHandler::eventFired(*itMfdPollingArray))
+                {                        
+                    listmPollIt = mListActivePolls.begin();
+                    std::advance(listmPollIt, std::distance(fdPollingArray.begin(), itMfdPollingArray));
+                    
+                    sh_poll_s & pollObj = *listmPollIt;
+                    
+                    listPoll.push_back(&pollObj);
+                    CAmSocketHandler::fire(&pollObj);
                 }
-                listmPollIt++;
             }
-            
+ 
             //stage 2, lets ask around if some dispatching is necessary, the ones who need stay on the list
             listPoll.remove_if(CAmSocketHandler::noDispatching);
 
@@ -415,7 +412,7 @@ am::am_Error_e CAmSocketHandler::addFDPoll(const int fd, const short event, IAmS
 {
 
     std::function<void(const sh_pollHandle_t handle, void* userData)> prepareCB; //preperation callback
-    std::function<void(const pollfd poll, const sh_pollHandle_t handle, void* userData)> firedCB; //fired callback
+    std::function<void(const pollfd pollfd, const sh_pollHandle_t handle, void* userData)> firedCB; //fired callback
     std::function<bool(const sh_pollHandle_t handle, void* userData)> checkCB; //check callback
     std::function<bool(const sh_pollHandle_t handle, void* userData)> dispatchCB; //check callback
 
@@ -439,7 +436,7 @@ am::am_Error_e CAmSocketHandler::addFDPoll(const int fd, const short event, IAmS
 am_Error_e CAmSocketHandler::removeFDPoll(const sh_pollHandle_t handle)
 {
     VectorListPoll_t::iterator iterator = mListPoll.begin();
-
+    
     for (; iterator != mListPoll.end(); ++iterator)
     {
         if (iterator->handle == handle)
@@ -447,10 +444,24 @@ am_Error_e CAmSocketHandler::removeFDPoll(const sh_pollHandle_t handle)
             iterator = mListPoll.erase(iterator);
             mSetPollKeys.pollHandles.erase(handle);
             mRecreatePollfds = true;
-            return (E_OK);
+            break;
         }
     }
-    return (E_UNKNOWN);
+    
+    if (iterator == mListPoll.end())
+        return (E_UNKNOWN);
+    
+    VectorListPoll_t::iterator iteratorActivePolls = mListActivePolls.begin();
+    for (; iteratorActivePolls != mListActivePolls.end(); ++iteratorActivePolls)
+    {
+        if (iteratorActivePolls->handle == handle)
+        {
+            iteratorActivePolls->isValid = false;
+            break;
+        }
+    }
+    
+    return (E_OK);
 }
 
 /**
@@ -990,11 +1001,11 @@ void CAmSocketHandler::prepare(am::CAmSocketHandler::sh_poll_s& row)
 /**
   * fire callback
   */
-void CAmSocketHandler::fire(sh_poll_s& a)
+void CAmSocketHandler::fire(const 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 Preparecallback,caught", e.what());
@@ -1004,23 +1015,23 @@ void CAmSocketHandler::fire(sh_poll_s& a)
 /**
   * 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 (nullptr == a.checkCB)
+    if (nullptr == a->checkCB || false==a->isValid )
         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 (nullptr == a.dispatchCB)
+    if (nullptr == a->dispatchCB || false==a->isValid )
         return (true);
-    return (!a.dispatchCB(a.handle, a.userData));
+    return (!a->dispatchCB(a->handle, a->userData));
 }
 
 /**
diff --git a/AudioManagerUtilities/test/AmSocketHandlerTest/CAmSocketHandlerTest.cpp b/AudioManagerUtilities/test/AmSocketHandlerTest/CAmSocketHandlerTest.cpp
index 129f896..07f6aaf 100644
--- a/AudioManagerUtilities/test/AmSocketHandlerTest/CAmSocketHandlerTest.cpp
+++ b/AudioManagerUtilities/test/AmSocketHandlerTest/CAmSocketHandlerTest.cpp
@@ -36,9 +36,12 @@
 //todo: expand test, implement more usecases
 //todo: test removeFD
 
+#undef ENABLED_SOCKETHANDLER_TEST_OUTPUT
+#undef ENABLED_TIMERS_TEST_OUTPUT
+
 #define SOCK_PATH "/tmp/mysock"
 
-#define SOCKET_TEST_LOOPS_COUNT 10
+#define SOCKET_TEST_LOOPS_COUNT 50
 #define TIMERS_TO_TEST 500  
 
 using namespace testing;
@@ -464,7 +467,7 @@ TEST(CAmSocketHandlerTest, timersStressTest)
     
     timespec timeoutTime;
     timeoutTime.tv_sec = 0;
-    timeoutTime.tv_nsec = 50000000;// 0,05 
+    timeoutTime.tv_nsec = 10000000;// 0,01 
     
     std::vector<CAmTimerStressTest*> timers; 
    
@@ -896,7 +899,7 @@ CAmSamplePluginStressTest::CAmSamplePluginStressTest(CAmSocketHandler *mySocketH
     userData.f = 1.f;
     timespec timeoutTime;
     timeoutTime.tv_sec = 0;
-    timeoutTime.tv_nsec = 500000000;// 0,5 
+    timeoutTime.tv_nsec = 10000000;// 0,01 
     for(int i=0;i<TIMERS_TO_TEST;i++)
     {
         CAmTimerStressTest2 *ptestCallback1 = new CAmTimerStressTest2(mySocketHandler, timeoutTime, 0);
-- 
cgit v1.2.1