Fix thread safe issue 15/315915/1
authorHwankyu Jhun <h.jhun@samsung.com>
Fri, 9 Aug 2024 10:16:12 +0000 (19:16 +0900)
committerHwankyu Jhun <h.jhun@samsung.com>
Fri, 9 Aug 2024 10:18:50 +0000 (19:18 +0900)
If the process tries to release the GSource when calling the idle callback,
it can make the crash issue as below:
+------------------------------------------------------------------------------+
| #0  0xb5a9201a in g_source_remove_child_source (source=<optimized out>,      |
|                child_source=<optimized out>) at /usr/src/debug/glib2-2.78.4- |
|                0.arm/_build/../glib/gmain.c:1725                             |
| #1  0xb5a92108 in g_source_destroy (source=<optimized out>) at /usr/src/debug|
|                /glib2-2.78.4-0.arm/_build/../glib/gmain.c:1477               |
+------------------------------------------------------------------------------+

This patch adds locking & unlocking the mutex to prevent the issue.

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

index 505c8f822e68e4e2127a0169c67dba4df9fd5ba3..e83209b3759db56de9bc05d8852747071c8166ea 100644 (file)
@@ -186,25 +186,32 @@ int AccessController::Check(int fd, const std::string& sender_appid) {
 
 void AccessController::CheckAsync(int fd, std::string sender_appid,
                                   CompleteCallback callback) {
-  /* This is for handle freed issue */
+  std::lock_guard<std::recursive_mutex> lock(GetMutex());
   auto* tmp_handle = new std::shared_ptr<AccessController>(shared_from_this());
   GMainContext* context = g_main_context_ref_thread_default();
   Job job([=]() -> Job::Type {
+    std::lock_guard<std::recursive_mutex> job_lock(GetMutex());
     if ((*tmp_handle).use_count() == 1) {
       delete tmp_handle;  // LCOV_EXCL_LINE
       return Job::Type::Continue;  // LCOV_EXCL_LINE
     }
 
     int res = Check(fd, sender_appid);
-    auto* cbdata = new std::pair<CompleteCallback, int>(
-        std::move(callback), res);
+    auto* cbdata = new std::tuple<CompleteCallback, int,
+                                  std::shared_ptr<AccessController>*>(
+        std::move(callback), res, tmp_handle);
 
     auto* source = GLib::IdleAdd(
         context,
         [](gpointer data) -> gboolean {
-          auto* cb_data = static_cast<std::pair<CompleteCallback, int>*>(data);
-          auto [callback, res] = *cb_data;
+          auto* cb_data =
+              static_cast<std::tuple<CompleteCallback, int,
+                                     std::shared_ptr<AccessController>*>*>(
+                  data);
+          auto [callback, res, ptr] = *cb_data;
+          std::lock_guard<std::recursive_mutex> cb_lock((*ptr)->GetMutex());
           callback(res);
+          delete ptr;
           delete cb_data;
           return G_SOURCE_REMOVE;
         },
@@ -215,12 +222,13 @@ void AccessController::CheckAsync(int fd, std::string sender_appid,
       g_source_set_priority(source, G_PRIORITY_DEFAULT);
 
     g_main_context_unref(context);
-    delete tmp_handle;
     return Job::Type::Continue;
   });
 
   CynaraThread::GetInst().Push(std::move(job));
 }
 
+std::recursive_mutex& AccessController::GetMutex() const { return mutex_; }
+
 }  // namespace internal
 }  // namespace rpc_port
index 0b48d5c3794f246acd274d3866d731767d4fb60c..aecdeb5414540196962709df40b6d55ac64e3ed8 100644 (file)
@@ -24,6 +24,7 @@
 #include <functional>
 #include <map>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -44,12 +45,14 @@ class AccessController : public std::enable_shared_from_this<AccessController> {
  private:
   int CheckTrusted(const std::string& sender_appid);
   int CheckPrivilege(int fd);
+  std::recursive_mutex& GetMutex() const;
 
  private:
   std::vector<std::string> privileges_;
   std::map<std::string, bool> cache_;
   bool trusted_;
   std::string appid_;
+  mutable std::recursive_mutex mutex_;
 };
 
 }  // namespace internal