[Application] Release data pointers as soon as possible

Result and reply callback share user_data and the moment of releasing it
can be difficult to determine.

[Bug] Previous implementation was just holding all pointers through the
application lifetime, but this can be become a problem for applications
working continuously for really long time and starting other
applications with AppControl.

[JIRA] https://jira.sec.samsung.net/browse/TDAF-2090

[Fix] Current implementation makes few assumptions:
1. If Web Application doesn't set reply callback just assume that data
   need to be released in result callback.
2. Result callback and reply callback are always called on main
   application thread, thus there is no concurrency issues.
3. If reply callback is set, reference count is done to control the data
   release and data is released after callee will respond with data.
4. If callee doesn't respond to caller as expected, data handler would
   stay as 'pending' in memory and released during application closure.

[Verification] Code compiles without errors.
TCT application passrate 100% (M + A)

Additional verification scenarios with Callee application starting
Callee application, which respond with one of below options:
        var data = new tizen.ApplicationControlData(
	        "test", ["test"]);
        // scenario 1 - reply once
        reqAppControl.replyResult([data]);

        // scenario 2 - call reply twice
	reqAppControl.replyResult([data]);
	reqAppControl.replyResult([data]);

	// scenario 3 - no reply

	// scenario 4 - delay
	setTimeout(function() {
		reqAppControl.replyResult([data]);
	}, 10000);

	// scenario 5 - instant reply and delayed
	reqAppControl.replyResult([data]);
	setTimeout(function() {
		reqAppControl.replyResult([data]);
	}, 5000);

	// scenario 6 - two delayed replies
	setTimeout(function() {
		reqAppControl.replyResult([data]);
	}, 5000);
	setTimeout(function() {
		reqAppControl.replyResult([data]);
	}, 10000);

        // Failure cases:
        // scenario 1 - reply once
        reqAppControl.replyFailure();

        // scenario 2 - call reply twice
	reqAppControl.replyFailure();
	reqAppControl.replyFailure();

	// scenario 3 - no reply

	// scenario 4 - delay
	setTimeout(function() {
		reqAppControl.replyFailure();
	}, 5000);

	// scenario 5 - instant reply and delayed
	reqAppControl.replyFailure();
	setTimeout(function() {
		reqAppControl.replyFailure();
	}, 5000);

	// scenario 6 - two delayed replies
	setTimeout(function() {
		reqAppControl.replyFailure();
	}, 5000);
	setTimeout(function() {
		reqAppControl.replyFailure();
	}, 10000);

Additionally checked scenarios with:
1. Scenario without optional reply callback - data released in result callback
2. Scenario with callee application being closed before receiving reply
   callback (instantly after launchAppControl() call) - data released in
destructor and properly handled in callbacks as 'not valid'
3. Scenario without a reply on callee side, caller keeps handle till the
   destructor and then just releases memory properly

Change-Id: If04d7fa488f416a2964303d3179d351fadf6f593

diff --git a/src/application/application_manager.cc b/src/application/application_manager.cc
index c8c7c259c7eddc4d6c6d24ac267cf0ac94abfcc6..2deeff7ebc6d3fe510bffd31a455b11505fd7546 100644 (file)
--- a/src/application/application_manager.cc
+++ b/src/application/application_manager.cc
@@ -140,14 +140,16 @@ ApplicationManager::~ApplicationManager() {
   // 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.
+  // thus pending callback_data is relesed here at the end of application lifetime.
   std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
+  LoggerD("Releasing pending handles");
   std::for_each(launch_app_control_set_.begin(), launch_app_control_set_.end(),
                 [](LaunchAppControlCallbackData* const& data) {
                   LoggerD("Releasing callback data: %p", data);
                   delete data;
                   launch_app_control_set_global_.erase(data);
                 });
+  LoggerD("Pending LaunchAppControlCallbackData - this instance: %u, globally: %u", launch_app_control_set_.size(), launch_app_control_set_global_.size());
 }
 
 void ApplicationManager::GetCurrentApplication(const std::string& app_id, picojson::object* out) {
@@ -437,6 +439,19 @@ PlatformResult PrepareAppControlForLaunchAppControl(const picojson::value& args,
 
 }  // namespace
 
+void ApplicationManager::TryRelease(LaunchAppControlCallbackData* callback_data) {
+  if (0 == --callback_data->reference_count) {
+    LoggerD("Releasing callback_data: %p", callback_data);
+    std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
+    launch_app_control_set_global_.erase(callback_data);
+    callback_data->manager->launch_app_control_set_.erase(callback_data);
+    delete callback_data;
+  } else {
+    LoggerD("NOT Releasing callback_data: %p, only decremented reference count to: %u", callback_data, callback_data->reference_count);
+    LoggerD("Pending LaunchAppControlCallbackData - this instance: %u, globally: %u", callback_data->manager->launch_app_control_set_.size(), launch_app_control_set_global_.size());
+  }
+}
+
 void ApplicationManager::LaunchAppControl(const picojson::value& args) {
   ScopeLogger();
 
@@ -468,7 +483,7 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
   }
 
   LaunchAppControlCallbackData* launch_app_user_data = new (std::nothrow)
-      LaunchAppControlCallbackData{&this->instance_, response, reply_callback_id};
+      LaunchAppControlCallbackData{&this->instance_, this, response, reply_callback_id, 1};
 
   if (!launch_app_user_data) {
     LoggerE("Memory allocation fail!");
@@ -479,6 +494,12 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
   app_control_reply_cb reply_callback = nullptr;
   if (!reply_callback_id.empty()) {
     launch_app_user_data->reply_callback_id = reply_callback_id;
+    // reply_callback depends on application scenario, which cannot be predicted,
+    // moreover reply_callback is optional and can be called or not.
+    // If Web API user define reply callback, we assume that callee application will
+    // send reply callback to caller and cause proper data release, otherwise there will
+    // be leak of memory.
+    launch_app_user_data->reference_count++;
 
     reply_callback = [](app_control_h request, app_control_h reply, app_control_result_e result,
                         void* user_data) {
@@ -487,10 +508,13 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
       LaunchAppControlCallbackData* callback_data =
           static_cast<LaunchAppControlCallbackData*>(user_data);
 
-      std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
-      if (!launch_app_control_set_global_.count(callback_data)) {
-        LoggerE("Invalid callback_data: %p", callback_data);
-        return;
+      {
+        std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
+        if (!launch_app_control_set_global_.count(callback_data)) {
+          LoggerE("Callback data: %p refers to already released instance", callback_data);
+          // Not releasing callback_data here - it was already released in destructor
+          return;
+        }
       }
 
       picojson::value return_value = picojson::value(picojson::object());
@@ -511,9 +535,7 @@ 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, but stored for release in destructor of
-      // ApplicationManager
+      TryRelease(callback_data);
     };
   }
 
@@ -524,10 +546,13 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
     LaunchAppControlCallbackData* callback_data =
         static_cast<LaunchAppControlCallbackData*>(user_data);
 
-    std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
-    if (!launch_app_control_set_global_.count(callback_data)) {
-      LoggerE("Invalid callback_data: %p", callback_data);
-      return;
+    {
+      std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);
+      if (!launch_app_control_set_global_.count(callback_data)) {
+        LoggerE("Callback data: %p refers to already released instance", callback_data);
+        // Not releasing callback_data here - it was already released in destructor
+        return;
+      }
     }
 
     auto result = utils::TranslateAppControlError(launch_result);
@@ -540,9 +565,7 @@ 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 not released here, but stored for release in destructor of
-    // ApplicationManager
+    TryRelease(callback_data);
   };
 
   /*
@@ -560,12 +583,12 @@ void ApplicationManager::LaunchAppControl(const picojson::value& args) {
     delete launch_app_user_data;
     AsyncResponse(launch_result, &response);
   } else {
-    // 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.
-    // We can not be sure whether callbach will not fire after instance destruction,
+    // reply_callback depends on application scenario, which cannot be predicted,
+    // moreover reply_callback is optional and can be called or not.
+    // If Web API user define reply callback, we assume that callee application will
+    // send reply callback to caller and cause proper data release, otherwise there will
+    // be leak of memory.
+    // We can not be sure whether callback will not fire after instance destruction,
     // we store all user_data pointers in static launch_app_control_set_global_ to check
     // if they are still valid.
     std::lock_guard<std::mutex> lock(launch_app_control_set_mutex_);

diff --git a/src/application/application_manager.h b/src/application/application_manager.h
index a7485ce0f689eaff334bc056384f308de1b579c8..73bb748c59d74661a3b1032ea8b7384dd1ca6853 100644 (file)
--- a/src/application/application_manager.h
+++ b/src/application/application_manager.h
@@ -41,10 +41,17 @@ namespace extension {
 namespace application {
 
 class ApplicationInstance;
+class ApplicationManager;
 typedef struct LaunchAppControlCallbackDataStruct {
   ApplicationInstance* instance;
+  ApplicationManager* manager;
   std::shared_ptr<picojson::value> response;
   std::string reply_callback_id;
+  // reference_count is used for determining the data release time, as reply_callback is optional,
+  // reference_count is by default equal to 1, but in case if WebAPI user register reply_callback,
+  // then it is incremented. Data release design assumes that the data is released after all callbacks
+  // are executed and reference count is decreased to 0.
+  size_t reference_count;
 } LaunchAppControlCallbackData;
 
 class ApplicationManager {
@@ -95,6 +102,7 @@ class ApplicationManager {
   // callback is fired after ApplicationInstance destruction
   static std::set<LaunchAppControlCallbackData*> launch_app_control_set_global_;
   static std::mutex launch_app_control_set_mutex_;
+  static void TryRelease(LaunchAppControlCallbackData* callback_data);
   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,