Refactor app_signal.cc

This is for the multi threading environment.
If some sub-threads unsubscribe the event, there is room for problems due to race condition.
This patch is changed to maintain the Event object without destruction after creation.

Change-Id: Ieefca296875c14e33d195d2809bc5134485915a5
Signed-off-by: Changgyu Choi <changyu.choi@samsung.com>

diff --git a/src/app_signal.cc b/src/app_signal.cc
index fff4780..0f40e5a 100644 (file)
--- a/src/app_signal.cc
+++ b/src/app_signal.cc
@@ -62,12 +62,10 @@ template <typename T>
 class Event {
  public:
   Event(std::string object_path, std::string interface_name,
-      std::string signal_name, T cb, void* user_data)
+      std::string signal_name)
       : object_path_(std::move(object_path)),
         interface_name_(std::move(interface_name)),
-        signal_name_(std::move(signal_name)),
-        cb_(cb),
-        user_data_(user_data) {
+        signal_name_(std::move(signal_name)) {
   }
 
   virtual ~Event() {
@@ -87,6 +85,7 @@ class Event {
   void* GetUserData() const { return user_data_; }
 
   bool Subscribe() {
+    std::lock_guard<std::recursive_mutex> lock(GetMutex());
     if (source_ != 0)
       return true;
 
@@ -98,9 +97,10 @@ class Event {
       return false;
     }
 
-    source_ = g_dbus_connection_signal_subscribe(conn_, nullptr,
-        interface_name_.c_str(), signal_name_.c_str(), object_path_.c_str(),
-         nullptr, G_DBUS_SIGNAL_FLAGS_NONE, DBusSignalCb, this, nullptr);
+    source_ = g_dbus_connection_signal_subscribe(
+        conn_, nullptr, interface_name_.c_str(), signal_name_.c_str(),
+        object_path_.c_str(), nullptr, G_DBUS_SIGNAL_FLAGS_NONE, DBusSignalCb,
+        this, nullptr);
     if (source_ == 0) {
       _E("g_dbus_connection_signal_subscribe() is failed");
       g_object_unref(conn_);
@@ -111,6 +111,19 @@ class Event {
     return true;
   }
 
+  bool SetCallback(T cb, void* user_data) {
+    std::lock_guard<std::recursive_mutex> lock(GetMutex());
+    cb_ = cb;
+    user_data_ = user_data;
+
+    if (cb_ == nullptr) {
+      Unsubscribe();
+      return true;
+    }
+
+    return Subscribe();
+  }
+
  private:
   void Unsubscribe() {
     std::lock_guard<std::recursive_mutex> lock(GetMutex());
@@ -134,15 +147,16 @@ class Event {
       GVariant* parameters, gpointer user_data) {
     auto* event = static_cast<Event*>(user_data);
     std::lock_guard<std::recursive_mutex> lock(event->GetMutex());
-    event->Invoke(parameters);
+    if (event->source_ != 0)
+      event->Invoke(parameters);
   }
 
  private:
   std::string object_path_;
   std::string interface_name_;
   std::string signal_name_;
-  T cb_;
-  void* user_data_;
+  std::remove_pointer_t<T>* cb_ = nullptr;
+  void* user_data_ = nullptr;
   GDBusConnection* conn_ = nullptr;
   guint source_ = 0;
   mutable std::recursive_mutex mutex_;
@@ -150,9 +164,8 @@ class Event {
 
 class AppDeadEvent : public Event<aul_app_dead_event_cb> {
  public:
-  AppDeadEvent(aul_app_dead_event_cb cb, void* user_data)
-      : Event<aul_app_dead_event_cb>(AUL_DBUS_PATH, AUL_DBUS_SIGNAL_INTERFACE,
-            AUL_DBUS_APPDEAD_SIGNAL, cb, user_data) {
+  AppDeadEvent() : Event<aul_app_dead_event_cb>(AUL_DBUS_PATH,
+            AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPDEAD_SIGNAL) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -167,9 +180,8 @@ class AppDeadEvent : public Event<aul_app_dead_event_cb> {
 
 class AppLaunchEvent : public Event<aul_app_launch_event_cb> {
  public:
-  AppLaunchEvent(aul_app_launch_event_cb cb, void* user_data)
-      : Event<aul_app_launch_event_cb>(AUL_DBUS_PATH, AUL_DBUS_SIGNAL_INTERFACE,
-            AUL_DBUS_APPLAUNCH_SIGNAL, cb, user_data) {
+  AppLaunchEvent() : Event<aul_app_launch_event_cb>(AUL_DBUS_PATH,
+            AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -185,10 +197,8 @@ class AppLaunchEvent : public Event<aul_app_launch_event_cb> {
 
 class AppLaunchEvent2 : public Event<aul_app_launch_event_cb_v2> {
  public:
-  AppLaunchEvent2(aul_app_launch_event_cb_v2 cb, void* user_data)
-      : Event<aul_app_launch_event_cb_v2>(AUL_DBUS_PATH,
-            AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL, cb,
-            user_data) {
+  AppLaunchEvent2() : Event<aul_app_launch_event_cb_v2>(AUL_DBUS_PATH,
+            AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -204,9 +214,8 @@ class AppLaunchEvent2 : public Event<aul_app_launch_event_cb_v2> {
 
 class BootingDoneEvent : public Event<aul_booting_done_event_cb> {
  public:
-  BootingDoneEvent(aul_booting_done_event_cb cb, void* user_data)
-      : Event<aul_booting_done_event_cb>(SYSTEM_PATH_CORE,
-            SYSTEM_INTERFACE_CORE, SYSTEM_SIGNAL_BOOTING_DONE, cb, user_data) {
+  BootingDoneEvent() : Event<aul_booting_done_event_cb>(SYSTEM_PATH_CORE,
+            SYSTEM_INTERFACE_CORE, SYSTEM_SIGNAL_BOOTING_DONE) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -218,10 +227,8 @@ class BootingDoneEvent : public Event<aul_booting_done_event_cb> {
 
 class CooldownEvent : public Event<aul_cooldown_event_cb> {
  public:
-  CooldownEvent(aul_cooldown_event_cb cb, void* user_data)
-      : Event<aul_cooldown_event_cb>(SYSTEM_PATH_THERMAL,
-            SYSTEM_INTERFACE_THERMAL, SYSTEM_SIGNAL_COOLDOWN_MODE_CHANGED, cb,
-            user_data) {
+  CooldownEvent() : Event<aul_cooldown_event_cb>(SYSTEM_PATH_THERMAL,
+            SYSTEM_INTERFACE_THERMAL, SYSTEM_SIGNAL_COOLDOWN_MODE_CHANGED) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -236,10 +243,9 @@ class CooldownEvent : public Event<aul_cooldown_event_cb> {
 
 class AppStatusChangedEvent : public Event<aul_app_status_changed_cb> {
  public:
-  AppStatusChangedEvent(aul_app_status_changed_cb cb, void* user_data)
-      : Event<aul_app_status_changed_cb>(RESOURCED_PROC_PATH,
-            RESOURCED_PROC_INTERFACE, RESOURCED_PROC_STATUS_SIGNAL, cb,
-            user_data) {
+  AppStatusChangedEvent()
+          : Event<aul_app_status_changed_cb>(RESOURCED_PROC_PATH,
+            RESOURCED_PROC_INTERFACE, RESOURCED_PROC_STATUS_SIGNAL) {
   }
 
   void Invoke(GVariant* parameters) override {
@@ -261,71 +267,53 @@ class AppSignalContext {
 
   bool AppDeadEventSubscribe(aul_app_dead_event_cb cb, void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      app_dead_event_.reset();
-      return true;
-    }
+    if (app_dead_event_ == nullptr)
+      app_dead_event_ = std::make_unique<AppDeadEvent>();
 
-    app_dead_event_.reset(new AppDeadEvent(cb, user_data));
-    return app_dead_event_->Subscribe();
+    return app_dead_event_->SetCallback(cb, user_data);
   }
 
   bool AppLaunchEventSubscribe(aul_app_launch_event_cb cb, void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      app_launch_event_.reset();
-      return true;
-    }
+    if (app_launch_event_ == nullptr)
+      app_launch_event_ = std::make_unique<AppLaunchEvent>();
 
-    app_launch_event_.reset(new AppLaunchEvent(cb, user_data));
-    return app_launch_event_->Subscribe();
+    return app_launch_event_->SetCallback(cb, user_data);
   }
 
   bool AppLaunchEvent2Subscribe(aul_app_launch_event_cb_v2 cb,
       void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      app_launch_event2_.reset();
-      return true;
-    }
+    if (app_launch_event2_ == nullptr)
+      app_launch_event2_ = std::make_unique<AppLaunchEvent2>();
 
-    app_launch_event2_.reset(new AppLaunchEvent2(cb, user_data));
-    return app_launch_event2_->Subscribe();
+    return app_launch_event2_->SetCallback(cb, user_data);
   }
 
   bool BootingDoneEventSubscribe(aul_booting_done_event_cb cb,
       void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      booting_done_event_.reset();
-      return true;
-    }
+    if (app_dead_event_ == nullptr)
+      app_dead_event_ = std::make_unique<AppDeadEvent>();
 
-    booting_done_event_.reset(new BootingDoneEvent(cb, user_data));
-    return booting_done_event_->Subscribe();
+    return booting_done_event_->SetCallback(cb, user_data);
   }
 
   bool CooldownEventSubscribe(aul_cooldown_event_cb cb, void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      cooldown_event_.reset();
-      return true;
-    }
+    if (cooldown_event_ == nullptr)
+      cooldown_event_ = std::make_unique<CooldownEvent>();
 
-    cooldown_event_.reset(new CooldownEvent(cb, user_data));
-    return cooldown_event_->Subscribe();
+    return cooldown_event_->SetCallback(cb, user_data);
   }
 
   bool AppStatusChangedEventSubscribe(aul_app_status_changed_cb cb,
       void* user_data) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
-    if (cb == nullptr) {
-      app_status_changed_event_.reset();
-      return true;
-    }
+    if (app_status_changed_event_ == nullptr)
+      app_status_changed_event_ = std::make_unique<AppStatusChangedEvent>();
 
-    app_status_changed_event_.reset(new AppStatusChangedEvent(cb, user_data));
-    return app_status_changed_event_->Subscribe();
+    return app_status_changed_event_->SetCallback(cb, user_data);
   }
 
  private: