From: Hwankyu Jhun <h.jhun@samsung.com>
Date: Fri, 19 Jun 2020 02:19:27 +0000 (+0900)
Subject: Fix thread safe issue
X-Git-Tag: submit/tizen/20200619.061341~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dcaadb698055fb38a6d6182a5318bdf6a3729d71;p=platform%2Fcore%2Fappfw%2Frpc-port.git

Fix thread safe issue

To avoid thread safe issues, FdBrokerManager class is added.
While calling the callback functions, the FdBroker ptr is checked
whether the ptr is valid or not.

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

diff --git a/src/fdbroker-internal.cc b/src/fdbroker-internal.cc
index 98b09e7..54ed899 100644
--- a/src/fdbroker-internal.cc
+++ b/src/fdbroker-internal.cc
@@ -25,6 +25,8 @@
 #include <aul_rpc_port.h>
 #include <dlog.h>
 
+#include <algorithm>
+
 #include "fdbroker-internal.h"
 
 #ifdef LOG_TAG
@@ -210,13 +212,20 @@ GUnixFDList* FdBroker::FdList::GetRaw() {
   return fd_list_;
 }
 
+FdBroker::FdBroker(bool mock) : mock_(mock) {
+  FdBrokerManager::GetInst().Add(this);
+}
+
 FdBroker::~FdBroker() {
   Cancel();
 }
 
 void FdBroker::Cancel() {
+  std::lock_guard<std::recursive_mutex> lock(mutex_);
   LOGI("FdBroker::Cancel!");
 
+  FdBrokerManager::GetInst().Remove(this);
+
   if (cancellable_) {
     LOGW("Cancel the send request");
     g_cancellable_cancel(cancellable_);
@@ -497,6 +506,12 @@ void FdBroker::OnReceiveDbusMethod(GDBusConnection *conn,
     GVariant *parameters, GDBusMethodInvocation *invocation,
     gpointer user_data) {
   FdBroker* broker = static_cast<FdBroker*>(user_data);
+  if (!FdBrokerManager::GetInst().Exist(broker)) {
+    LOGE("No such broker(%p)", broker);
+    return;
+  }
+
+  std::lock_guard<std::recursive_mutex> lock(broker->mutex_);
   int ret = -1;
   char sender_appid[255];
   int sender_pid;
@@ -696,6 +711,12 @@ void FdBroker::OnNameAppeared(GDBusConnection *connection,
                               const gchar *name_owner,
                               gpointer user_data) {
   FdBroker* broker = static_cast<FdBroker*>(user_data);
+  if (!FdBrokerManager::GetInst().Exist(broker)) {
+    LOGE("No such broker(%p)", broker);
+    return;
+  }
+
+  std::lock_guard<std::recursive_mutex> lock(broker->mutex_);
   int pid = broker->GetSenderPid(connection, name_owner);
   char owner_appid[255] = { 0, };
 
@@ -719,6 +740,12 @@ void FdBroker::OnNameVanished(GDBusConnection *connection,
                               const gchar *name,
                               gpointer user_data) {
   FdBroker* broker = static_cast<FdBroker*>(user_data);
+  if (!FdBrokerManager::GetInst().Exist(broker)) {
+    LOGE("No such broker(%p)", broker);
+    return;
+  }
+
+  std::lock_guard<std::recursive_mutex> lock(broker->mutex_);
   broker->watcher_->OnPortVanished(broker->watch_appid_,
       broker->watch_port_name_);
 }
@@ -796,6 +823,7 @@ int FdBroker::Prepare(const std::string& target_appid,
 void FdBroker::OnResultReceived(GObject* source_object,
                                 GAsyncResult* res,
                                 gpointer user_data) {
+  LOGW("OnResultReceived() : FdBroker(%p)", user_data);
   GDBusConnection* conn = reinterpret_cast<GDBusConnection*>(source_object);
   GError* err = nullptr;
   GDBusMessage* reply = g_dbus_connection_send_message_with_reply_finish(conn,
@@ -808,10 +836,12 @@ void FdBroker::OnResultReceived(GObject* source_object,
   }
 
   FdBroker* broker = static_cast<FdBroker*>(user_data);
-  if (broker == nullptr) {
-      LOGW("Null broker");  // LCOV_EXCL_LINE
-      return;  // LCOV_EXCL_LINE
+  if (!FdBrokerManager::GetInst().Exist(broker)) {
+    LOGE("No such FdBroker(%p)", broker);
+    return;
   }
+
+  std::lock_guard<std::recursive_mutex> lock(broker->mutex_);
   IEventWatcher* watcher = broker->watcher_;
 
   if (err) {
@@ -857,5 +887,29 @@ void FdBroker::OnResultReceived(GObject* source_object,
   LOGD("[Reply : %d]", ret);
 }
 
+FdBrokerManager& FdBrokerManager::GetInst() {
+  static FdBrokerManager inst;
+  return inst;
+}
+
+void FdBrokerManager::Add(FdBroker* broker) {
+  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  brokers_.push_back(broker);
+}
+
+void FdBrokerManager::Remove(FdBroker* broker) {
+  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  brokers_.remove(broker);
+}
+
+bool FdBrokerManager::Exist(FdBroker* broker) {
+  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  auto iter = std::find(brokers_.begin(), brokers_.end(), broker);
+  if (iter != brokers_.end())
+    return true;
+
+  return false;
+}
+
 }  // namespace internal
 }  // namespace rpc_port
diff --git a/src/fdbroker-internal.h b/src/fdbroker-internal.h
index 0e5e325..8ddec5a 100644
--- a/src/fdbroker-internal.h
+++ b/src/fdbroker-internal.h
@@ -22,9 +22,12 @@
 #include <gio/gunixfdlist.h>
 #include <glib-unix.h>
 
-#include <string>
 #include <map>
 #include <memory>
+#include <mutex>
+#include <string>
+#include <thread>
+#include <list>
 
 #include "ac-internal.h"
 #include "rpc-port.h"
@@ -53,7 +56,7 @@ class FdBroker {
                                     bool cancel = false) = 0;
   };
 
-  explicit FdBroker(bool mock = false) : mock_(mock) {}
+  explicit FdBroker(bool mock = false);
   ~FdBroker();
 
   static void Dispose() {
@@ -182,6 +185,23 @@ class FdBroker {
   std::string watch_appid_;
   std::string watch_port_name_;
   GCancellable* cancellable_ = nullptr;
+  mutable std::recursive_mutex mutex_;
+};
+
+class FdBrokerManager {
+ private:
+  FdBrokerManager() = default;
+  ~FdBrokerManager() = default;
+
+ public:
+  static FdBrokerManager& GetInst();
+  void Add(FdBroker* broker);
+  void Remove(FdBroker* broker);
+  bool Exist(FdBroker* broker);
+
+ private:
+  std::list<FdBroker*> brokers_;
+  mutable std::recursive_mutex mutex_;
 };
 
 }  // namespace internal