From: Piotr Kosko <p.kosko@samsung.com>
Date: Thu, 5 Nov 2020 11:20:21 +0000 (+0100)
Subject: [NBS] Add validation of instance to prevent crash
X-Git-Tag: submit/tizen/20201209.085808~7
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2a877c12b9031ed302f91f919ab0cd93b1c10ce7;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git

[NBS] Add validation of instance to prevent crash

[Bug] In case of destroying instance before the operation:
* requestRouteToHost
* releaseRouteToHost
is finished, invalidated instance was used in code.

[Reproduction] In my case it was execution of below code max 10 times:
function crashApplicationWithNBS() {
    try {
        tizen.networkbearerselection.releaseRouteToHost(
            "CELLULAR", "www.onet.pl", addRouteAndReload, addRouteAndReload);
    } catch (e) {
        addRouteAndReload();
    }
}

function addRouteAndReload() {
    var statuscb =
    {
        onsuccess: function () {
            console.log("onsuccess func is called");
        },
        ondisconnected: function () {
            console.log("ondisconnected func is called");
        }
    };
    function ecb(e) {
        console.log("error callback is called: " + e.message + ": " + e.code);
    }
    setTimeout(() => {
        try {
            tizen.networkbearerselection.requestRouteToHost(
                "CELLULAR", "www.onet.pl", statuscb, ecb)
        } catch (e) { console.log(e) }
    }, 500);
    setTimeout(() => { location.href = "index.html"; }, 490);
}

[Verification]
* Crash does not occur. Crash situation is only signalled
with log:
DoAndPostMessage(264) > Trying to post message to non-existing instance: [0xb8cb1730], ignoring
* Success and error callbacks are properly triggered

Change-Id: I6adac13a8a99dbc114102bbd2ffced6bf012cfce
---

diff --git a/src/common/extension.cc b/src/common/extension.cc
index 713e03e6..6a0a2c3a 100644
--- a/src/common/extension.cc
+++ b/src/common/extension.cc
@@ -238,6 +238,20 @@ Instance::~Instance() {
   Assert(xw_instance_ == 0);
 }
 
+void Instance::DoAndPostMessage(Instance* that, const std::function<void()>& work,
+                                const picojson::value& json) {
+  ScopeLogger();
+  if (nullptr != that) {
+    std::lock_guard<std::mutex> lock{instances_mutex_};
+    if (all_instances_.end() != all_instances_.find(that)) {
+      work();
+      that->PostMessage(json.serialize().c_str());
+      return;
+    }
+  }
+  LoggerE("Trying to post message to non-existing instance: [%p], ignoring", that);
+}
+
 void Instance::PostMessage(Instance* that, const char* msg) {
   ScopeLogger();
   if (nullptr != that) {
diff --git a/src/common/extension.h b/src/common/extension.h
index f9e0e68f..f35c8226 100644
--- a/src/common/extension.h
+++ b/src/common/extension.h
@@ -93,6 +93,7 @@ class Instance {
   Instance();
   virtual ~Instance();
 
+  static void DoAndPostMessage(Instance* that, const std::function<void()>& work, const picojson::value& json);
   static void PostMessage(Instance* that, const char* msg);
   static void PostMessage(Instance* that, const picojson::value& json);
   void PostMessage(const char* msg);
diff --git a/src/networkbearerselection/networkbearerselection_instance.cc b/src/networkbearerselection/networkbearerselection_instance.cc
index f4175fef..d7e1dd0b 100644
--- a/src/networkbearerselection/networkbearerselection_instance.cc
+++ b/src/networkbearerselection/networkbearerselection_instance.cc
@@ -86,21 +86,22 @@ void NetworkBearerSelectionInstance::NetworkBearerSelectionRequestRouteToHost(
   auto request = [=]() -> void {
     auto response = [this, domain_name, id](const common::PlatformResult result) -> void {
       ScopeLogger();
-
       picojson::value value{picojson::object{}};
       picojson::object& obj = value.get<picojson::object>();
 
       obj["id"] = picojson::value(static_cast<double>(id));
       obj["listenerId"] = picojson::value(kNBSCallback + std::to_string(id));
 
-      if (result.IsSuccess()) {
-        ReportSuccess(obj);
-        this->addDomainListener(domain_name, id);
-      } else {
-        LogAndReportError(result, &obj);
-      }
-
-      Instance::PostMessage(this, value.serialize().c_str());
+      Instance::DoAndPostMessage(this,
+                                 [this, &domain_name, &id, &result, &obj]() -> void {
+                                   if (result.IsSuccess()) {
+                                     ReportSuccess(obj);
+                                     this->addDomainListener(domain_name, id);
+                                   } else {
+                                     LogAndReportError(result, &obj);
+                                   }
+                                 },
+                                 value);
     };
 
     NetworkBearerSelectionManager::GetInstance()->requestRouteToHost(domain_name, response);
@@ -138,20 +139,21 @@ void NetworkBearerSelectionInstance::NetworkBearerSelectionReleaseRouteToHost(
   auto release = [=]() -> void {
     auto response = [this, domain_name, id](const common::PlatformResult result) -> void {
       ScopeLogger();
-
       picojson::value value{picojson::object{}};
       picojson::object& obj = value.get<picojson::object>();
 
       obj["callbackId"] = picojson::value(id);
 
-      if (result.IsSuccess()) {
-        ReportSuccess(obj);
-        this->removeDomainListener(domain_name);
-      } else {
-        LogAndReportError(result, &obj);
-      }
-
-      Instance::PostMessage(this, value.serialize().c_str());
+      Instance::DoAndPostMessage(this,
+                                 [this, &domain_name, &result, &obj]() -> void {
+                                   if (result.IsSuccess()) {
+                                     ReportSuccess(obj);
+                                     this->removeDomainListener(domain_name);
+                                   } else {
+                                     LogAndReportError(result, &obj);
+                                   }
+                                 },
+                                 value);
     };
 
     NetworkBearerSelectionManager::GetInstance()->releaseRouteToHost(domain_name, response);