fix: Ensure Isolate is alive before using it (#63)

Author: darind

NativeScript/runtime/ArgConverter.mm

@@ -6,6 +6,7 @@
 #include "ObjectManager.h"
 #include "Interop.h"
 #include "Helpers.h"
+#include "Runtime.h"
 
 using namespace v8;
 using namespace std;
@@ -91,6 +92,12 @@
     MethodCallbackWrapper* data = static_cast<MethodCallbackWrapper*>(userData);
 
     Isolate* isolate = data->isolate_;
+
+    if (!Runtime::IsAlive(isolate)) {
+        memset(retValue, 0, cif->rtype->size);
+        return;
+    }
+
     v8::Locker locker(isolate);
     Isolate::Scope isolate_scope(isolate);
     HandleScope handle_scope(isolate);

NativeScript/runtime/ClassBuilder.mm

@@ -10,6 +10,7 @@
 #include "Helpers.h"
 #include "Caches.h"
 #include "Interop.h"
+#include "Runtime.h"
 
 using namespace v8;
 
@@ -254,45 +255,49 @@
         /// in order to make both of them destroyable/GC-able. When the JavaScript object is GC-ed we release the native counterpart as well.
         void (*retain)(id, SEL) = (void (*)(id, SEL))FindNotOverridenMethod(extendedClass, @selector(retain));
         IMP newRetain = imp_implementationWithBlock(^(id self) {
-          if ([self retainCount] == 1) {
-              auto it = cache->Instances.find(self);
-              if (it != cache->Instances.end()) {
-                  v8::Locker locker(isolate);
-                  Isolate::Scope isolate_scope(isolate);
-                  HandleScope handle_scope(isolate);
-                  Local<Value> value = it->second->Get(isolate);
-                  BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
-                  if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
-                      ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                      objcWrapper->GcProtect();
-                  }
-              }
-          }
-
-          return retain(self, @selector(retain));
+            if ([self retainCount] == 1) {
+                auto it = cache->Instances.find(self);
+                if (it != cache->Instances.end()) {
+                    v8::Locker locker(isolate);
+                    Isolate::Scope isolate_scope(isolate);
+                    HandleScope handle_scope(isolate);
+                    Local<Value> value = it->second->Get(isolate);
+                    BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
+                    if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
+                        ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
+                        objcWrapper->GcProtect();
+                    }
+                }
+            }
+
+            return retain(self, @selector(retain));
         });
         class_addMethod(extendedClass, @selector(retain), newRetain, "@@:");
 
         void (*release)(id, SEL) = (void (*)(id, SEL))FindNotOverridenMethod(extendedClass, @selector(release));
         IMP newRelease = imp_implementationWithBlock(^(id self) {
-          if ([self retainCount] == 2) {
-              auto it = cache->Instances.find(self);
-              if (it != cache->Instances.end()) {
-                  v8::Locker locker(isolate);
-                  Isolate::Scope isolate_scope(isolate);
-                  HandleScope handle_scope(isolate);
-                  if (it->second != nullptr) {
-                      Local<Value> value = it->second->Get(isolate);
-                      BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
-                      if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
-                          ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                          objcWrapper->GcUnprotect();
-                      }
-                  }
-              }
-          }
-
-          release(self, @selector(release));
+            if (!Runtime::IsAlive(isolate)) {
+                return;
+            }
+
+            if ([self retainCount] == 2) {
+                auto it = cache->Instances.find(self);
+                if (it != cache->Instances.end()) {
+                    v8::Locker locker(isolate);
+                    Isolate::Scope isolate_scope(isolate);
+                    HandleScope handle_scope(isolate);
+                    if (it->second != nullptr) {
+                        Local<Value> value = it->second->Get(isolate);
+                        BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
+                        if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
+                            ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
+                            objcWrapper->GcUnprotect();
+                        }
+                    }
+                }
+            }
+
+            release(self, @selector(release));
         });
         class_addMethod(extendedClass, @selector(release), newRelease, "v@:");
 

NativeScript/runtime/Runtime.h

@@ -42,9 +42,12 @@ class Runtime {
     }
 
     static id GetAppConfigValue(std::string key);
+
+    static bool IsAlive(v8::Isolate* isolate);
 private:
     static thread_local Runtime* currentRuntime_;
     static std::shared_ptr<v8::Platform> platform_;
+    static std::vector<v8::Isolate*> isolates_;
     static bool mainThreadInitialized_;
 
     void DefineGlobalObject(v8::Local<v8::Context> context);

NativeScript/runtime/Runtime.mm

@@ -48,6 +48,8 @@
         this->isolate_->Exit();
     }
 
+    Runtime::isolates_.erase(std::remove(Runtime::isolates_.begin(), Runtime::isolates_.end(), this->isolate_), Runtime::isolates_.end());
+
     this->isolate_->Dispose();
 
     currentRuntime_ = nullptr;
@@ -71,6 +73,8 @@
     create_params.array_buffer_allocator = &allocator_;
     Isolate* isolate = Isolate::New(create_params);
 
+    Runtime::isolates_.emplace_back(isolate);
+
     return isolate;
 }
 
@@ -226,7 +230,12 @@
     globalTemplate->Set(ToV8String(isolate, "__time"), timeFunctionTemplate);
 }
 
+bool Runtime::IsAlive(Isolate* isolate) {
+    return std::find(Runtime::isolates_.begin(), Runtime::isolates_.end(), isolate) != Runtime::isolates_.end();
+}
+
 std::shared_ptr<Platform> Runtime::platform_;
+std::vector<Isolate*> Runtime::isolates_;
 bool Runtime::mainThreadInitialized_ = false;
 thread_local Runtime* Runtime::currentRuntime_ = nullptr;
 

TestRunner/app/tests/shared/Workers/index.js

@@ -497,6 +497,44 @@ describe("TNS Workers", () => {
                 task.resume();
             ` });
         });
+
+        it("Should not crash if the worker registers a notification", done => {
+            var worker = new Worker("./tests/shared/Workers/EvalWorker.js");
+            worker.onmessage = (msg) => {
+                worker.terminate();
+                NSNotificationCenter.defaultCenter.postNotificationNameObject("MyNotification", null);
+                done();
+            };
+
+            var workerScript = `
+                var NotificationObserver = /** @class */ (function (_super) {
+                    __extends(NotificationObserver, _super);
+                    function NotificationObserver() {
+                        return _super !== null && _super.apply(this, arguments) || this;
+                    }
+                    NotificationObserver.initWithCallback = function (onReceiveCallback) {
+                        var observer = _super.new.call(this);
+                        observer._onReceiveCallback = onReceiveCallback;
+                        return observer;
+                    };
+                    NotificationObserver.prototype.onReceive = function (notification) {
+                        this._onReceiveCallback(notification);
+                    };
+                    NotificationObserver.ObjCExposedMethods = {
+                        onReceive: { returns: interop.types.void, params: [NSNotification] },
+                    };
+                    return NotificationObserver;
+                    }(NSObject));
+
+                    const observer = NotificationObserver.initWithCallback(notification => { });
+
+                    NSNotificationCenter.defaultCenter.addObserverSelectorNameObject(observer, "onReceive", "MyNotification", null);
+
+                    postMessage(self === global);
+            `;
+
+            worker.postMessage({ eval: workerScript });
+        });
     }
 
     function generateRandomString(strLen) {