Fix accessing deleted object using shared_ ptr

Change-Id: I0d05cf38ce457c30dc84f8e86e5f37576b95452c
Signed-off-by: pjh9216 <jh9216.park@samsung.com>

diff --git a/src/es.cc b/src/es.cc
index 1ce6becea156bd9f26b7db430330ae042621265d..28d2a366cd61ed738feb047f2e504e6c8873cf0b 100644 (file)
--- 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<rpc_proxy::EventSystem> es_proxy_;
+std::future<void> connection_fut_;
+
+std::shared_ptr<rpc_proxy::EventSystem> 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<void> 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<rpc_proxy::EventSystem::EventListener>(
         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 cefc1f4a1d7c909c3084bc0d9131aa2e76ff3961..784594a8287365e9b6f2df91589cf2f620bde0b9 100644 (file)
--- 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<std::recursive_mutex> lock(mutex_q_);
   while (!result_queue_.empty()) {
     auto parcel = result_queue_.front();
     result_queue_.pop();
@@ -193,7 +194,7 @@ bool LocalExecution::LoadSymbols() {
         reinterpret_cast<StubConnectFunc>(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<StubSendFunc>(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<std::recursive_mutex> lock(mutex_);
+  std::lock_guard<std::recursive_mutex> lock(mutex_q_);
   result_queue_.push(parcel);
 }
 
 rpc_port_parcel_h LocalExecution::ResultQueuePop() {
-  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  std::lock_guard<std::recursive_mutex> lock(mutex_q_);
   auto parcel = result_queue_.front();
   result_queue_.pop();
   return parcel;
 }
 
 bool LocalExecution::ResultQueueEmpty() const {
-  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  std::lock_guard<std::recursive_mutex> lock(mutex_q_);
   return result_queue_.empty();
 }
 
 void LocalExecution::RequestQueuePush(rpc_port_parcel_h parcel) {
-  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  std::lock_guard<std::recursive_mutex> lock(mutex_q_);
   request_queue_.push(parcel);
 }
 
 rpc_port_parcel_h LocalExecution::RequestQueuePop() {
-  std::lock_guard<std::recursive_mutex> lock(mutex_);
+  std::lock_guard<std::recursive_mutex> 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<EventListener> event) {
+                                    std::unique_ptr<EventListener> event) {
   std::lock_guard<std::recursive_mutex> 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<std::recursive_mutex> 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<std::recursive_mutex> 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<std::recursive_mutex> 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<rpc_port::es_proxy::LocalExecution*>(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<rpc_port::es_proxy::LocalExecution*>(h);
+  /* LCOV_EXCL_START */
   auto* ptr = new std::weak_ptr<rpc_port::es_proxy::LocalExecution>(
       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<std::weak_ptr<rpc_port::es_proxy::LocalExecution>*>(
                 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<rpc_port::es_proxy::LocalExecution*>(h);
+  handle->OnDisconnected();
   return RPC_PORT_ERROR_NONE;
 }
 

diff --git a/src/es_proxy.h b/src/es_proxy.h
index 679ec4b9254ebbb04567f68a7404412106f57688..a6fb0fef6e38c18bf73e1948ccaa4c7d6339eabc 100644 (file)
--- a/src/es_proxy.h
+++ b/src/es_proxy.h
@@ -87,6 +87,7 @@ class LocalExecution : public std::enable_shared_from_this<LocalExecution> {
   std::queue<rpc_port_parcel_h> result_queue_;
   std::queue<rpc_port_parcel_h> request_queue_;
   mutable std::recursive_mutex mutex_;
+  mutable std::recursive_mutex mutex_q_;
   std::unique_ptr<GMainContext, decltype(g_main_context_unref)*> context_;
 };
 

diff --git a/tests/integ_tests/eventsystem_test.cc b/tests/integ_tests/eventsystem_test.cc
index 508a76dc02413f7312566c986d28b412d63bff9d..592f54f131f65966045da52b34970d1de54a45a2 100644 (file)
--- 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 80d0975f442d5d15f65027cc7c3ab692175e1508..428caf3c4198cac96a6eabf61ea2145bcf6a2f4c 100644 (file)
--- 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);
+}