fix(workers): Dispose the isolate when terminating workers (#1550)

Author: darind

test-app/runtime/src/main/cpp/JSONObjectHelper.cpp

@@ -7,33 +7,54 @@
 using namespace v8;
 using namespace tns;
 
-JSONObjectHelper::JSONObjectHelper()
-    : m_objectManager(nullptr), m_serializeFunc(nullptr) {
-}
-
-void JSONObjectHelper::CreateConvertFunctions(Isolate *isolate, const Local<Object> &global, ObjectManager* objectManager) {
-    m_objectManager = objectManager;
+void JSONObjectHelper::RegisterFromFunction(Isolate *isolate, Local<Value>& jsonObject) {
+    if (!jsonObject->IsFunction()) {
+        return;
+    }
 
-    m_serializeFunc = new Persistent<Function>(isolate, CreateSerializeFunc(isolate));
+    Isolate::Scope isolate_scope(isolate);
+    HandleScope handle_scope(isolate);
 
     Local<Context> context = isolate->GetCurrentContext();
+    Context::Scope context_scope(context);
 
-    Local<External> extData = External::New(isolate, this);
-    Local<Function> fromFunc = FunctionTemplate::New(isolate, ConvertCallbackStatic, extData)->GetFunction(context).ToLocalChecked();
-
-    Local<Function> jsonObjectFunc = global->Get(context, ArgConverter::ConvertToV8String(isolate, "org"))
-            .ToLocalChecked().As<Object>()->Get(context, ArgConverter::ConvertToV8String(isolate, "json"))
-            .ToLocalChecked().As<Object>()->Get(context, ArgConverter::ConvertToV8String(isolate, "JSONObject"))
-            .ToLocalChecked().As<Function>();
+    Local<Function> jsonObjectFunc =  jsonObject.As<Function>();
+    auto fromKey = ArgConverter::ConvertToV8String(isolate, "from");
+    if (jsonObjectFunc->Has(context, fromKey).FromMaybe(false)) {
+        return;
+    }
 
-    jsonObjectFunc->Set(context, ArgConverter::ConvertToV8String(isolate, "from"), fromFunc);
+    Persistent<Function>* serializeFunc = new Persistent<Function>(isolate, CreateSerializeFunc(context));
+    Local<External> extData = External::New(isolate, serializeFunc);
+    Local<Function> fromFunc;
+    bool ok = FunctionTemplate::New(isolate, ConvertCallbackStatic, extData)->GetFunction(context).ToLocal(&fromFunc);
+    assert(ok);
+    jsonObjectFunc->Set(context, fromKey, fromFunc);
 }
 
 void JSONObjectHelper::ConvertCallbackStatic(const FunctionCallbackInfo<Value>& info) {
     try {
         Local<External> extData = info.Data().As<External>();
-        auto thiz = reinterpret_cast<JSONObjectHelper*>(extData->Value());
-        thiz->ConvertCallback(info);
+        auto poSerializeFunc = reinterpret_cast<Persistent<Function>*>(extData->Value());
+        Isolate* isolate = info.GetIsolate();
+        Local<Function> serializeFunc = poSerializeFunc->Get(isolate);
+
+        if (info.Length() < 1) {
+            NativeScriptException nsEx(std::string("The \"from\" function expects one parameter"));
+            nsEx.ReThrowToV8();
+            return;
+        }
+
+        Local<Context> context = isolate->GetCurrentContext();
+
+        Local<Value> args[] = { info[0] };
+        Local<Value> result;
+        TryCatch tc(isolate);
+        if (!serializeFunc->Call(context, Undefined(isolate), 1, args).ToLocal(&result)) {
+            throw NativeScriptException(tc, "Error serializing to JSONObject");
+        }
+
+        info.GetReturnValue().Set(result);
     } catch (NativeScriptException& e) {
         e.ReThrowToV8();
     } catch (std::exception e) {
@@ -47,28 +68,7 @@ void JSONObjectHelper::ConvertCallbackStatic(const FunctionCallbackInfo<Value>&
     }
 }
 
-void JSONObjectHelper::ConvertCallback(const FunctionCallbackInfo<Value>& info) {
-    if (info.Length() < 1) {
-        NativeScriptException nsEx(std::string("The \"from\" function expects one parameter"));
-        nsEx.ReThrowToV8();
-        return;
-    }
-
-    Isolate* isolate = info.GetIsolate();
-    Local<Context> context = isolate->GetCurrentContext();
-
-    Local<Function> serializeFunc = m_serializeFunc->Get(isolate);
-    Local<Value> args[] = { info[0] };
-    Local<Value> result;
-    TryCatch tc(isolate);
-    if (!serializeFunc->Call(context, Undefined(isolate), 1, args).ToLocal(&result)) {
-        throw NativeScriptException(tc, "Error serializing to JSONObject");
-    }
-
-    info.GetReturnValue().Set(result);
-}
-
-Local<Function> JSONObjectHelper::CreateSerializeFunc(Isolate* isolate) {
+Local<Function> JSONObjectHelper::CreateSerializeFunc(Local<Context> context) {
     std::string source =
         "(() => function serialize(data) {"
         "    let store;"
@@ -102,7 +102,7 @@ Local<Function> JSONObjectHelper::CreateSerializeFunc(Isolate* isolate) {
         "    }"
         "})()";
 
-    Local<Context> context = isolate->GetCurrentContext();
+    Isolate* isolate = context->GetIsolate();
 
     Local<Script> script = Script::Compile(context, ArgConverter::ConvertToV8String(isolate, source)).ToLocalChecked();
 

test-app/runtime/src/main/cpp/JSONObjectHelper.h

@@ -2,20 +2,14 @@
 #define JSONOBJECTHELPER_H_
 
 #include "v8.h"
-#include "ObjectManager.h"
 
 namespace tns {
 
 class JSONObjectHelper {
 public:
-    JSONObjectHelper();
-    void CreateConvertFunctions(v8::Isolate* isolate, const v8::Local<v8::Object>& global, ObjectManager* objectManager);
+    static void RegisterFromFunction(v8::Isolate *isolate, v8::Local<v8::Value>& jsonObject);
 private:
-    ObjectManager* m_objectManager;
-    v8::Persistent<v8::Function>* m_serializeFunc;
-
-    v8::Local<v8::Function> CreateSerializeFunc(v8::Isolate* isolate);
-    void ConvertCallback(const v8::FunctionCallbackInfo<v8::Value>& info);
+    static v8::Local<v8::Function> CreateSerializeFunc(v8::Local<v8::Context> context);
     static void ConvertCallbackStatic(const v8::FunctionCallbackInfo<v8::Value>& info);
 };
 

test-app/runtime/src/main/cpp/MetadataNode.cpp

@@ -13,6 +13,7 @@
 #include <cctype>
 #include <dirent.h>
 #include "ManualInstrumentation.h"
+#include "JSONObjectHelper.h"
 
 #include "v8.h"
 
@@ -1367,6 +1368,10 @@ void MetadataNode::PackageGetterCallback(Local<Name> property, const PropertyCal
                 }
 
                 V8SetPrivateValue(isolate, thiz, strProperty, cachedItem);
+
+                if (node->m_name == "org/json" && child.name == "JSONObject") {
+                    JSONObjectHelper::RegisterFromFunction(isolate, cachedItem);
+                }
             }
         }
 

test-app/runtime/src/main/cpp/ModuleInternal.cpp

@@ -31,6 +31,15 @@ ModuleInternal::ModuleInternal()
     : m_isolate(nullptr), m_requireFunction(nullptr), m_requireFactoryFunction(nullptr) {
 }
 
+ModuleInternal::~ModuleInternal() {
+    delete this->m_requireFunction;
+    delete this->m_requireFactoryFunction;
+    for (const auto pair: this->m_requireCache) {
+        delete pair.second;
+    }
+    this->m_requireCache.clear();
+}
+
 void ModuleInternal::Init(Isolate* isolate, const string& baseDir) {
     JEnv env;
 

test-app/runtime/src/main/cpp/ModuleInternal.h

@@ -19,6 +19,8 @@ class ModuleInternal {
     public:
         ModuleInternal();
 
+        ~ModuleInternal();
+
         void Init(v8::Isolate* isolate, const std::string& baseDir = "");
 
         void Load(const std::string& path);

test-app/runtime/src/main/cpp/Runtime.cpp

@@ -203,6 +203,12 @@ void Runtime::Init(jstring filesPath, jstring nativeLibDir, bool verboseLoggingE
     m_isolate = PrepareV8Runtime(filesRoot, nativeLibDirStr, packageNameStr, isDebuggable, callingDirStr, profilerOutputDirStr, maxLogcatObjectSize, forceLog);
 }
 
+Runtime::~Runtime() {
+    delete this->m_objectManager;
+    delete this->m_heapSnapshotBlob;
+    delete this->m_startupData;
+}
+
 std::string Runtime::ReadFileText(const std::string& filePath) {
 #ifdef APPLICATION_IN_DEBUG
     std::lock_guard<std::mutex> lock(m_fileWriteMutex);
@@ -717,7 +723,6 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, const string& native
     ArrayHelper::Init(context);
 
     m_arrayBufferHelper.CreateConvertFunctions(isolate, global, m_objectManager);
-    m_jsonObjectHelper.CreateConvertFunctions(isolate, global, m_objectManager);
 
     s_mainThreadInitialized = true;
 

test-app/runtime/src/main/cpp/Runtime.h

@@ -7,7 +7,6 @@
 #include "SimpleAllocator.h"
 #include "WeakRef.h"
 #include "ArrayBufferHelper.h"
-#include "JSONObjectHelper.h"
 #include "Profiler.h"
 #include "ModuleInternal.h"
 #include "File.h"
@@ -23,6 +22,8 @@ class Runtime {
             CONSTANTS = 1
         };
 
+        ~Runtime();
+
         static Runtime* GetRuntime(int runtimeId);
 
         static Runtime* GetRuntime(v8::Isolate* isolate);
@@ -78,7 +79,6 @@ class Runtime {
         ModuleInternal m_module;
 
         ArrayBufferHelper m_arrayBufferHelper;
-        JSONObjectHelper m_jsonObjectHelper;
 
         WeakRef m_weakRef;
 

test-app/runtime/src/main/cpp/com_tns_Runtime.cpp

@@ -322,12 +322,18 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_TerminateWorkerCallback(JNIEnv* e
 
     auto isolate = runtime->GetIsolate();
 
-    v8::Isolate::Scope isolate_scope(isolate);
-    v8::HandleScope handleScope(isolate);
+    {
+        v8::Isolate::Scope isolate_scope(isolate);
+        v8::HandleScope handleScope(isolate);
+
+        CallbackHandlers::TerminateWorkerThread(isolate);
+
+        runtime->DestroyRuntime();
+    }
 
-    CallbackHandlers::TerminateWorkerThread(isolate);
+    isolate->Dispose();
 
-    runtime->DestroyRuntime();
+    delete runtime;
 }
 
 extern "C" JNIEXPORT void Java_com_tns_Runtime_ClearWorkerPersistent(JNIEnv* env, jobject obj, jint runtimeId, jint workerId) {