Check connection validation before write response

If client disconnect connection while server is handling the client's
request, server could write response to invalid connection by
connection->send() and crash occured. To prevent it, (client's abnormal
operation affect server) connection validation check is performed before
write on it.

Change-Id: I4c02fb0b40a6fd3a50e715a619334a3e290957f0
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>

diff --git a/src/framework/common/service.cpp b/src/framework/common/service.cpp
index de5399e..2997dcf 100644 (file)
--- a/src/framework/common/service.cpp
+++ b/src/framework/common/service.cpp
@@ -73,11 +73,11 @@ void Service::start(int timeout)
 
 void Service::setNewConnectionCallback(const ConnCallback &callback)
 {
-       this->m_onNewConnection = [this, &callback](const ConnShPtr &connection) {
+       this->m_onNewConnection = [this, &callback](ConnShPtr &&connection) {
                if (!connection)
                        ThrowExc(CSR_ERROR_SERVER, "onNewConnection called but ConnShPtr is nullptr.");
 
-               int fd = connection->getFd();
+               auto fd = connection->getFd();
 
                INFO("welcome! accepted client socket fd[" << fd << "]");
 
@@ -86,6 +86,8 @@ void Service::setNewConnectionCallback(const ConnCallback &callback)
 
                this->m_loop.addEventSource(fd, EPOLLIN | EPOLLHUP | EPOLLRDHUP,
                [&, fd](uint32_t event) {
+                       std::unique_lock<std::mutex> lock(this->m_crMtx);
+
                        DEBUG("read event comes in to fd[" << fd << "]");
 
                        if (this->m_connectionRegistry.count(fd) == 0)
@@ -100,12 +102,17 @@ void Service::setNewConnectionCallback(const ConnCallback &callback)
                                return;
                        }
 
+                       lock.unlock();
+
                        DEBUG("Start message process on fd[" << fd << "]");
 
                        onMessageProcess(conn);
                });
 
-               this->m_connectionRegistry[fd] = connection;
+               {
+                       std::lock_guard<std::mutex> l(this->m_crMtx);
+                       this->m_connectionRegistry[fd] = std::move(connection);
+               }
        };
 }
 
@@ -115,7 +122,7 @@ void Service::setCloseConnectionCallback(const ConnCallback &callback)
                if (!connection)
                        ThrowExc(CSR_ERROR_SERVER, "no connection to close");
 
-               int fd = connection->getFd();
+               auto fd = connection->getFd();
 
                if (this->m_connectionRegistry.count(fd) == 0)
                        ThrowExc(CSR_ERROR_SERVER, "no connection in registry to remove "
@@ -124,6 +131,7 @@ void Service::setCloseConnectionCallback(const ConnCallback &callback)
                INFO("good-bye! close socket fd[" << fd << "]");
 
                this->m_loop.removeEventSource(fd);
+
                this->m_connectionRegistry.erase(fd);
 
                if (callback)
@@ -131,4 +139,11 @@ void Service::setCloseConnectionCallback(const ConnCallback &callback)
        };
 }
 
+bool Service::isConnectionValid(int fd) const
+{
+       std::lock_guard<std::mutex> l(this->m_crMtx);
+
+       return this->m_connectionRegistry.count(fd) != 0;
+}
+
 }

diff --git a/src/framework/common/service.h b/src/framework/common/service.h
index c405285..bdcd0c3 100644 (file)
--- a/src/framework/common/service.h
+++ b/src/framework/common/service.h
@@ -24,6 +24,7 @@
 #include <string>
 #include <functional>
 #include <set>
+#include <mutex>
 
 #include "common/macros.h"
 #include "common/connection.h"
@@ -53,13 +54,15 @@ public:
 
 protected:
        void setIdleChecker(std::function<bool()> &&idleChecker);
+       bool isConnectionValid(int fd) const;
 
 private:
        virtual void onMessageProcess(const ConnShPtr &) = 0;
 
-       ConnCallback m_onNewConnection;
+       std::function<void(ConnShPtr &&)> m_onNewConnection;
        ConnCallback m_onCloseConnection;
 
+       mutable std::mutex m_crMtx;
        std::unordered_map<int, ConnShPtr> m_connectionRegistry;
        Mainloop m_loop;
 

diff --git a/src/framework/service/server-service.cpp b/src/framework/service/server-service.cpp
index 0a5a87d..6f22cfd 100644 (file)
--- a/src/framework/service/server-service.cpp
+++ b/src/framework/service/server-service.cpp
@@ -378,12 +378,19 @@ void ServerService::onMessageProcess(const ConnShPtr &connection)
 
        auto inbufPtr = std::make_shared<RawBuffer>(connection->receive());
 
-       this->m_workqueue.submit([this, &connection, process, inbufPtr]() {
-               auto outbuf = (*process)(connection, *inbufPtr);
+       auto fd = connection->getFd();
 
-               connection->send(outbuf);
+       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;
+               }
+
+               connection->send(outbuf);
        });
 }