From 2c12c07206d585a96649c8bb7225f4f3491a37ee Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Tue, 27 Dec 2022 20:12:30 +0900 Subject: [PATCH] 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 --- src/app_signal.cc | 122 ++++++++++++++++++++++++------------------------------ 1 file changed, 55 insertions(+), 67 deletions(-) diff --git a/src/app_signal.cc b/src/app_signal.cc index fff4780..0f40e5a 100644 --- a/src/app_signal.cc +++ b/src/app_signal.cc @@ -62,12 +62,10 @@ template 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 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 lock(GetMutex()); + cb_ = cb; + user_data_ = user_data; + + if (cb_ == nullptr) { + Unsubscribe(); + return true; + } + + return Subscribe(); + } + private: void Unsubscribe() { std::lock_guard lock(GetMutex()); @@ -134,15 +147,16 @@ class Event { GVariant* parameters, gpointer user_data) { auto* event = static_cast(user_data); std::lock_guard 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* 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 { public: - AppDeadEvent(aul_app_dead_event_cb cb, void* user_data) - : Event(AUL_DBUS_PATH, AUL_DBUS_SIGNAL_INTERFACE, - AUL_DBUS_APPDEAD_SIGNAL, cb, user_data) { + AppDeadEvent() : Event(AUL_DBUS_PATH, + AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPDEAD_SIGNAL) { } void Invoke(GVariant* parameters) override { @@ -167,9 +180,8 @@ class AppDeadEvent : public Event { class AppLaunchEvent : public Event { public: - AppLaunchEvent(aul_app_launch_event_cb cb, void* user_data) - : Event(AUL_DBUS_PATH, AUL_DBUS_SIGNAL_INTERFACE, - AUL_DBUS_APPLAUNCH_SIGNAL, cb, user_data) { + AppLaunchEvent() : Event(AUL_DBUS_PATH, + AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL) { } void Invoke(GVariant* parameters) override { @@ -185,10 +197,8 @@ class AppLaunchEvent : public Event { class AppLaunchEvent2 : public Event { public: - AppLaunchEvent2(aul_app_launch_event_cb_v2 cb, void* user_data) - : Event(AUL_DBUS_PATH, - AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL, cb, - user_data) { + AppLaunchEvent2() : Event(AUL_DBUS_PATH, + AUL_DBUS_SIGNAL_INTERFACE, AUL_DBUS_APPLAUNCH_SIGNAL) { } void Invoke(GVariant* parameters) override { @@ -204,9 +214,8 @@ class AppLaunchEvent2 : public Event { class BootingDoneEvent : public Event { public: - BootingDoneEvent(aul_booting_done_event_cb cb, void* user_data) - : Event(SYSTEM_PATH_CORE, - SYSTEM_INTERFACE_CORE, SYSTEM_SIGNAL_BOOTING_DONE, cb, user_data) { + BootingDoneEvent() : Event(SYSTEM_PATH_CORE, + SYSTEM_INTERFACE_CORE, SYSTEM_SIGNAL_BOOTING_DONE) { } void Invoke(GVariant* parameters) override { @@ -218,10 +227,8 @@ class BootingDoneEvent : public Event { class CooldownEvent : public Event { public: - CooldownEvent(aul_cooldown_event_cb cb, void* user_data) - : Event(SYSTEM_PATH_THERMAL, - SYSTEM_INTERFACE_THERMAL, SYSTEM_SIGNAL_COOLDOWN_MODE_CHANGED, cb, - user_data) { + CooldownEvent() : Event(SYSTEM_PATH_THERMAL, + SYSTEM_INTERFACE_THERMAL, SYSTEM_SIGNAL_COOLDOWN_MODE_CHANGED) { } void Invoke(GVariant* parameters) override { @@ -236,10 +243,9 @@ class CooldownEvent : public Event { class AppStatusChangedEvent : public Event { public: - AppStatusChangedEvent(aul_app_status_changed_cb cb, void* user_data) - : Event(RESOURCED_PROC_PATH, - RESOURCED_PROC_INTERFACE, RESOURCED_PROC_STATUS_SIGNAL, cb, - user_data) { + AppStatusChangedEvent() + : Event(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 lock(mutex_); - if (cb == nullptr) { - app_dead_event_.reset(); - return true; - } + if (app_dead_event_ == nullptr) + app_dead_event_ = std::make_unique(); - 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 lock(mutex_); - if (cb == nullptr) { - app_launch_event_.reset(); - return true; - } + if (app_launch_event_ == nullptr) + app_launch_event_ = std::make_unique(); - 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 lock(mutex_); - if (cb == nullptr) { - app_launch_event2_.reset(); - return true; - } + if (app_launch_event2_ == nullptr) + app_launch_event2_ = std::make_unique(); - 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 lock(mutex_); - if (cb == nullptr) { - booting_done_event_.reset(); - return true; - } + if (app_dead_event_ == nullptr) + app_dead_event_ = std::make_unique(); - 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 lock(mutex_); - if (cb == nullptr) { - cooldown_event_.reset(); - return true; - } + if (cooldown_event_ == nullptr) + cooldown_event_ = std::make_unique(); - 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 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(); - 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: -- 2.7.4