From: pjh9216 Date: Mon, 11 Nov 2024 00:00:11 +0000 (+0900) Subject: Fix accessing deleted object using shared_ ptr X-Git-Tag: accepted/tizen/unified/20241115.152446~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F01%2F320201%2F13;p=platform%2Fcore%2Fappfw%2Flibeventsystem.git Fix accessing deleted object using shared_ ptr Change-Id: I0d05cf38ce457c30dc84f8e86e5f37576b95452c Signed-off-by: pjh9216 --- diff --git a/src/es.cc b/src/es.cc index 1ce6bec..28d2a36 100644 --- a/src/es.cc +++ b/src/es.cc @@ -32,7 +32,9 @@ namespace { constexpr const char SYS_EVENT_NAME_PREFIX[] = "tizen.system.event"; -std::unique_ptr es_proxy_; +std::future connection_fut_; + +std::shared_ptr es_proxy_; class Event : public rpc_proxy::EventSystem::EventListener { public: @@ -60,39 +62,31 @@ class Event : public rpc_proxy::EventSystem::EventListener { class ESConnection : public rpc_proxy::EventSystem::IEventListener { public: void OnConnected() override { - connected_ = true; _D("OnConnected"); } /* LCOV_EXCL_START */ void OnDisconnected() override { - connected_ = false; - es_proxy_.reset(); _E("OnDisconnected"); + es_proxy_.reset(); } void OnRejected() override { - es_proxy_.reset(); _E("OnRejected"); + es_proxy_.reset(); } /* LCOV_EXCL_STOP */ - bool IsConnected() const { return connected_; } - - void Reset() { connected_ = false; } - - private: - bool connected_ = false; } connection_listener_; -std::future connection_fut_; - void Connect() { - if (!es_proxy_) { + if (!es_proxy_ ) { es_proxy_.reset(new rpc_proxy::EventSystem( &connection_listener_, "d::org.tizen.appfw.service.esd")); - connection_fut_ = - std::async(std::launch::async, []() { es_proxy_->Connect(true); }); + connection_fut_ = std::async(std::launch::async, [p = es_proxy_]() mutable { + p->Connect(true); + p.reset(); + }); } connection_fut_.wait(); @@ -144,23 +138,20 @@ extern "C" EXPORT_API int eventsystem_register_application_event( auto ev = std::unique_ptr( new Event(callback, user_data)); - if (!connection_listener_.IsConnected() && ::IsReservedEvent(name)) { + if (::IsReservedEvent(name) && !es_proxy_) { _W("Connect Asynchronously"); - bool first_time = false; - if (!es_proxy_) { - es_proxy_.reset(new rpc_proxy::EventSystem( - &connection_listener_, "d::org.tizen.appfw.service.esd")); - first_time = true; - } + es_proxy_.reset(new rpc_proxy::EventSystem( + &connection_listener_, "d::org.tizen.appfw.service.esd")); *reg_id = ev->GetSeqId(); es_proxy_->AddAsyncEventList(name, std::move(ev)); *event_type = ::GetEventType(name); - if (first_time) { - connection_fut_ = - std::async(std::launch::async, []() { es_proxy_->Connect(true); }); - } + connection_fut_ = + std::async(std::launch::async, [p = es_proxy_]() mutable { + p->Connect(true); + p.reset(); + }); return ES_R_OK; } @@ -203,9 +194,7 @@ extern "C" EXPORT_API int eventsystem_unregister_application_event( extern "C" EXPORT_API int eventsystem_application_finalize() { if (!es_proxy_) return 0; - es_proxy_->Disconnect(); es_proxy_.reset(); - connection_listener_.Reset(); return 0; } @@ -246,8 +235,6 @@ extern "C" EXPORT_API int eventsystem_unregister_event(unsigned int reg_id) { if (es_proxy_->GetSizeAsyncEventList() == 0) { es_proxy_.reset(); - connection_listener_.Reset(); - es_proxy_ = nullptr; } return ES_R_OK; diff --git a/src/es_proxy.cc b/src/es_proxy.cc index cefc1f4..784594a 100644 --- a/src/es_proxy.cc +++ b/src/es_proxy.cc @@ -104,6 +104,7 @@ LocalExecution::LocalExecution(std::string port_name, } LocalExecution::~LocalExecution() { + std::lock_guard lock(mutex_q_); while (!result_queue_.empty()) { auto parcel = result_queue_.front(); result_queue_.pop(); @@ -193,7 +194,7 @@ bool LocalExecution::LoadSymbols() { reinterpret_cast(dlsym(RTLD_DEFAULT, symbol.c_str())); if (connect_func_ == nullptr) { _E("Failed to find symbol(%s)", symbol.c_str()); /* LCOV_EXCL_LINE */ - return false; /* LCOV_EXCL_LINE */ + return false; /* LCOV_EXCL_LINE */ } } @@ -204,7 +205,7 @@ bool LocalExecution::LoadSymbols() { dlsym(RTLD_DEFAULT, symbol.c_str())); if (disconnect_func_ == nullptr) { _E("Failed to find symbol(%s)", symbol.c_str()); /* LCOV_EXCL_LINE */ - return false; /* LCOV_EXCL_LINE */ + return false; /* LCOV_EXCL_LINE */ } } @@ -215,7 +216,7 @@ bool LocalExecution::LoadSymbols() { reinterpret_cast(dlsym(RTLD_DEFAULT, symbol.c_str())); if (send_func_ == nullptr) { _E("Failed to find symbol(%s)", symbol.c_str()); /* LCOV_EXCL_LINE */ - return false; /* LCOV_EXCL_LINE */ + return false; /* LCOV_EXCL_LINE */ } } @@ -224,29 +225,29 @@ bool LocalExecution::LoadSymbols() { } void LocalExecution::ResultQueuePush(rpc_port_parcel_h parcel) { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_q_); result_queue_.push(parcel); } rpc_port_parcel_h LocalExecution::ResultQueuePop() { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_q_); auto parcel = result_queue_.front(); result_queue_.pop(); return parcel; } bool LocalExecution::ResultQueueEmpty() const { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_q_); return result_queue_.empty(); } void LocalExecution::RequestQueuePush(rpc_port_parcel_h parcel) { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_q_); request_queue_.push(parcel); } rpc_port_parcel_h LocalExecution::RequestQueuePop() { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_q_); auto parcel = request_queue_.front(); request_queue_.pop(); return parcel; @@ -359,6 +360,10 @@ EventSystem::EventSystem(EventSystem::IEventListener* listener, EventSystem::~EventSystem() { _W("EventSystem dtor"); + if (local_execution_.get() != nullptr && local_execution_->IsConnected()) { + local_execution_->Disconnect(); + } + if (proxy_ != nullptr) rpc_port_proxy_destroy(proxy_); } @@ -787,7 +792,7 @@ bool EventSystem::GetEarlierData(std::string event_name, Bundle& data) { } void EventSystem::AddAsyncEventList(std::string event_name, - std::unique_ptr event) { + std::unique_ptr event) { std::lock_guard lock(mutex_); async_event_list_.push_back(std::pair(event_name, std::move(event))); } @@ -795,7 +800,8 @@ void EventSystem::AddAsyncEventList(std::string event_name, bool EventSystem::RemoveAsyncEventList(int id) { bool found = false; std::lock_guard lock(mutex_); - for (auto it = async_event_list_.begin(); it != async_event_list_.end(); ++it) { + for (auto it = async_event_list_.begin(); it != async_event_list_.end(); + ++it) { if (it->second->GetSeqId() == id) { async_event_list_.erase(it); found = true; @@ -813,6 +819,12 @@ int EventSystem::GetSizeAsyncEventList() { void EventSystem::OnLocalConnected() { if (listener_) listener_->OnConnected(); + + std::lock_guard lock(mutex_); + for (auto& i : async_event_list_) + RegisterEvent(EventSystem::EventType::User, i.first, + std::move(i.second)); + async_event_list_.clear(); } void EventSystem::OnLocalDisconnected() { @@ -877,8 +889,8 @@ void EventSystem::OnConnectedCb(const char* endpoint, const char* port_name, std::lock_guard lock(handle->mutex_); for (auto& i : handle->async_event_list_) - handle->RegisterEvent(EventSystem::EventType::User, - i.first, std::move(i.second)); + handle->RegisterEvent(EventSystem::EventType::User, i.first, + std::move(i.second)); handle->async_event_list_.clear(); } @@ -927,21 +939,19 @@ void EventSystem::OnReceivedCb(const char* endpoint, const char* port_name, EXPORT_API int rpc_port_proxy_eventsystem_lem_EventSystem_connect(void* h, bool sync) { auto* handle = static_cast(h); - handle->OnConnected(); - return RPC_PORT_ERROR_NONE; -} + if (sync) { + handle->OnConnected(); + return RPC_PORT_ERROR_NONE; + } -EXPORT_API int rpc_port_proxy_eventsystem_lem_EventSystem_disconnect(void* h) { - auto* handle = static_cast(h); + /* LCOV_EXCL_START */ auto* ptr = new std::weak_ptr( handle->shared_from_this()); auto* source = g_idle_source_new(); if (source == nullptr) { - /* LCOV_EXCL_START */ _E("Failed to create idle source"); delete ptr; return RPC_PORT_ERROR_OUT_OF_MEMORY; - /* LCOV_EXCL_STOP */ } g_source_set_callback( @@ -950,7 +960,7 @@ EXPORT_API int rpc_port_proxy_eventsystem_lem_EventSystem_disconnect(void* h) { static_cast*>( user_data); auto p = wp->lock(); - if (p != nullptr) p->OnDisconnected(); + if (p != nullptr) p->OnConnected(); delete wp; return G_SOURCE_REMOVE; @@ -958,6 +968,13 @@ EXPORT_API int rpc_port_proxy_eventsystem_lem_EventSystem_disconnect(void* h) { ptr, nullptr); g_source_attach(source, handle->GetContext()); g_source_unref(source); + + return RPC_PORT_ERROR_NONE; +} + +EXPORT_API int rpc_port_proxy_eventsystem_lem_EventSystem_disconnect(void* h) { + auto* handle = static_cast(h); + handle->OnDisconnected(); return RPC_PORT_ERROR_NONE; } diff --git a/src/es_proxy.h b/src/es_proxy.h index 679ec4b..a6fb0fe 100644 --- a/src/es_proxy.h +++ b/src/es_proxy.h @@ -87,6 +87,7 @@ class LocalExecution : public std::enable_shared_from_this { std::queue result_queue_; std::queue request_queue_; mutable std::recursive_mutex mutex_; + mutable std::recursive_mutex mutex_q_; std::unique_ptr context_; }; diff --git a/tests/integ_tests/eventsystem_test.cc b/tests/integ_tests/eventsystem_test.cc index 508a76d..592f54f 100644 --- a/tests/integ_tests/eventsystem_test.cc +++ b/tests/integ_tests/eventsystem_test.cc @@ -204,3 +204,37 @@ TEST_F(EventSystemTest, eventsystem_application_finalize) { int ret = eventsystem_application_finalize(); EXPECT_EQ(ret, ES_R_OK); } + +TEST_F(EventSystemTest, eventsystem_register_application_event3) { + unsigned int id; + int type; + static GMainLoop* loop; + static std::string g_event_name; + + g_event_name = ""; + loop = g_main_loop_new(nullptr, FALSE); + int ret = eventsystem_register_application_event( + "event.signal_agent.tidl_iface_PkgSignal.test", &id, &type, + [](const char* event_name, bundle_raw* event_data, int len, + void* user_data) { + g_event_name = event_name; + g_main_loop_quit(loop); + }, + nullptr); + EXPECT_EQ(ret, ES_R_OK); + + g_timeout_add( + 1000, + [](auto* l) { + tizen_base::Bundle data; + eventsystem_send_system_event( + "event.signal_agent.tidl_iface_PkgSignal.test", data.GetHandle()); + return G_SOURCE_REMOVE; + }, + loop); + + g_main_loop_run(loop); + + EXPECT_EQ(g_event_name, "event.signal_agent.tidl_iface_PkgSignal.test"); + g_main_loop_unref(loop); +} diff --git a/tests/unit_tests/eventsystem_test.cc b/tests/unit_tests/eventsystem_test.cc index 80d0975..428caf3 100644 --- a/tests/unit_tests/eventsystem_test.cc +++ b/tests/unit_tests/eventsystem_test.cc @@ -1084,3 +1084,38 @@ TEST_F(EventSystemTest, eventsystem_launch_event_user_event2) { EXPECT_EQ(g_event_name, "event.d::org.tizen.appfw.service.esd.test"); g_main_loop_unref(loop); } + +TEST_F(EventSystemTest, eventsystem_register_application_event3) { + unsigned int id; + int type; + static GMainLoop* loop; + static std::string g_event_name; + + RunModule(); + g_event_name = ""; + loop = g_main_loop_new(nullptr, FALSE); + int ret = eventsystem_register_application_event( + "event.signal_agent.tidl_iface_PkgSignal.test", &id, &type, + [](const char* event_name, bundle_raw* event_data, int len, + void* user_data) { + g_event_name = event_name; + g_main_loop_quit(loop); + }, + nullptr); + EXPECT_EQ(ret, ES_R_OK); + + g_timeout_add( + 1000, + [](auto* l) { + tizen_base::Bundle data; + eventsystem_send_system_event( + "event.signal_agent.tidl_iface_PkgSignal.test", data.GetHandle()); + return G_SOURCE_REMOVE; + }, + loop); + + g_main_loop_run(loop); + + EXPECT_EQ(g_event_name, "event.signal_agent.tidl_iface_PkgSignal.test"); + g_main_loop_unref(loop); +}