Fix DebugPort implementation 36/281336/2
authorHwankyu Jhun <h.jhun@samsung.com>
Thu, 15 Sep 2022 10:42:36 +0000 (10:42 +0000)
committerHwanKyu Jhun <h.jhun@samsung.com>
Thu, 15 Sep 2022 23:12:16 +0000 (23:12 +0000)
The DebugPort implementation is changed. The method are changed to 'static'.
The implementation of the DebugPort is moved to the DebugPortImpl.
To avoid unnecessary locking mutex, the flag that is connected_ is changed to
std::atomic<bool>.

Change-Id: I4aca67730639664c7a2646f9486be49124dad0ff
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>
src/debug-port-internal.cc
src/debug-port-internal.hh
src/proxy-internal.cc
src/rpc-port.cc
src/stub-internal.cc

index 9787f962bf81a47e32ebeb872b8ab67d9b30821d..a6c8e2eeefb1ff69e6ceda478817c42b4b43985a 100644 (file)
 
 #include "debug-port-internal.hh"
 
+#include <aul_app_com.h>
 #include <bundle_internal.h>
+#include <gio/gio.h>
+#include <glib.h>
 #include <parcel.hh>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <sys/un.h>
 #include <time.h>
 
+#include <atomic>
+#include <list>
+#include <memory>
+#include <mutex>
+#include <thread>
 #include <utility>
 
 #include "log-private.hh"
-
-#undef RPC_DTOR
-#define RPC_DTOR __attribute__ ((destructor))
+#include "port-internal.hh"
+#include "shared-queue-internal.hh"
 
 namespace rpc_port {
 namespace internal {
-
 namespace {
 
 constexpr const char PATH_RPC_PORT_UTIL_SOCK[] =
@@ -40,43 +46,94 @@ constexpr const char PATH_RPC_PORT_UTIL_SOCK[] =
 constexpr const char ENDPOINT_RPC_PORT_DEBUG[] = "org.tizen.rpcport.debug";
 constexpr const char KEY_PORT_NAME[] = "__K_PORT_NAME__";
 
-RPC_DTOR void DebugPortDtor() {
-  DebugPort::GetInst()->Dispose(false);
-}
-
-}  // namespace
+class Session {
+ public:
+  Session(std::string port_name, std::string destination,
+      int main_port, int delegate_port)
+      : port_name_(std::move(port_name)),
+        destination_(std::move(destination)),
+        main_port_(main_port),
+        delegate_port_(delegate_port) {
+  }
 
-std::atomic<DebugPort*> DebugPort::inst_;
-std::mutex DebugPort::mutex_;
+  const std::string& GetPortName() const {
+    return port_name_;
+  }
 
-DebugPort::~DebugPort() {
-  Dispose();
-}
+  const std::string& GetDestination() const {
+    return destination_;
+  }
 
-DebugPort* DebugPort::GetInst() {
-  DebugPort* inst = inst_.load(std::memory_order_acquire);
-  if (inst == nullptr) {
-    std::lock_guard<std::mutex> lock(mutex_);
-    inst = inst_.load(std::memory_order_relaxed);
-    if (inst == nullptr) {
-      inst = new DebugPort();
-      inst_.store(inst, std::memory_order_release);
-    }
+  int GetMainPort() const {
+    return main_port_;
   }
 
-  std::lock_guard<std::recursive_mutex> rec_lock(inst->GetMutex());
-  if (inst->disposed_)
-    inst->Init();
+  int GetDelegatePort() const {
+    return delegate_port_;
+  }
 
-  return inst;
+ private:
+  std::string port_name_;
+  std::string destination_;
+  int main_port_;
+  int delegate_port_;
+};
+
+class DebugPortImpl {
+ public:
+  DebugPortImpl() = default;
+  ~DebugPortImpl();
+
+  void Dispose();
+  bool IsConnected() const;
+  void AddSession(std::string port_name, std::string destination,
+      int main_port, int delegate_port);
+  void RemoveSession(int port);
+  int Send(int port, bool is_read, uint32_t seq,
+      const void* buf, unsigned int size);
+  void Init();
+
+ private:
+  int Connect();
+  int Watch(int fd);
+  void Unwatch();
+  void SetConnectionStatus(bool status);
+  void CreateThread();
+  void JoinThread();
+
+  std::recursive_mutex& GetMutex() const;
+  std::shared_ptr<Session> FindSession(int port);
+  std::shared_ptr<Session> FindSession(const std::string& port_name);
+
+  static gboolean OnDebugPortDisconnectedCb(GIOChannel* io,
+      GIOCondition cond, gpointer data);
+  static int AppComCb(const char* endpoint, aul_app_com_result_e result,
+      bundle* envelope, void* user_data);
+
+ private:
+  bool disposed_ = true;
+  std::atomic<bool> connected_;
+  std::unique_ptr<Port> port_;
+  GIOChannel* io_ = nullptr;
+  guint watch_tag_ = 0;
+  std::list<std::shared_ptr<Session>> sessions_;
+  std::thread thread_;
+  std::atomic<bool> is_running_;
+  SharedQueue<std::shared_ptr<tizen_base::Parcel>> queue_;
+  mutable std::recursive_mutex mutex_;
+  aul_app_com_connection_h conn_ = nullptr;
+};
+
+DebugPortImpl::~DebugPortImpl() {
+  Dispose();
 }
 
-void DebugPort::Dispose(bool do_leave) {
+void DebugPortImpl::Dispose() {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   if (disposed_)
     return;
 
-  if (do_leave && conn_) {
+  if (conn_) {
     aul_app_com_leave(conn_);
     conn_ = nullptr;
   }
@@ -86,23 +143,22 @@ void DebugPort::Dispose(bool do_leave) {
   disposed_ = true;
 }
 
-bool DebugPort::IsConnected() {
-  std::lock_guard<std::recursive_mutex> lock(GetMutex());
+bool DebugPortImpl::IsConnected() const {
   return connected_;
 }
 
-void DebugPort::AddSession(std::string port_name, std::string destination,
+void DebugPortImpl::AddSession(std::string port_name, std::string destination,
     int main_port, int delegate_port) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   sessions_.emplace_back(
-      new DebugPort::Session(std::move(port_name), std::move(destination),
-      main_port, delegate_port));
+      new Session(std::move(port_name), std::move(destination),
+        main_port, delegate_port));
 }
 
-void DebugPort::RemoveSession(int port) {
+void DebugPortImpl::RemoveSession(int port) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   auto iter = std::find_if(sessions_.begin(), sessions_.end(),
-      [port](std::shared_ptr<DebugPort::Session>& sess) -> bool {
+      [port](std::shared_ptr<Session>& sess) -> bool {
         return sess->GetMainPort() == port || sess->GetDelegatePort() == port;
       });
 
@@ -112,7 +168,7 @@ void DebugPort::RemoveSession(int port) {
   }
 }
 
-std::shared_ptr<DebugPort::Session> DebugPort::FindSession(int port) {
+std::shared_ptr<Session> DebugPortImpl::FindSession(int port) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   for (auto& s : sessions_) {
     if (s->GetMainPort() == port || s->GetDelegatePort() == port)
@@ -122,7 +178,7 @@ std::shared_ptr<DebugPort::Session> DebugPort::FindSession(int port) {
   return nullptr;
 }
 
-std::shared_ptr<DebugPort::Session> DebugPort::FindSession(
+std::shared_ptr<Session> DebugPortImpl::FindSession(
     const std::string& port_name) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   for (auto& s : sessions_) {
@@ -133,9 +189,8 @@ std::shared_ptr<DebugPort::Session> DebugPort::FindSession(
   return nullptr;
 }
 
-int DebugPort::Send(int port, bool is_read, uint32_t seq,
+int DebugPortImpl::Send(int port, bool is_read, uint32_t seq,
     const void* buf, unsigned int size) {
-  std::lock_guard<std::recursive_mutex> lock(GetMutex());
   if (!IsConnected())
     return 0;
 
@@ -161,7 +216,11 @@ int DebugPort::Send(int port, bool is_read, uint32_t seq,
   return 0;
 }
 
-void DebugPort::Init() {
+void DebugPortImpl::Init() {
+  if (!disposed_)
+    return;
+
+  std::lock_guard<std::recursive_mutex> lock(GetMutex());
   aul_app_com_create_async(ENDPOINT_RPC_PORT_DEBUG, nullptr, AppComCb, this,
       &conn_);
   if (conn_ == nullptr)
@@ -184,7 +243,7 @@ void DebugPort::Init() {
   disposed_ = false;
 }
 
-int DebugPort::Connect() {
+int DebugPortImpl::Connect() {
   if (access(PATH_RPC_PORT_UTIL_SOCK, F_OK) != 0)
     return -1;
 
@@ -209,7 +268,7 @@ int DebugPort::Connect() {
   return fd;
 }
 
-int DebugPort::Watch(int fd) {
+int DebugPortImpl::Watch(int fd) {
   GIOChannel* io = g_io_channel_unix_new(fd);
   if (io == nullptr) {
     _E("g_io_channel_unix_new() is failed");
@@ -230,7 +289,7 @@ int DebugPort::Watch(int fd) {
   return 0;
 }
 
-void DebugPort::Unwatch() {
+void DebugPortImpl::Unwatch() {
   if (io_) {
     g_io_channel_unref(io_);
     io_ = nullptr;
@@ -242,10 +301,10 @@ void DebugPort::Unwatch() {
   }
 }
 
-gboolean DebugPort::OnDebugPortDisconnectedCb(GIOChannel* io,
+gboolean DebugPortImpl::OnDebugPortDisconnectedCb(GIOChannel* io,
     GIOCondition cond, gpointer data) {
   _W("cond(%d)", static_cast<int>(cond));
-  auto* debug_port = static_cast<DebugPort*>(data);
+  auto* debug_port = static_cast<DebugPortImpl*>(data);
   std::lock_guard<std::recursive_mutex> lock(debug_port->GetMutex());
   debug_port->SetConnectionStatus(false);
   debug_port->watch_tag_ = 0;
@@ -255,12 +314,11 @@ gboolean DebugPort::OnDebugPortDisconnectedCb(GIOChannel* io,
   return G_SOURCE_REMOVE;
 }
 
-void DebugPort::SetConnectionStatus(bool status) {
-  std::lock_guard<std::recursive_mutex> lock(GetMutex());
-  connected_ = status;
+void DebugPortImpl::SetConnectionStatus(bool status) {
+  connected_.exchange(status);
 }
 
-void DebugPort::CreateThread() {
+void DebugPortImpl::CreateThread() {
   if (is_running_)
     return;
 
@@ -297,7 +355,7 @@ void DebugPort::CreateThread() {
   is_running_ = true;
 }
 
-void DebugPort::JoinThread() {
+void DebugPortImpl::JoinThread() {
   if (is_running_)
     queue_.Push(std::shared_ptr<tizen_base::Parcel>(new tizen_base::Parcel()));
 
@@ -307,16 +365,21 @@ void DebugPort::JoinThread() {
   }
 }
 
-int DebugPort::AppComCb(const char* endpoint, aul_app_com_result_e result,
+
+std::recursive_mutex& DebugPortImpl::GetMutex() const {
+  return mutex_;
+}
+
+int DebugPortImpl::AppComCb(const char* endpoint, aul_app_com_result_e result,
     bundle* envelope, void* user_data) {
   const char* val = bundle_get_val(envelope, KEY_PORT_NAME);
   if (val == nullptr)
     return -1;
 
-  auto* handle = static_cast<DebugPort*>(user_data);
+  auto* handle = static_cast<DebugPortImpl*>(user_data);
   std::string port_name(val);
   if (port_name.empty() || handle->FindSession(port_name) != nullptr) {
-    auto* handle = static_cast<DebugPort*>(user_data);
+    auto* handle = static_cast<DebugPortImpl*>(user_data);
     int fd = handle->Connect();
     if (fd < 0)
       return -1;
@@ -335,5 +398,32 @@ int DebugPort::AppComCb(const char* endpoint, aul_app_com_result_e result,
   return 0;
 }
 
+DebugPortImpl impl;
+
+}  // namespace
+
+bool DebugPort::IsConnected() {
+  impl.Init();
+  return impl.IsConnected();
+}
+
+void DebugPort::AddSession(std::string port_name, std::string destination,
+    int main_port, int delegate_port) {
+  impl.Init();
+  return impl.AddSession(std::move(port_name), std::move(destination),
+      main_port, delegate_port);
+}
+
+void DebugPort::RemoveSession(int port) {
+  impl.Init();
+  return impl.RemoveSession(port);
+}
+
+int DebugPort::Send(int port, bool is_read, uint32_t seq, const void* buf,
+    unsigned int size) {
+  impl.Init();
+  return impl.Send(port, is_read, seq, buf, size);
+}
+
 }  // namespace internal
 }  // namespace rpc_port
index e6246602e55d532e90838fe713de909e5d12dd80..1fb2285fcfef8679378c8fa7c298e2b5f90ee782 100644 (file)
 #ifndef DEBUG_PORT_INTERNAL_HH_
 #define DEBUG_PORT_INTERNAL_HH_
 
-#include <aul_app_com.h>
-#include <gio/gio.h>
-#include <glib.h>
-#include <parcel.hh>
-
-#include <atomic>
-#include <list>
-#include <memory>
-#include <mutex>
 #include <string>
-#include <thread>
-#include <utility>
-
-#include "port-internal.hh"
-#include "shared-queue-internal.hh"
 
 namespace rpc_port {
 namespace internal {
 
 class DebugPort {
- private:
-  DebugPort() = default;
-  ~DebugPort();
-
  public:
-  class Session {
-   public:
-    Session(std::string port_name, std::string destination,
-         int main_port, int delegate_port)
-      : port_name_(std::move(port_name)),
-        destination_(std::move(destination)),
-        main_port_(main_port),
-        delegate_port_(delegate_port) {
-    }
-    virtual ~Session() = default;
-
-    const std::string& GetPortName() {
-      return port_name_;
-    }
-
-    const std::string& GetDestination() {
-      return destination_;
-    }
-
-    int GetMainPort() {
-      return main_port_;
-    }
-
-    int GetDelegatePort() {
-      return delegate_port_;
-    }
-
-   private:
-    std::string port_name_;
-    std::string destination_;
-    int main_port_;
-    int delegate_port_;
-  };
-
-  static DebugPort* GetInst();
-
-  void Dispose(bool do_leave = true);
-  bool IsConnected();
-
-  void AddSession(std::string port_name, std::string destination,
+  static bool IsConnected();
+  static void AddSession(std::string port_name, std::string destination,
       int main_port, int delegate_port);
-  void RemoveSession(int port);
-
-  int Send(int port, bool is_read, uint32_t seq,
+  static void RemoveSession(int port);
+  static int Send(int port, bool is_read, uint32_t seq,
       const void* buf, unsigned int size);
-
- private:
-  static std::atomic<DebugPort*> inst_;
-  static std::mutex mutex_;
-
-  std::recursive_mutex& GetMutex() const {
-    return rec_mutex_;
-  }
-
-  void Init();
-  int Connect();
-  int Watch(int fd);
-  void Unwatch();
-  void SetConnectionStatus(bool status);
-  void CreateThread();
-  void JoinThread();
-
-  std::shared_ptr<DebugPort::Session> FindSession(int port);
-  std::shared_ptr<DebugPort::Session> FindSession(const std::string& port_name);
-
-  static gboolean OnDebugPortDisconnectedCb(GIOChannel* io,
-      GIOCondition cond, gpointer data);
-  static int AppComCb(const char* endpoint, aul_app_com_result_e result,
-      bundle* envelope, void* user_data);
-
- private:
-  bool disposed_ = true;
-  bool connected_ = false;
-  std::unique_ptr<Port> port_;
-  GIOChannel* io_ = nullptr;
-  guint watch_tag_ = 0;
-  std::list<std::shared_ptr<DebugPort::Session>> sessions_;
-  std::thread thread_;
-  std::atomic<bool> is_running_;
-  SharedQueue<std::shared_ptr<tizen_base::Parcel>> queue_;
-  mutable std::recursive_mutex rec_mutex_;
-  aul_app_com_connection_h conn_ = nullptr;
 };
 
 }  // namespace internal
index bd03cacf22b659e8683b99fc0d2ccdb571630fc4..771a303ae7d205a7c205d5786fe7b0b005708c21 100644 (file)
@@ -106,7 +106,7 @@ Proxy::~Proxy() {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   _D("Proxy::~Proxy()");
   if (main_port_.get() != nullptr)
-    DebugPort::GetInst()->RemoveSession(main_port_->GetFd());
+    DebugPort::RemoveSession(main_port_->GetFd());
 
   listener_ = nullptr;
   UnsetIdler();
@@ -267,7 +267,7 @@ int Proxy::ConnectSync(std::string appid, std::string port_name,
 
   main_port_.reset(new ProxyPort(this, fds_[0], target_appid_, false));
   delegate_port_.reset(new ProxyPort(this, fds_[1], target_appid_));
-  DebugPort::GetInst()->AddSession(port_name, target_appid_, fds_[0], fds_[1]);
+  DebugPort::AddSession(port_name, target_appid_, fds_[0], fds_[1]);
   listener_->OnConnected(target_appid_, main_port_.get());
   return RPC_PORT_ERROR_NONE;
 }
@@ -275,7 +275,7 @@ int Proxy::ConnectSync(std::string appid, std::string port_name,
 void Proxy::DisconnectPort() {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   if (main_port_.get() != nullptr) {
-    DebugPort::GetInst()->RemoveSession(main_port_->GetFd());
+    DebugPort::RemoveSession(main_port_->GetFd());
     main_port_.reset();
   }
 }
@@ -556,7 +556,7 @@ gboolean Proxy::ProxyPort::OnSocketDisconnected(GIOChannel* channel,
   proxy->delegate_port_.reset();
   proxy->listener_ = nullptr;
   listener->OnDisconnected(proxy->target_appid_);
-  DebugPort::GetInst()->RemoveSession(fd);
+  DebugPort::RemoveSession(fd);
   return G_SOURCE_REMOVE;;
 }
 
@@ -578,7 +578,7 @@ gboolean Proxy::ProxyPort::OnDataReceived(GIOChannel* channel,
       proxy->listener_ = nullptr;
       proxy->delegate_port_->SetSource(0);
       if (proxy->main_port_.get() != nullptr) {
-        DebugPort::GetInst()->RemoveSession(proxy->main_port_->GetFd());
+        DebugPort::RemoveSession(proxy->main_port_->GetFd());
         proxy->main_port_.reset();
       }
       proxy->delegate_port_.reset();
@@ -763,7 +763,7 @@ gboolean Proxy::Client::OnResponseReceived(GIOChannel* channel,
     proxy->fds_[1] = client_fd;
     proxy->delegate_port_.reset(
         new ProxyPort(proxy, proxy->fds_[1], proxy->target_appid_));
-    DebugPort::GetInst()->AddSession(proxy->port_name_, proxy->target_appid_,
+    DebugPort::AddSession(proxy->port_name_, proxy->target_appid_,
         proxy->fds_[0], proxy->fds_[1]);
     listener->OnConnected(proxy->target_appid_, proxy->main_port_.get());
     _W("target_appid(%s), port_name(%s), main_fd(%d), delegate_fd(%d)",
index 2ed8c892b608804b32da1608191340ad2ea69fb2..1ccd5e533055eb76e2ed4717390cf8a4a7e50fe5 100644 (file)
@@ -241,8 +241,7 @@ RPC_API int rpc_port_read(rpc_port_h h, void* buf, unsigned int size) {
     return ret;
   }
 
-  auto* debug_port = DebugPort::GetInst();
-  debug_port->Send(port->GetFd(), true, seq, buf, size);
+  DebugPort::Send(port->GetFd(), true, seq, buf, size);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -260,8 +259,7 @@ RPC_API int rpc_port_write(rpc_port_h h, const void* buf, unsigned int size) {
   if (ret < 0)
     return ret;
 
-  auto* debug_port = DebugPort::GetInst();
-  debug_port->Send(port->GetFd(), false, seq, buf, size);
+  DebugPort::Send(port->GetFd(), false, seq, buf, size);
   return RPC_PORT_ERROR_NONE;
 }
 
index 1ffc2e8df3ca976aa9d1d7d2be202f9ceb649b77..5a9d01413b3be5e4b87c660e5872936b9e8724dd 100644 (file)
@@ -97,7 +97,7 @@ Stub::~Stub() {
   _D("Stub::~Stub");
   for (auto& p : ports_) {
     if (!p->IsDelegate())
-      DebugPort::GetInst()->RemoveSession(p->GetFd());
+      DebugPort::RemoveSession(p->GetFd());
   }
 
   listener_ = nullptr;
@@ -167,7 +167,7 @@ void Stub::RemoveAcceptedPorts(std::string instance) {
   while (iter != ports_.end()) {
     if ((*iter)->GetInstance().compare(instance) == 0) {
       LOGI("Close: fd(%d)", (*iter)->GetFd());
-      DebugPort::GetInst()->RemoveSession((*iter)->GetFd());
+      DebugPort::RemoveSession((*iter)->GetFd());
       iter = ports_.erase(iter);
     } else {
       iter++;
@@ -264,7 +264,7 @@ void Stub::AddAcceptedPort(const std::string& sender_appid,
 
   _W("sender_appid(%s), instance(%s), main_fd(%d), delegate_fd(%d)",
       sender_appid.c_str(), instance.c_str(), main_fd, fd);
-  DebugPort::GetInst()->AddSession(port_name_, sender_appid, main_fd, fd);
+  DebugPort::AddSession(port_name_, sender_appid, main_fd, fd);
   listener_->OnConnected(sender_appid, instance);
 }