Fix deadlock issue

When the rpc_port_proxy_connect() is called in the sub thread,
the port appeared event can be received in the main thread.
In this case, the process can be a deadlock state as below:
+------------------------------------------------------------------------------+
| -- #0  0xb5d3e6c8 in __lll_lock_wait () from /usr/lib/libpthread-2.30.so     |
| -- #1  0xb5d35de4 in __pthread_mutex_lock () from /usr/lib/libpthread-2.30.so|
| -- #2  0xa63ee315 in _ZN8rpc_port8internal5Proxy14OnPortAppearedEPKcS3_iPv ()|
|                      from /usr/lib/librpc-port.so.1.9.3		       |
| -- #3  0xb5f69721 in _ZN12_GLOBAL__N_19WatchInfo8AppComCbEPKc20aul_app_com_re|
|                      sult_eP9_bundle_tPv () from /usr/lib/libaul.so.0.38.0   |
| -- #4  0xb5f74d2d in app_com_recv () from /usr/lib/libaul.so.0.38.0          |
| -- #5  0xb5f74fe7 in __dispatch_request () from /usr/lib/libaul.so.0.38.0    |
| -- #6  0xb5b10fb3 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.|
                       0.6200.3                                                |
| -- #7  0xb58a01a9 in _ecore_glib_select.lto_priv.0 () from /usr/lib/libecore.|
|                      so.1.25.1                                               |
| -- #8  0xb589f037 in _ecore_main_select () from /usr/lib/libecore.so.1.25.1  |
| -- #9  0xb589f8c7 in _ecore_main_loop_iterate_internal () from /usr/lib/libec|
|                      ore.so.1.25.1                                           |
| -- [pthread wait detected] pthread_mutex_lock/pthread_join owner tid:1412    |
+------------------------------------------------------------------------------+
| -- [User Backtrace] [Thread state:S (sleeping), Frozen:No]                   |
| -- Pid: 683, Tid: 1412, comm: Parallel[0] exec_start[69.429195162]           |
| -- #0  0xb5d3e6c8 in __lll_lock_wait () from /usr/lib/libpthread-2.30.so     |
| -- #1  0xb5d35de4 in __pthread_mutex_lock () from /usr/lib/libpthread-2.30.so|
| -- #2  0xb5f789b5 in aul_app_com_leave () from /usr/lib/libaul.so.0.38.0     |
| -- #3  0xb5f6d197 in aul_rpc_port_remove_watch () from /usr/lib/libaul.so.0.3|
|                      8.0                                                     |
| -- #4  0xa63ec503 in _ZN8rpc_port8internal5Proxy6CancelEv () from /usr/lib/li|
|                      brpc-port.so.1.9.3                                      |
| -- #5  0xa63ee329 in _ZN8rpc_port8internal5Proxy14OnPortAppearedEPKcS3_iPv ()|
|                      from /usr/lib/librpc-port.so.1.9.3                      |
| -- #6  0xa63ee45b in _ZN8rpc_port8internal5Proxy5WatchEv () from /usr/lib/lib|
|                      rpc-port.so.1.9.3                                       |
| -- #7  0xa63ee58f in _ZN8rpc_port8internal5Proxy7ConnectENSt7__cxx1112basic_s|
|                      tringIcSt11char_traitsIcESaIcEEES7_PNS1_14IEventListener|
|                      E () from /usr/lib/librpc-port.so.1.9.3                 |
| -- #8  0xa63efd51 in rpc_port_proxy_connect () from /usr/lib/librpc-port.so.1|
|                      .9.3                                                    |
+------------------------------------------------------------------------------+

This patch fixes the proxy handle management using the shared_ptr. And,
the port check is moved to the main thread to avoid the deadlock issue.

Change-Id: I8ef7f775491427e48eb5894c66dc1629d172a4d5
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>

diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc
index c85f17a..f22c6ca 100644 (file)
--- a/src/proxy-internal.cc
+++ b/src/proxy-internal.cc
@@ -104,6 +104,7 @@ Proxy::~Proxy() {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   _D("Proxy::~Proxy()");
   listener_ = nullptr;
+  UnsetIdler();
   UnsetConnTimer();
   Cancel();
 }
@@ -284,13 +285,8 @@ int Proxy::Watch() {
     return -1;
   }
 
-  if (Aul::ExistPort(real_appid_, port_name_, rpc_port_get_target_uid())) {
-    OnPortAppeared(real_appid_.c_str(), port_name_.c_str(), -1, this);
-  } else {
-    OnPortVanished(real_appid_.c_str(), port_name_.c_str(), -1, this);
-    SetConnTimer();
-  }
-
+  SetConnTimer();
+  SetIdler();
   return 0;
 }
 
@@ -324,15 +320,59 @@ std::recursive_mutex& Proxy::GetMutex() const {
 }
 
 void Proxy::SetConnTimer() {
-  if (conn_timer_ == 0)
-    conn_timer_ = g_timeout_add_seconds(10, OnTimedOut, this);
+  if (conn_timer_data_) {
+    _W("Already exists");
+    return;
+  }
+
+  conn_timer_data_ = CreateWeakPtr();
+  if (conn_timer_data_ == nullptr) {
+    _E("Out of memory");
+    return;
+  }
+
+  g_timeout_add_seconds(10, OnTimedOut, conn_timer_data_);
 }
 
 void Proxy::UnsetConnTimer() {
-  if (conn_timer_) {
-    g_source_remove(conn_timer_);
-    conn_timer_ = 0;
+  if (conn_timer_data_ == nullptr)
+    return;
+
+  GSource* source = g_main_context_find_source_by_user_data(nullptr,
+      conn_timer_data_);
+  if (source && !g_source_is_destroyed(source))
+    g_source_destroy(source);
+
+  DestroyWeakPtr(conn_timer_data_);
+  conn_timer_data_ = nullptr;
+}
+
+void Proxy::SetIdler() {
+  if (idler_data_) {
+    _W("Already exists");
+    return;
   }
+
+  idler_data_ = CreateWeakPtr();
+  if (idler_data_ == nullptr) {
+    _E("Out of memory");
+    return;
+  }
+
+  g_idle_add(OnIdle, idler_data_);
+}
+
+void Proxy::UnsetIdler() {
+  if (idler_data_ == nullptr)
+    return;
+
+  GSource* source = g_main_context_find_source_by_user_data(nullptr,
+      idler_data_);
+  if (source && !g_source_is_destroyed(source))
+    g_source_destroy(source);
+
+  DestroyWeakPtr(idler_data_);
+  idler_data_ = nullptr;
 }
 
 void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid,
@@ -340,6 +380,13 @@ void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid,
   _W("app_id(%s), port_name(%s), pid(%d)", app_id, port_name, pid);
   auto* proxy = static_cast<Proxy*>(user_data);
   std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->UnsetIdler();
+
+  if (proxy->watch_handle_ == nullptr) {
+    _E("Invalid context");
+    return;
+  }
+
   auto* listener = proxy->listener_;
   if (listener == nullptr) {
     _E("Invalid context");
@@ -362,21 +409,67 @@ void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid,
 void Proxy::OnPortVanished(const char* app_id, const char* port_name, int pid,
     void* user_data) {
   _W("app_id(%s), port_name(%s), pid(%d)", app_id, port_name, pid);
+  auto* proxy = static_cast<Proxy*>(user_data);
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->UnsetIdler();
 }
 
 gboolean Proxy::OnTimedOut(gpointer user_data) {
   _E("Timed out");
-  auto* proxy = static_cast<Proxy*>(user_data);
+  auto* ptr = static_cast<std::weak_ptr<Proxy>*>(user_data);
+  auto proxy = ptr->lock();
+  if (proxy == nullptr) {
+    _E("Proxy is nullptr");
+    return G_SOURCE_REMOVE;
+  }
+
   std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  if (proxy->conn_timer_data_ == nullptr) {
+    _E("Invalid context. proxy(%p)", proxy.get());
+    return G_SOURCE_REMOVE;
+  }
+
+  DestroyWeakPtr(proxy->conn_timer_data_);
+  proxy->conn_timer_data_ = nullptr;
+
   auto* listener = proxy->listener_;
   if (listener == nullptr) {
     _E("Invalid context");
     return G_SOURCE_REMOVE;
   }
 
-  proxy->UnsetConnTimer();
   listener->OnRejected(proxy->target_appid_, RPC_PORT_ERROR_IO_ERROR);
-  return G_SOURCE_CONTINUE;
+  return G_SOURCE_REMOVE;
+}
+
+gboolean Proxy::OnIdle(gpointer user_data) {
+  auto* ptr = static_cast<std::weak_ptr<Proxy>*>(user_data);
+  auto proxy = ptr->lock();
+  if (proxy == nullptr) {
+    _E("Proxy is nullptr");
+    return G_SOURCE_REMOVE;
+  }
+
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  if (proxy->idler_data_ == nullptr) {
+    _E("Invalid context. proxy(%p)", proxy.get());
+    return G_SOURCE_REMOVE;
+  }
+
+  DestroyWeakPtr(proxy->idler_data_);
+  proxy->idler_data_ = nullptr;
+
+  bool exist = Aul::ExistPort(proxy->real_appid_, proxy->port_name_,
+      rpc_port_get_target_uid());
+  if (exist) {
+    proxy->OnPortAppeared(proxy->real_appid_.c_str(),
+        proxy->port_name_.c_str(), -1, proxy.get());
+  } else {
+    proxy->OnPortVanished(proxy->real_appid_.c_str(),
+        proxy->port_name_.c_str(), -1, proxy.get());
+  }
+
+  return G_SOURCE_REMOVE;
 }
 
 Proxy::ProxyPort::ProxyPort(Proxy* parent, int fd, const std::string& id,
@@ -670,5 +763,19 @@ gboolean Proxy::Client::OnResponseReceived(GIOChannel* channel,
   return G_SOURCE_REMOVE;
 }
 
+std::shared_ptr<Proxy> Proxy::GetSharedPtr() {
+  return shared_from_this();
+}
+
+gpointer Proxy::CreateWeakPtr() {
+  auto* ptr = new (std::nothrow) std::weak_ptr<Proxy>(GetSharedPtr());
+  return static_cast<gpointer>(ptr);
+}
+
+void Proxy::DestroyWeakPtr(gpointer data) {
+  auto* ptr = static_cast<std::weak_ptr<Proxy>*>(data);
+  delete ptr;
+}
+
 }  // namespace internal
 }  // namespace rpc_port

diff --git a/src/proxy-internal.hh b/src/proxy-internal.hh
index df0fbcb..414888f 100644 (file)
--- a/src/proxy-internal.hh
+++ b/src/proxy-internal.hh
@@ -32,7 +32,7 @@
 namespace rpc_port {
 namespace internal {
 
-class Proxy {
+class Proxy : public std::enable_shared_from_this<Proxy> {
  public:
   Proxy();
   virtual ~Proxy();
@@ -115,6 +115,7 @@ class Proxy {
   static void OnPortVanished(const char* app_id, const char* port_name, int pid,
       void* user_data);
   static gboolean OnTimedOut(gpointer user_data);
+  static gboolean OnIdle(gpointer user_data);
 
   void SetRealAppId(const std::string& alias_appid);
   std::recursive_mutex& GetMutex() const;
@@ -123,6 +124,12 @@ class Proxy {
   void Cancel();
   void SetConnTimer();
   void UnsetConnTimer();
+  void SetIdler();
+  void UnsetIdler();
+
+  std::shared_ptr<Proxy> GetSharedPtr();
+  gpointer CreateWeakPtr();
+  static void DestroyWeakPtr(gpointer data);
 
  private:
   std::string port_name_;
@@ -135,7 +142,8 @@ class Proxy {
   std::unique_ptr<Client> main_client_;
   std::unique_ptr<Client> delegate_client_;
   aul_rpc_port_watch_h watch_handle_ = nullptr;
-  guint conn_timer_ = 0;
+  gpointer conn_timer_data_ = nullptr;
+  gpointer idler_data_ = nullptr;
   mutable std::recursive_mutex mutex_;
 };
 

diff --git a/src/rpc-port.cc b/src/rpc-port.cc
index 7340c2b..1410a04 100644 (file)
--- a/src/rpc-port.cc
+++ b/src/rpc-port.cc
@@ -250,9 +250,19 @@ RPC_API int rpc_port_proxy_create(rpc_port_proxy_h* h) {
   if (h == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = new ::ProxyExt();
+  auto p = new (std::nothrow) std::shared_ptr<::ProxyExt>(
+      new (std::nothrow) ::ProxyExt());
+  if (p == nullptr) {
+    _E("Out of memory");
+  } else if (p->get() == nullptr) {
+    _E("Out of memory");
+    delete p;
+    p = nullptr;
+  } else {
+    _W("rpc_port_proxy_create(%p)", p->get());
+  }
+
   *h = p;
-  _W("rpc_port_proxy_create(%p)", p);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -260,15 +270,16 @@ RPC_API int rpc_port_proxy_destroy(rpc_port_proxy_h h) {
   if (h == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  _W("rpc_port_proxy_destroy(%p)", p);
-  p->SetDestroying(true);
-  p->DisconnectPort();
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  _W("rpc_port_proxy_destroy(%p)", proxy);
+  proxy->SetDestroying(true);
+  proxy->DisconnectPort();
 
   g_idle_add_full(G_PRIORITY_HIGH,
       [](gpointer data) -> gboolean {
-        auto p = static_cast<::ProxyExt*>(data);
-        _W("rpc_port_proxy_destroy(%p)", p);
+        auto p = static_cast<std::shared_ptr<::ProxyExt>*>(data);
+        _W("rpc_port_proxy_destroy(%p)", p->get());
         delete p;
         return G_SOURCE_REMOVE;
       }, h, nullptr);
@@ -280,11 +291,11 @@ RPC_API int rpc_port_proxy_connect(rpc_port_proxy_h h, const char* appid,
   if (h == nullptr || appid == nullptr || port == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  _W("rpc_port_proxy_connect(%p, %s, %s)", h, appid, port);
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  return p->Connect(appid, port, p);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  _W("rpc_port_proxy_connect(%p, %s, %s)", proxy, appid, port);
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  return proxy->Connect(appid, port, proxy);
 }
 
 // LCOV_EXCL_START
@@ -293,11 +304,11 @@ RPC_API int rpc_port_proxy_connect_sync(rpc_port_proxy_h h, const char* appid,
   if (h == nullptr || appid == nullptr || port == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  _W("rpc_port_proxy_connect(%p, %s, %s)", h, appid, port);
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  return p->ConnectSync(appid, port, p);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  _W("rpc_port_proxy_connect(%p, %s, %s)", proxy, appid, port);
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  return proxy->ConnectSync(appid, port, proxy);
 }
 // LCOV_EXCL_STOP
 
@@ -306,10 +317,10 @@ RPC_API int rpc_port_proxy_add_connected_event_cb(rpc_port_proxy_h h,
   if (h == nullptr || cb == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  p->AddConnectedEventListener(cb, user_data);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->AddConnectedEventListener(cb, user_data);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -318,10 +329,10 @@ RPC_API int rpc_port_proxy_add_disconnected_event_cb(rpc_port_proxy_h h,
   if (h == nullptr || cb == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  p->AddDisconnectedEventListener(cb, user_data);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->AddDisconnectedEventListener(cb, user_data);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -330,10 +341,10 @@ RPC_API int rpc_port_proxy_add_rejected_event_cb(rpc_port_proxy_h h,
   if (h == nullptr || cb == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  p->AddRejectedEventListener(cb, user_data);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->AddRejectedEventListener(cb, user_data);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -342,10 +353,10 @@ RPC_API int rpc_port_proxy_add_received_event_cb(rpc_port_proxy_h h,
   if (h == nullptr || cb == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
-
-  p->AddReceivedEventListener(cb, user_data);
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
+  proxy->AddReceivedEventListener(cb, user_data);
   return RPC_PORT_ERROR_NONE;
 }
 
@@ -354,19 +365,20 @@ RPC_API int rpc_port_proxy_get_port(rpc_port_proxy_h h,
   if (h == nullptr || port == nullptr)
     return RPC_PORT_ERROR_INVALID_PARAMETER;
 
-  auto p = static_cast<::ProxyExt*>(h);
-  std::lock_guard<std::recursive_mutex> lock(p->GetMutex());
+  auto p = static_cast<std::shared_ptr<::ProxyExt>*>(h);
+  auto* proxy = p->get();
+  std::lock_guard<std::recursive_mutex> lock(proxy->GetMutex());
   rpc_port_h ret_port;
 
   switch (type) {
     case RPC_PORT_PORT_MAIN:
-      ret_port = static_cast<rpc_port_h>(p->GetPort().get());
+      ret_port = static_cast<rpc_port_h>(proxy->GetPort().get());
       if (ret_port == nullptr)
         return RPC_PORT_ERROR_IO_ERROR;
       *port = ret_port;
       break;
     case RPC_PORT_PORT_CALLBACK:
-      ret_port = static_cast<rpc_port_h>(p->GetDelegatePort().get());
+      ret_port = static_cast<rpc_port_h>(proxy->GetDelegatePort().get());
       if (ret_port == nullptr)
         return RPC_PORT_ERROR_IO_ERROR;
       *port = ret_port;