Move connection timer to FdBroker

To make the thread safe Proxy class, the connection timer is moved to
the FdBroker class.

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

diff --git a/src/fdbroker-internal.cc b/src/fdbroker-internal.cc
index daf3090..c280339 100644 (file)
--- a/src/fdbroker-internal.cc
+++ b/src/fdbroker-internal.cc
@@ -232,6 +232,11 @@ void FdBroker::Cancel() {
     cancellable_ = nullptr;
   }
 
+  if (conn_timer_ > 0) {
+    g_source_remove(conn_timer_);
+    conn_timer_ = 0;
+  }
+
   if (watcher_id_ > 0) {
     g_bus_unwatch_name(watcher_id_);
     watcher_id_ = 0;
@@ -726,19 +731,30 @@ void FdBroker::OnNameAppeared(GDBusConnection *connection,
     return;
   }
 
+  if (broker->conn_timer_ > 0) {
+    g_source_remove(broker->conn_timer_);
+    broker->conn_timer_ = 0;
+  }
+
   int pid = broker->GetSenderPid(connection, name_owner);
   char owner_appid[255] = { 0, };
 
   if (aul_app_get_appid_bypid(pid, owner_appid, sizeof(owner_appid)) < 0) {
-    LOGE("aul_app_get_appid_bypid failed %d", pid);  // LCOV_EXCL_LINE
-    broker->watcher_->OnPortRejected(owner_appid);  // LCOV_EXCL_LINE
-    return;  // LCOV_EXCL_LINE
+    // LCOV_EXCL_START
+    LOGE("aul_app_get_appid_bypid failed %d", pid);
+    broker->watcher_->OnPortRejected(owner_appid,
+        RPC_PORT_ERROR_IO_ERROR);
+    return;
+    // LCOV_EXCL_STOP
   }
 
   if (broker->watch_appid_ != owner_appid) {
-    LOGE("invalid appid %s", owner_appid);  // LCOV_EXCL_LINE
-    broker->watcher_->OnPortRejected(owner_appid);  // LCOV_EXCL_LINE
-    return;  // LCOV_EXCL_LINE
+    // LCOV_EXCL_START
+    LOGE("invalid appid %s", owner_appid);
+    broker->watcher_->OnPortRejected(owner_appid,
+        RPC_PORT_ERROR_IO_ERROR);
+    return;
+    // LCOV_EXCL_STOP
   }
 
   broker->watcher_->OnPortAppeared(broker->watch_appid_,
@@ -766,6 +782,7 @@ void FdBroker::OnNameVanished(GDBusConnection *connection,
 
 int FdBroker::Watch(IEventWatcher* ev, const std::string& target_appid,
                     const std::string& port_name) {
+  std::lock_guard<std::recursive_mutex> lock(mutex_);
   if (ev == nullptr)
     return -1;
 
@@ -777,6 +794,11 @@ int FdBroker::Watch(IEventWatcher* ev, const std::string& target_appid,
   watch_appid_ = target_appid;
   watch_port_name_ = port_name;
 
+  if (conn_timer_ > 0)
+    g_source_remove(conn_timer_);
+
+  conn_timer_ = g_timeout_add(10 * 1000, OnDbusNameTimeout, this);
+
   if (mock_) {
     // LCOV_EXCL_START
     ret = DBusMock::GetInst().Watch(ev, target_appid, port_name);
@@ -806,9 +828,11 @@ int FdBroker::Watch(IEventWatcher* ev, const std::string& target_appid,
       this,
       NULL);
   if (watcher_id_ == 0) {
-    LOGE("Failed to watch connection(%s)", interface_name.c_str());  // LCOV_EXCL_LINE
-    watcher_ = nullptr;  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
+    // LCOV_EXCL_START
+    LOGE("Failed to watch connection(%s)", interface_name.c_str());
+    watcher_ = nullptr;
+    return -1;
+    // LCOV_EXCL_STOP
   }
 
   return 0;
@@ -898,7 +922,8 @@ void FdBroker::OnResultReceived(GObject* source_object,
   g_object_unref(reply);
   if (ret != 0) {
     LOGE("Access Denied[sender_appid : %s]", broker->watch_appid_.c_str());
-    watcher->OnPortRejected(broker->watch_appid_);
+    watcher->OnPortRejected(broker->watch_appid_,
+        RPC_PORT_ERROR_PERMISSION_DENIED);
     return;
   }
 
@@ -906,6 +931,27 @@ void FdBroker::OnResultReceived(GObject* source_object,
   LOGD("[Reply : %d]", ret);
 }
 
+gboolean FdBroker::OnDbusNameTimeout(gpointer user_data) {
+  FdBroker* broker = static_cast<FdBroker*>(user_data);
+  if (!FdBrokerManager::GetInst().Exist(broker)) {
+    LOGE("No such FdBroker(%p)", broker);
+    return G_SOURCE_REMOVE;
+  }
+
+  std::lock_guard<std::recursive_mutex> lock(broker->mutex_);
+  if (broker->conn_timer_ == 0) {
+    LOGE("Invalid context. FdBroker(%p)", broker);
+    return G_SOURCE_REMOVE;
+  }
+
+  broker->conn_timer_ = 0;
+  broker->Cancel();
+  auto* watcher = broker->watcher_;
+  watcher->OnPortRejected(broker->watch_appid_, RPC_PORT_ERROR_IO_ERROR);
+
+  return G_SOURCE_REMOVE;
+}
+
 FdBrokerManager& FdBrokerManager::GetInst() {
   static FdBrokerManager inst;
   return inst;

diff --git a/src/fdbroker-internal.h b/src/fdbroker-internal.h
index 8ddec5a..881df3a 100644 (file)
--- a/src/fdbroker-internal.h
+++ b/src/fdbroker-internal.h
@@ -48,7 +48,8 @@ class FdBroker {
                                 const std::string& port_name) = 0;
     virtual void OnPortVanished(const std::string& appid,
                                 const std::string& port_name) = 0;
-    virtual void OnPortRejected(const std::string& appid) = 0;
+    virtual void OnPortRejected(const std::string& appid,
+                                int error) = 0;
     virtual void OnPortConnected(const std::string& appid,
                                  const std::string& port_name) = 0;
     virtual void OnPortDisconnected(const std::string& appid,
@@ -174,6 +175,7 @@ class FdBroker {
   static void OnResultReceived(GObject* source_object,
                                GAsyncResult* res,
                                gpointer user_data);
+  static gboolean OnDbusNameTimeout(gpointer user_data);
 
  private:
   IEventListener* listener_ = nullptr;
@@ -185,6 +187,7 @@ class FdBroker {
   std::string watch_appid_;
   std::string watch_port_name_;
   GCancellable* cancellable_ = nullptr;
+  guint conn_timer_ = 0;
   mutable std::recursive_mutex mutex_;
 };
 

diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc
index dcbe1bf..040116d 100644 (file)
--- a/src/proxy-internal.cc
+++ b/src/proxy-internal.cc
@@ -36,10 +36,6 @@ Proxy::Proxy(bool mock)
 
 Proxy::~Proxy() {
   _D("Proxy::~Proxy");
-  if (conn_timer_) {
-    g_source_remove(conn_timer_);  // LCOV_EXCL_LINE
-    conn_timer_ = 0;  // LCOV_EXCL_LINE
-  }
 }
 
 gboolean Proxy::OnSocketDisconnected(GIOChannel *gio, GIOCondition cond,
@@ -100,22 +96,18 @@ gboolean Proxy::OnDataReceived(GIOChannel *gio, GIOCondition cond,
   return TRUE;
 }
 
-void Proxy::OnPortRejected(const std::string& appid) {
+void Proxy::OnPortRejected(const std::string& appid, int error) {
   if (listener_ == nullptr)
     return;
 
   IEventListener* listener = listener_;
   listener_ = nullptr;
-  listener->OnRejected(appid, RPC_PORT_ERROR_PERMISSION_DENIED);
+  listener->OnRejected(appid, error);
 }
 
 void Proxy::OnPortAppeared(const std::string& appid,
                            const std::string& port_name) {
   _D("endpoint(%s), port_name(%s)", appid.c_str(), port_name.c_str());
-  if (conn_timer_) {
-    g_source_remove(conn_timer_);
-    conn_timer_ = 0;
-  }
 
   if (listener_ == nullptr)
     return;
@@ -196,14 +188,9 @@ int Proxy::Connect(std::string appid, std::string port_name,
   target_appid_ = std::move(appid);
   port_name_ = std::move(port_name);
 
-  if (conn_timer_)
-    g_source_remove(conn_timer_);  // LCOV_EXCL_LINE
-  conn_timer_ = g_timeout_add(10 * 1000, DbusNameTimeout, this);
-
   int r = fd_broker_.Watch(this, target_appid_, port_name_);
   if (r < 0) {
     // LCOV_EXCL_START
-    g_source_remove(conn_timer_);
     listener_ = nullptr;
     if (r == -EILLEGALACCESS)
       return RPC_PORT_ERROR_PERMISSION_DENIED;
@@ -259,23 +246,6 @@ int Proxy::ConnectSync(std::string appid, std::string port_name,
 }
 // LCOV_EXCL_STOP
 
-// LCOV_EXCL_START
-gboolean Proxy::DbusNameTimeout(gpointer user_data) {
-  Proxy* obj = static_cast<Proxy*>(user_data);
-
-  _W("[__DbusNameTimeout__] endpoint(%s)", obj->target_appid_.c_str());
-  obj->conn_timer_ = 0;
-  if (obj->listener_) {
-    IEventListener* listener = obj->listener_;
-    obj->listener_ = nullptr;
-    obj->fd_broker_.Cancel();
-    listener->OnRejected(obj->target_appid_, RPC_PORT_ERROR_IO_ERROR);
-  }
-
-  return G_SOURCE_REMOVE;
-}
-// LCOV_EXCL_STOP
-
 Proxy::ProxyPort::ProxyPort(Proxy* parent, int fd, const std::string& id,
     bool receive) : Port(fd, id), parent_(parent) {
   Watch(receive);

diff --git a/src/proxy-internal.h b/src/proxy-internal.h
index 63cbf52..485373e 100644 (file)
--- a/src/proxy-internal.h
+++ b/src/proxy-internal.h
@@ -86,7 +86,8 @@ class Proxy : public FdBroker::IEventWatcher {
                       const std::string& port_name) override;
   void OnPortVanished(const std::string& appid,
                       const std::string& port_name) override;
-  void OnPortRejected(const std::string& appid) override;
+  void OnPortRejected(const std::string& appid,
+                      int error) override;
   void OnPortConnected(const std::string& appid,
                        const std::string& port_name) override;
   void OnPortDisconnected(const std::string& appid,