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 9787f96..a6c8e2e 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 e624660..1fb2285 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 bd03cac..771a303 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 2ed8c89..1ccd5e5 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 1ffc2e8..5a9d014 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);
 }