Check & Clean all TODOs.

Socket::create added to divided domain socket creation and client socket
connection easily.

Mutex locks in mainloop and service are not needed because callback
add/erase are done by single thread (main thread) only.

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

diff --git a/src/framework/client/handle-ext.cpp b/src/framework/client/handle-ext.cpp
index 3a7b065..9d1aea1 100644 (file)
--- a/src/framework/client/handle-ext.cpp
+++ b/src/framework/client/handle-ext.cpp
@@ -79,7 +79,6 @@ void HandleExt::dispatchAsync(const std::shared_ptr<Task> &f)
        this->m_isRunning = true;
        this->m_stop = false;
 
-       // TODO: how to handle exceptions in workers
        this->m_worker = std::thread([this, f] {
                DEBUG("client async thread dispatched! tid: " << std::this_thread::get_id());
 

diff --git a/src/framework/common/dispatcher.cpp b/src/framework/common/dispatcher.cpp
index 019edf7..ea15f9c 100644 (file)
--- a/src/framework/common/dispatcher.cpp
+++ b/src/framework/common/dispatcher.cpp
@@ -33,7 +33,8 @@ Dispatcher::Dispatcher(SockId sockId) noexcept : m_sockId(sockId)
 
 void Dispatcher::connect()
 {
-       this->m_connection = std::make_shared<Connection>(Socket::connect(this->m_sockId));
+       this->m_connection = std::make_shared<Connection>(
+                       Socket::create(this->m_sockId, Socket::Type::CLIENT));
 }
 
 bool Dispatcher::isConnected() const noexcept

diff --git a/src/framework/common/dispatcher.h b/src/framework/common/dispatcher.h
index 716e4e0..211d3e2 100644 (file)
--- a/src/framework/common/dispatcher.h
+++ b/src/framework/common/dispatcher.h
@@ -53,13 +53,13 @@ private:
 template<typename Type, typename ...Args>
 Type Dispatcher::methodCall(Args &&...args)
 {
-       if (!isConnected())
-               connect();
+       if (!this->isConnected())
+               this->connect();
 
-       m_connection->send(BinaryQueue::Serialize(std::forward<Args>(args)...).pop());
+       this->m_connection->send(BinaryQueue::Serialize(std::forward<Args>(args)...).pop());
 
        BinaryQueue q;
-       q.push(m_connection->receive());
+       q.push(this->m_connection->receive());
 
        Type response;
        q.Deserialize(response);

diff --git a/src/framework/common/mainloop.cpp b/src/framework/common/mainloop.cpp
index 02875af..3316c0a 100644 (file)
--- a/src/framework/common/mainloop.cpp
+++ b/src/framework/common/mainloop.cpp
@@ -61,7 +61,6 @@ void Mainloop::run(int timeout)
 
 void Mainloop::addEventSource(int fd, uint32_t event, Callback &&callback)
 {
-       /* TODO: use scoped-lock to thread safe on member variables */
        if (this->m_callbacks.count(fd) != 0)
                ThrowExc(CSR_ERROR_SERVER, "event source on fd[" << fd << "] already added!");
 
@@ -82,7 +81,6 @@ void Mainloop::addEventSource(int fd, uint32_t event, Callback &&callback)
 
 void Mainloop::removeEventSource(int fd)
 {
-       /* TODO: use scoped-lock to thread safe on member variables */
        if (this->m_callbacks.count(fd) == 0)
                ThrowExc(CSR_ERROR_SERVER, "event source on fd[" << fd << "] isn't added at all");
 
@@ -134,7 +132,6 @@ void Mainloop::dispatch(int timeout)
        }
 
        for (int i = 0; i < nfds; i++) {
-               /* TODO: use scoped-lock to thread safe on member variables */
                int fd = event[i].data.fd;
 
                if (this->m_callbacks.count(fd) == 0)

diff --git a/src/framework/common/service.cpp b/src/framework/common/service.cpp
index c95234f..de5399e 100644 (file)
--- a/src/framework/common/service.cpp
+++ b/src/framework/common/service.cpp
@@ -54,7 +54,7 @@ void Service::start(int timeout)
        INFO("Service start!");
 
        for (const auto &id : this->m_sockIds) {
-               auto socket = std::make_shared<Socket>(id);
+               auto socket = std::make_shared<Socket>(Socket::create(id, Socket::Type::SERVER));
 
                DEBUG("Get systemd socket[" << socket->getFd() <<
                          "] for sock id: " << static_cast<int>(id));
@@ -71,10 +71,9 @@ void Service::start(int timeout)
        this->m_loop.run(timeout);
 }
 
-void Service::setNewConnectionCallback(const ConnCallback &/*callback*/)
+void Service::setNewConnectionCallback(const ConnCallback &callback)
 {
-       /* TODO: scoped-lock */
-       this->m_onNewConnection = [&](const ConnShPtr & connection) {
+       this->m_onNewConnection = [this, &callback](const ConnShPtr &connection) {
                if (!connection)
                        ThrowExc(CSR_ERROR_SERVER, "onNewConnection called but ConnShPtr is nullptr.");
 
@@ -82,14 +81,11 @@ void Service::setNewConnectionCallback(const ConnCallback &/*callback*/)
 
                INFO("welcome! accepted client socket fd[" << fd << "]");
 
-               /*
-                   // TODO: disable temporarily
-                   if (callback)
-                   callback(connection);
-               */
+               if (callback)
+                       callback(connection);
 
                this->m_loop.addEventSource(fd, EPOLLIN | EPOLLHUP | EPOLLRDHUP,
-               [ &, fd](uint32_t event) {
+               [&, fd](uint32_t event) {
                        DEBUG("read event comes in to fd[" << fd << "]");
 
                        if (this->m_connectionRegistry.count(fd) == 0)
@@ -113,10 +109,9 @@ void Service::setNewConnectionCallback(const ConnCallback &/*callback*/)
        };
 }
 
-void Service::setCloseConnectionCallback(const ConnCallback &/*callback*/)
+void Service::setCloseConnectionCallback(const ConnCallback &callback)
 {
-       /* TODO: scoped-lock */
-       this->m_onCloseConnection = [&](const ConnShPtr & connection) {
+       this->m_onCloseConnection = [this, &callback](const ConnShPtr &connection) {
                if (!connection)
                        ThrowExc(CSR_ERROR_SERVER, "no connection to close");
 
@@ -129,13 +124,10 @@ void Service::setCloseConnectionCallback(const ConnCallback &/*callback*/)
                INFO("good-bye! close socket fd[" << fd << "]");
 
                this->m_loop.removeEventSource(fd);
-               this->m_connectionRegistry.erase(fd); /* scoped-lock needed? */
+               this->m_connectionRegistry.erase(fd);
 
-               /*
-                   // TODO: disable temporarily
-                   if (callback)
-                       callback(connection);
-               */
+               if (callback)
+                       callback(connection);
        };
 }
 

diff --git a/src/framework/common/socket.cpp b/src/framework/common/socket.cpp
index b582797..d60e1e2 100644 (file)
--- a/src/framework/common/socket.cpp
+++ b/src/framework/common/socket.cpp
@@ -57,6 +57,18 @@ int createSystemdSocket(const std::string &path)
 
 } // namespace anonymous
 
+Socket Socket::create(SockId sockId, Socket::Type type)
+{
+       switch (type) {
+       case Socket::Type::SERVER:
+               return Socket(sockId);
+       case Socket::Type::CLIENT:
+               return Socket::connect(sockId);
+       default:
+               ThrowExc(CSR_ERROR_SOCKET, "Invalid type to Socket::create");
+       }
+}
+
 Socket::Socket(SockId sockId, int fd) : m_sockId(sockId), m_fd(fd)
 {
        if (this->m_fd < 0)

diff --git a/src/framework/common/socket.h b/src/framework/common/socket.h
index a896307..c902902 100644 (file)
--- a/src/framework/common/socket.h
+++ b/src/framework/common/socket.h
@@ -31,11 +31,12 @@ namespace Csr {
 
 class API Socket {
 public:
-       // Socket with accepted / connected
-       Socket(SockId sockId, int fd);
+       enum class Type : int {
+               SERVER = 0x01,
+               CLIENT = 0x02
+       };
 
-       // Create systemd socket
-       Socket(SockId sockId);
+       static Socket create(SockId, Socket::Type);
 
        Socket(const Socket &) = delete;
        Socket &operator=(const Socket &) = delete;
@@ -53,10 +54,11 @@ public:
        RawBuffer read(void) const;
        void write(const RawBuffer &data) const;
 
-       /* TODO: can it be constructor? */
+private:
        static Socket connect(SockId);
+       Socket(SockId sockId, int fd);
+       Socket(SockId sockId);
 
-private:
        SockId m_sockId;
        int m_fd;
 };