Connection on onMessageProcess captured as copy 67/73667/2
authorKyungwook Tak <k.tak@samsung.com>
Thu, 9 Jun 2016 06:16:52 +0000 (15:16 +0900)
committerKyungwook Tak <k.tak@samsung.com>
Thu, 9 Jun 2016 06:21:34 +0000 (15:21 +0900)
Connection can be abnormally closed by the peer and with
std::shared_ptr<Connection> &, it's hardly guarantee validity of the
reference. So we capture std::shared_ptr<Connection> as copy so
increment shared pointer's reference so as to guarantee validity of
connection. Catch exceptions when using connection in process of
onMessageProcess because the connection could be already closed by the
peer.

Change-Id: I391bae78cf663b875b96124f8356d27a38a97dc6
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>
src/framework/common/service.cpp
src/framework/common/service.h
src/framework/service/server-service.cpp

index a23aa01..2ab7530 100644 (file)
@@ -81,7 +81,7 @@ void Service::onNewConnection(ConnShPtr &&connection)
 
        this->m_loop.addEventSource(fd, EPOLLIN | EPOLLHUP | EPOLLRDHUP,
        [&, fd](uint32_t event) {
-               std::unique_lock<std::mutex> lock(this->m_crMtx);
+               std::lock_guard<std::mutex> lock(this->m_crMtx);
 
                DEBUG("read event comes in to fd[" << fd << "]");
 
@@ -97,17 +97,13 @@ void Service::onNewConnection(ConnShPtr &&connection)
                        return;
                }
 
-               lock.unlock();
-
                DEBUG("Start message process on fd[" << fd << "]");
 
                onMessageProcess(conn);
        });
 
-       {
-               std::lock_guard<std::mutex> l(this->m_crMtx);
-               this->m_connectionRegistry[fd] = std::move(connection);
-       }
+       std::lock_guard<std::mutex> lock(this->m_crMtx);
+       this->m_connectionRegistry[fd] = std::move(connection);
 }
 
 void Service::onCloseConnection(const ConnShPtr &connection)
@@ -128,11 +124,4 @@ void Service::onCloseConnection(const ConnShPtr &connection)
        this->m_connectionRegistry.erase(fd);
 }
 
-bool Service::isConnectionValid(int fd) const
-{
-       std::lock_guard<std::mutex> l(this->m_crMtx);
-
-       return this->m_connectionRegistry.count(fd) != 0;
-}
-
 }
index a2d054b..7025d8f 100644 (file)
@@ -48,7 +48,6 @@ public:
 
 protected:
        void setIdleChecker(std::function<bool()> &&idleChecker);
-       bool isConnectionValid(int fd) const;
 
 private:
        virtual void onMessageProcess(const ConnShPtr &) = 0;
index 6f22cfd..cfdb568 100644 (file)
@@ -378,19 +378,21 @@ void ServerService::onMessageProcess(const ConnShPtr &connection)
 
        auto inbufPtr = std::make_shared<RawBuffer>(connection->receive());
 
-       auto fd = connection->getFd();
-
-       this->m_workqueue.submit([this, &connection, fd, process, inbufPtr]() {
-               auto outbuf = (*process)(connection, *inbufPtr);
-
-               CpuUsageManager::reset();
-
-               if (!this->isConnectionValid(fd)) {
-                       ERROR("Connection for fd[] is closed while task is in processing...");
-                       return;
+       this->m_workqueue.submit([this, connection, process, inbufPtr]() {
+               try {
+                       auto outbuf = (*process)(connection, *inbufPtr);
+
+                       CpuUsageManager::reset();
+
+                       connection->send(outbuf);
+               } catch (const std::exception &e) {
+                       ERROR("exception on workqueue task: " << e.what());
+                       try {
+                               connection->send(BinaryQueue::Serialize(CSR_ERROR_SYSTEM).pop());
+                       } catch (const std::exception &e) {
+                               ERROR("The connection is abnormally closed by the peer: " << e.what());
+                       }
                }
-
-               connection->send(outbuf);
        });
 }