IPC: Refactoring

[Bug/Feature]   N/A
[Cause]         N/A
[Solution]      N/A
[Verification]  Build, install, run tests

Change-Id: I421b6af0c5da5b6f1d73d69e491e976f84894272

diff --git a/common/ipc/internals/processor.cpp b/common/ipc/internals/processor.cpp
index 9677991..9aadfff 100644 (file)
--- a/common/ipc/internals/processor.cpp
+++ b/common/ipc/internals/processor.cpp
@@ -110,7 +110,7 @@ Processor::PeerID Processor::addPeer(const std::shared_ptr<Socket>& socketPtr)
         mNewSockets.push(std::move(socketInfo));
     }
     LOGI("New peer added. Id: " << peerID);
-    mEventQueue.send(Event::NEW_PEER);
+    mEventQueue.send(Event::ADD_PEER);
 
     return peerID;
 }
@@ -125,7 +125,7 @@ void Processor::removePeer(const PeerID peerID)
         mPeersToDelete.push(std::move(request));
     }
 
-    mEventQueue.send(Event::DELETE_PEER);
+    mEventQueue.send(Event::REMOVE_PEER);
 
     auto isPeerDeleted = [&peerID, this] {
         Lock lock(mSocketsMutex);
@@ -168,41 +168,6 @@ void Processor::removePeerInternal(const PeerID peerID, Status status)
     resetPolling();
 }
 
-void Processor::cleanCommunication()
-{
-    while (!mEventQueue.isEmpty()) {
-        switch (mEventQueue.receive()) {
-        case Event::FINISH: {
-            LOGD("Event FINISH after FINISH");
-            break;
-        }
-        case Event::CALL: {
-            LOGD("Event CALL after FINISH");
-            Call call = getCall();
-            IGNORE_EXCEPTIONS(call.process(Status::CLOSING, call.data));
-            break;
-        }
-
-        case Event::NEW_PEER: {
-            LOGD("Event NEW_PEER after FINISH");
-            break;
-        }
-
-        case Event::DELETE_PEER: {
-            LOGD("Event DELETE_PEER after FINISH");
-            RemovePeerRequest request;
-            {
-                Lock lock(mSocketsMutex);
-                request = std::move(mPeersToDelete.front());
-                mPeersToDelete.pop();
-            }
-            request.conditionPtr->notify_all();
-            break;
-        }
-        }
-    }
-}
-
 void Processor::resetPolling()
 {
     LOGI("Resetting polling");
@@ -432,53 +397,63 @@ bool Processor::handleEvent()
 
     case Event::CALL: {
         LOGD("Event CALL");
-        return handleCall();
+        return onCall();
     }
 
-    case Event::NEW_PEER: {
-        LOGD("Event NEW_PEER");
-        SocketInfo socketInfo;
-        {
-            Lock lock(mSocketsMutex);
+    case Event::ADD_PEER: {
+        LOGD("Event ADD_PEER");
+        return onNewPeer();
+    }
 
-            socketInfo = std::move(mNewSockets.front());
-            mNewSockets.pop();
+    case Event::REMOVE_PEER: {
+        LOGD("Event REMOVE_PEER");
+        return onRemovePeer();
+    }
+    }
 
-            if (mSockets.size() > mMaxNumberOfPeers) {
-                LOGE("There are too many peers. I don't accept the connection with " << socketInfo.peerID);
-                return false;
-            }
-            if (mSockets.count(socketInfo.peerID) != 0) {
-                LOGE("There already was a socket for peerID: " << socketInfo.peerID);
-                return false;
-            }
+    return false;
+}
 
-            mSockets[socketInfo.peerID] = std::move(socketInfo.socketPtr);
-        }
-        resetPolling();
-        if (mNewPeerCallback) {
-            // Notify about the new user.
-            mNewPeerCallback(socketInfo.peerID);
-        }
-        return true;
-    }
+bool Processor::onNewPeer()
+{
+    SocketInfo socketInfo;
+    {
+        Lock lock(mSocketsMutex);
 
-    case Event::DELETE_PEER: {
-        LOGD("Event DELETE_PEER");
-        RemovePeerRequest request;
-        {
-            Lock lock(mSocketsMutex);
-            request = std::move(mPeersToDelete.front());
-            mPeersToDelete.pop();
+        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.peerID);
+            return false;
+        }
+        if (mSockets.count(socketInfo.peerID) != 0) {
+            LOGE("There already was a socket for peerID: " << socketInfo.peerID);
+            return false;
         }
 
-        removePeerInternal(request.peerID, Status::REMOVED_PEER);
-        request.conditionPtr->notify_all();
-        return true;
+        mSockets[socketInfo.peerID] = std::move(socketInfo.socketPtr);
     }
+    resetPolling();
+    if (mNewPeerCallback) {
+        // Notify about the new user.
+        mNewPeerCallback(socketInfo.peerID);
     }
+    return true;
+}
 
-    return false;
+bool Processor::onRemovePeer()
+{
+    RemovePeerRequest request;
+    {
+        Lock lock(mSocketsMutex);
+        request = std::move(mPeersToDelete.front());
+        mPeersToDelete.pop();
+    }
+
+    removePeerInternal(request.peerID, Status::REMOVED_PEER);
+    request.conditionPtr->notify_all();
+    return true;
 }
 
 Processor::MessageID Processor::getNextMessageID()
@@ -505,7 +480,7 @@ Processor::Call Processor::getCall()
     return call;
 }
 
-bool Processor::handleCall()
+bool Processor::onCall()
 {
     LOGT("Handle call (from another thread) to send a message.");
     Call call = getCall();
@@ -558,5 +533,40 @@ bool Processor::handleCall()
     return false;
 }
 
+void Processor::cleanCommunication()
+{
+    while (!mEventQueue.isEmpty()) {
+        switch (mEventQueue.receive()) {
+        case Event::FINISH: {
+            LOGD("Event FINISH after FINISH");
+            break;
+        }
+        case Event::CALL: {
+            LOGD("Event CALL after FINISH");
+            Call call = getCall();
+            IGNORE_EXCEPTIONS(call.process(Status::CLOSING, call.data));
+            break;
+        }
+
+        case Event::ADD_PEER: {
+            LOGD("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();
+            break;
+        }
+        }
+    }
+}
+
 } // namespace ipc
 } // namespace security_containers

diff --git a/common/ipc/internals/processor.hpp b/common/ipc/internals/processor.hpp
index e34bdb6..7554eb8 100644 (file)
--- a/common/ipc/internals/processor.hpp
+++ b/common/ipc/internals/processor.hpp
@@ -65,14 +65,7 @@ const unsigned int DEFAULT_MAX_NUMBER_OF_PEERS = 500;
 * - Rest: The data written in a callback. One type per method.ReturnCallbacks
 *
 * TODO:
-*  - remove ReturnCallbacks on peer disconnect
-*  - on sync timeout erase the return callback
-*  - don't throw timeout if the message is already processed
-*  - naming convention or methods that just commissions the PROCESS thread to do something
-*  - removePeer API function
-*  - error handling - special message type
 *  - some mutexes may not be needed
-*  - make addPeer synchronous like removePeer
 */
 class Processor {
 public:
@@ -81,7 +74,6 @@ public:
     typedef unsigned int MethodID;
     typedef unsigned int MessageID;
 
-
     /**
      * Method ID. Used to indicate a message with the return value.
      */
@@ -265,8 +257,8 @@ private:
     enum class Event : int {
         FINISH,     // Shutdown request
         CALL,       // New method call in the queue
-        NEW_PEER,   // New peer in the queue
-        DELETE_PEER // Delete peer
+        ADD_PEER,   // New peer in the queue
+        REMOVE_PEER // Remove peer
     };
     EventQueue<Event> mEventQueue;
 
@@ -303,7 +295,9 @@ private:
 
     void run();
     bool handleEvent();
-    bool handleCall();
+    bool onCall();
+    bool onNewPeer();
+    bool onRemovePeer();
     bool handleLostConnections();
     bool handleInputs();
     bool handleInput(const PeerID peerID, const Socket& socket);