From: Piotr Kosko/Native/Web API (PLT) /SRPOL/Engineer/Samsung Electronics <p.kosko@samsung.com>
Date: Tue, 26 May 2020 09:07:42 +0000 (+0200)
Subject: [Application] Fixed behaviour of clearing data for reply callbacks
X-Git-Tag: submit/tizen_5.5/20200526.102615~2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9b23bb424ce2975261a84590abcd273cdca3ee51;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git

[Application] Fixed behaviour of clearing data for reply callbacks

[Bug] Issue reported by PLM P200525-06256
When application suspends the reply callback, e.g. because of waiting for
user interaction, the data for callback was released too early. This caused
that everytime when reply callback is delayed, data was already freed and it
caused crash of application.

[Fix] Issue was fixed with saving pointers for releasing during manager destruction.

[Verification] PLM application has no longer any issues.
TCT passrate is 100%.

Change-Id: I575ab5f9ce4ccd0ce0bff0e6e9bdc15cadc09ed5
---

diff --git a/src/application/application_manager.cc b/src/application/application_manager.cc
index af0f9cb1..4de3140f 100644
--- a/src/application/application_manager.cc
+++ b/src/application/application_manager.cc
@@ -134,6 +134,16 @@ ApplicationManager::~ApplicationManager() {
       LoggerE("app_manager_event_destroy failed, error: %d", ret);
     }
   }
+
+  // Calling order of reply_callback and result_callback of launchAppControl
+  // cannot be determined, reply_callback depends on application scenario, which
+  // cannot be predicted. Moreover reply_callback is optional and can be called or not,
+  // thus callback_data is relesed here at the end of application lifetime.
+  std::for_each(launch_app_control_set.begin(), launch_app_control_set.end(),
+                [](LaunchAppControlCallbackData* const& data) {
+                  LoggerD("Releasing callback data: %p", data);
+                  delete data;
+                });
 }
 
 void ApplicationManager::GetCurrentApplication(const std::string& app_id, picojson::object* out) {
@@ -453,11 +463,7 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
     reply_callback_id = reply.get<std::string>();
   }
 
-  struct LaunchAppControlCallbackData {
-    ApplicationInstance* instance;
-    std::shared_ptr<picojson::value> response;
-    std::string reply_callback_id;
-  }* launch_app_user_data = new (std::nothrow)
+  LaunchAppControlCallbackData* launch_app_user_data = new (std::nothrow)
       LaunchAppControlCallbackData{&this->instance_, response, reply_callback_id};
 
   if (!launch_app_user_data) {
@@ -500,8 +506,8 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
 
       Instance::PostMessage(callback_data->instance, return_value.serialize().c_str());
       // Calling order of reply_callback and result_callback cannot be determined,
-      // thus callback_data is not released here - it is always
-      // released in result_callback with delay to prevent using a released memory.
+      // thus callback_data is not released here, but stored for release in destructor of
+      // ApplicationManager
     };
   }
 
@@ -523,14 +529,8 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
     Instance::PostMessage(callback_data->instance, callback_data->response->serialize().c_str());
 
     // Calling order of reply_callback and result_callback cannot be determined,
-    // thus callback_data is released here with delay to prevent using a released
-    // memory in reply_callback function.
-    int timeout = 10;
-    std::thread([callback_data, timeout]() {
-      std::this_thread::sleep_for(std::chrono::seconds(timeout));
-      LoggerD("Deleting callback_data: %p", callback_data);
-      delete callback_data;
-    }).detach();
+    // thus callback_data is not released here, but stored for release in destructor of
+    // ApplicationManager
   };
 
   /*
@@ -548,10 +548,15 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
     delete launch_app_user_data;
     AsyncResponse(launch_result, &response);
   } else {
-    LoggerD("App launched");
+    // Calling order of reply_callback and result_callback cannot be determined,
+    // reply_callback depends on application scenario, which cannot be predicted.
+    // Moreover reply_callback is optional and can be called or not,
+    // thus callback_data is stored for releasing in destructor of ApplicationManager
+    // at the end of application lifetime.
+    launch_app_control_set.insert(launch_app_user_data);
+    LoggerD("App launched, data %p stored for later release", launch_app_user_data);
   }
 }
-
 namespace {
 
 PlatformResult TranslateLaunchError(app_control_error_e return_code) {
diff --git a/src/application/application_manager.h b/src/application/application_manager.h
index be0ebbf6..5a9d123b 100644
--- a/src/application/application_manager.h
+++ b/src/application/application_manager.h
@@ -23,8 +23,8 @@
 #include <package-manager.h>
 #include <pkgmgr-info.h>
 #include <functional>
-#include <functional>
 #include <memory>
+#include <set>
 #include <string>
 #if defined(TIZEN_MOBILE)
 #include <context-service/context_history.h>
@@ -36,11 +36,15 @@
 #include "common/platform_result.h"
 
 typedef std::function<void(picojson::value*)> JsonCallback;
-
 namespace extension {
 namespace application {
 
 class ApplicationInstance;
+typedef struct LaunchAppControlCallbackDataStruct {
+  ApplicationInstance* instance;
+  std::shared_ptr<picojson::value> response;
+  std::string reply_callback_id;
+} LaunchAppControlCallbackData;
 
 class ApplicationManager {
  public:
@@ -85,6 +89,7 @@ class ApplicationManager {
   JsonCallback event_callback_;
   JsonCallback status_callback_;
   std::map<std::string, event_handler_h> event_handler_map_;
+  std::set<LaunchAppControlCallbackData*> launch_app_control_set;
   static void OnEvent(const char* event_name, bundle* event_data, void* user_data);
   static void OnStatusEvent(const char* type, const char* app_id,
                             app_manager_event_type_e event_type,