fix: RunOnMainThreadCallback & PostFrameCallback (#1721)

triniwiz · web-flow · commit 1ccd033194d4 · 2022-09-29T06:52:28.000-04:00
* fix: RunOnMainThreadCallback &amp; PostFrameCallback

* fix: remove allocation

* fix: RunOnMainThreadCallback callback

* chore: bump alpha

* fix: PostFrameCallback

* chore: bump
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@nativescript/android",
   "description": "NativeScript Runtime for Android",
-  "version": "8.2.4",
+  "version": "8.3.1",
   "repository": {
     "type": "git",
     "url": "https://github.com/NativeScript/android-runtime.git"
diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp
@@ -1,5 +1,4 @@
 #include "CallbackHandlers.h"
-#include "NativeScriptAssert.h"
 #include "MetadataNode.h"
 #include "Util.h"
 #include "V8GlobalHelpers.h"
@@ -8,7 +7,6 @@
 #include "JsArgToArrayConverter.h"
 #include "ArgConverter.h"
 #include "v8-profiler.h"
-#include "NativeScriptException.h"
 #include <iostream>
 #include <sstream>
 #include <fstream>
@@ -20,7 +18,6 @@
 #include <unistd.h>
 #include <dlfcn.h>
 
-
 using namespace v8;
 using namespace std;
 using namespace tns;
@@ -670,55 +667,59 @@ void CallbackHandlers::RunOnMainThreadCallback(const FunctionCallbackInfo<v8::Va
     Isolate::Scope isolate_scope(isolate);
     HandleScope handle_scope(isolate);
 
-
     Local<Context> context = isolate->GetCurrentContext();
 
-    uint32_t key = ++count_;
+    uint64_t key = ++count_;
     Local<v8::Function> callback = args[0].As<v8::Function>();
-    CacheEntry entry(isolate, new Persistent<v8::Function>(isolate, callback));
-    cache_.emplace(key, entry);
-    auto key_str = std::to_string(key);
-    write(Runtime::GetWriter(), key_str.c_str(), 1);
+    CacheEntry entry(isolate, callback, context);
+    cache_.emplace(key, std::move(entry));
+    auto value = Callback(key);
+    auto size = sizeof(Callback);
+    auto wrote = write(Runtime::GetWriter(),&value , size);
 }
 
 int CallbackHandlers::RunOnMainThreadFdCallback(int fd, int events, void *data) {
-    std::string buf;
-    read(fd, &buf, 1);
+    struct Callback value;
+    auto size = sizeof(Callback);
+    ssize_t nr = read(fd, &value, sizeof(value));
 
-    auto key = std::stoi(buf);
+    auto key = value.id_;
 
     auto it = cache_.find(key);
     if (it == cache_.end()) {
         return 1;
     }
 
     Isolate *isolate = it->second.isolate_;
-    Persistent<v8::Function> *poCallback = it->second.callback_;
     v8::Locker locker(isolate);
     Isolate::Scope isolate_scope(isolate);
     HandleScope handle_scope(isolate);
-    auto context = v8::Context::New(isolate);
+    auto context = it->second.context_.Get(isolate);
     Context::Scope context_scope(context);
-    Local<v8::Function> cb = poCallback->Get(isolate);
+    Local<v8::Function> cb = it->second.callback_.Get(isolate);
     Local<Value> result;
 
-    if (!cb->Call(context, context->Global(), 0, nullptr).ToLocal(&result)) {
-        assert(false);
+    v8::TryCatch tc(isolate);
+
+    if (!cb->Call(context, context->Global(), 0, nullptr).ToLocal(&result)) {}
+
+    if(tc.HasCaught()){
+        throw NativeScriptException(tc);
     }
 
     RemoveKey(key);
+
     return 1;
 }
 
-void CallbackHandlers::RemoveKey(const uint32_t key) {
+void CallbackHandlers::RemoveKey(const uint64_t key) {
     auto it = cache_.find(key);
     if (it == cache_.end()) {
         return;
     }
 
-    Persistent<v8::Function> *poCallback = it->second.callback_;
-    poCallback->Reset();
-    delete poCallback;
+    it->second.callback_.Reset();
+    it->second.context_.Reset();
     cache_.erase(it);
 }
 
@@ -1584,12 +1585,16 @@ void CallbackHandlers::TerminateWorkerThread(Isolate *isolate) {
 void CallbackHandlers::RemoveIsolateEntries(v8::Isolate *isolate) {
     for (auto &item: cache_) {
         if (item.second.isolate_ == isolate) {
+            item.second.callback_.Reset();
+            item.second.context_.Reset();
             cache_.erase(item.first);
         }
     }
 
     for (auto &item: frameCallbackCache_) {
         if (item.second.isolate_ == isolate) {
+            item.second.callback_.Reset();
+            item.second.context_.Reset();
             frameCallbackCache_.erase(item.first);
         }
     }
@@ -1632,7 +1637,7 @@ void CallbackHandlers::PostFrameCallback(const FunctionCallbackInfo<v8::Value> &
         v8::Locker locker(isolate);
         Isolate::Scope isolate_scope(isolate);
         HandleScope handle_scope(isolate);
-        auto context = Context::New(isolate);
+        auto context = isolate->GetCurrentContext();
         Context::Scope context_scope(context);
 
         auto func = args[0].As<Function>();
@@ -1651,27 +1656,33 @@ void CallbackHandlers::PostFrameCallback(const FunctionCallbackInfo<v8::Value> &
             }
         }
 
-        Local<v8::Function> callback = args[0].As<v8::Function>();
+        Local<v8::Function> callback = func;
         uint64_t key = ++frameCallbackCount_;
 
         V8SetPrivateValue(isolate, func, idKey, v8::Number::New(isolate, (double) key));
 
-        FrameCallbackCacheEntry entry (isolate, new Persistent<v8::Function>(isolate, callback));
+        FrameCallbackCacheEntry entry (isolate, callback, context);
         entry.id = key;
-        frameCallbackCache_.emplace(key, entry);
 
         auto finalCallback = [](const v8::WeakCallbackInfo<FrameCallbackCacheEntry> &data) {
             auto value = data.GetParameter();
             for (auto &item: frameCallbackCache_) {
                 if (item.second.id == value->id) {
-                    cache_.erase(item.first);
+                    item.second.context_.Reset();
+                    item.second.callback_.Reset();
+                    frameCallbackCache_.erase(item.first);
                     break;
                 }
             }
         };
-        entry.callback_->SetWeak(&entry, finalCallback, v8::WeakCallbackType::kFinalizer);
 
-        PostCallback(args, &entry, context);
+        entry.callback_.SetWeak(&entry, finalCallback, v8::WeakCallbackType::kFinalizer);
+
+        frameCallbackCache_.emplace(key, std::move(entry));
+
+        auto val = frameCallbackCache_.find(key);
+
+        PostCallback(args, &val->second, context);
     }
 }
 
@@ -1685,7 +1696,7 @@ void CallbackHandlers::RemoveFrameCallback(const FunctionCallbackInfo<v8::Value>
         v8::Locker locker(isolate);
         Isolate::Scope isolate_scope(isolate);
         HandleScope handle_scope(isolate);
-        auto context = Context::New(isolate);
+        auto context = isolate->GetCurrentContext();
         Context::Scope context_scope(context);
 
         auto func = args[0].As<Function>();
@@ -1745,12 +1756,12 @@ void CallbackHandlers::InitChoreographer() {
 }
 
 
-robin_hood::unordered_map<uint32_t, CallbackHandlers::CacheEntry> CallbackHandlers::cache_;
+robin_hood::unordered_map<uint64_t, CallbackHandlers::CacheEntry> CallbackHandlers::cache_;
 
 robin_hood::unordered_map<uint64_t, CallbackHandlers::FrameCallbackCacheEntry> CallbackHandlers::frameCallbackCache_;
 
-_Atomic uint32_t CallbackHandlers::count_ = 0;
-_Atomic uint64_t CallbackHandlers::frameCallbackCount_ = 0;
+std::atomic_int64_t CallbackHandlers::count_ = {0};
+std::atomic_uint64_t CallbackHandlers::frameCallbackCount_ = {0};
 
 
 int CallbackHandlers::nextWorkerId = 0;
@@ -1770,4 +1781,4 @@ jmethodID CallbackHandlers::INIT_WORKER_METHOD_ID = nullptr;
 
 NumericCasts CallbackHandlers::castFunctions;
 ArrayElementAccessor CallbackHandlers::arrayElementAccessor;
-FieldAccessor CallbackHandlers::fieldAccessor;
+FieldAccessor CallbackHandlers::fieldAccessor;
diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.h b/test-app/runtime/src/main/cpp/CallbackHandlers.h
@@ -15,6 +15,9 @@
 #include "ObjectManager.h"
 #include "include/v8.h"
 #include "robin_hood.h"
+#include <errno.h>
+#include "NativeScriptAssert.h"
+#include "NativeScriptException.h"
 
 namespace tns {
     class CallbackHandlers {
@@ -281,34 +284,48 @@ namespace tns {
             jobject _runtime;
         };
 
-        static void RemoveKey(const uint32_t key);
+        static void RemoveKey(const uint64_t key);
+
+        static std::atomic_int64_t count_;
+
+
+        struct Callback {
+            Callback(){}
+            Callback(uint64_t id)
+                    : id_(id){
+            }
+            uint64_t id_;
+        };
 
-        static _Atomic uint32_t count_;
 
         struct CacheEntry {
-            CacheEntry(v8::Isolate* isolate, v8::Persistent<v8::Function>* callback)
+            CacheEntry(v8::Isolate* isolate, v8::Local<v8::Function> callback, v8::Local<v8::Context> context)
                     : isolate_(isolate),
-                      callback_(callback) {
+                      callback_(isolate, callback),
+                      context_(isolate, context){
             }
 
             v8::Isolate* isolate_;
-            v8::Persistent<v8::Function>* callback_;
-
+            v8::Global<v8::Function> callback_;
+            v8::Global<v8::Context> context_;
         };
 
-        static robin_hood::unordered_map<uint32_t, CacheEntry> cache_;
+        static robin_hood::unordered_map<uint64_t, CacheEntry> cache_;
 
 
-        static _Atomic uint64_t frameCallbackCount_;
+        static std::atomic_uint64_t frameCallbackCount_;
 
         struct FrameCallbackCacheEntry {
-            FrameCallbackCacheEntry(v8::Isolate *isolate, v8::Persistent<v8::Function> *callback)
+            FrameCallbackCacheEntry(v8::Isolate *isolate, v8::Local<v8::Function> callback, v8::Local<v8::Context> context)
                     : isolate_(isolate),
-                      callback_(callback) {
+                      callback_(isolate,callback),
+                      context_(isolate, context)
+                      {
             }
 
             v8::Isolate *isolate_;
-            v8::Persistent<v8::Function> *callback_;
+            v8::Global<v8::Function> callback_;
+            v8::Global<v8::Context> context_;
             bool removed;
             uint64_t id;
 
@@ -328,22 +345,25 @@ namespace tns {
                     }
                     v8::Isolate *isolate = entry->isolate_;
 
-                    v8::Persistent<v8::Function> *poCallback = entry->callback_;
-
                     v8::Locker locker(isolate);
                     v8::Isolate::Scope isolate_scope(isolate);
                     v8::HandleScope handle_scope(isolate);
-                    auto context = v8::Context::New(isolate);
+                    auto context = entry->context_.Get(isolate);
                     v8::Context::Scope context_scope(context);
 
-
-                    v8::Local<v8::Function> cb = poCallback->Get(isolate);
+                    v8::Local<v8::Function> cb = entry->callback_.Get(isolate);
                     v8::Local<v8::Value> result;
 
                     v8::Local<v8::Value> args[1] = {v8::Number::New(isolate, ts)};
 
+                    v8::TryCatch tc(isolate);
+
                     if (!cb->Call(context, context->Global(), 1, args).ToLocal(&result)) {
-                        assert(false);
+                        // TODO
+                    }
+
+                    if(tc.HasCaught()){
+                        throw NativeScriptException(tc);
                     }
 
                 }
diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp
@@ -31,6 +31,8 @@
 #include <snapshot_blob.h>
 #include "IsolateDisposer.h"
 #include <unistd.h>
+#include <thread>
+#include "File.h"
 
 #ifdef APPLICATION_IN_DEBUG
 #include "JsV8InspectorClient.h"
@@ -650,11 +652,25 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, const string& native
      */
     if (!s_mainThreadInitialized) {
         m_isMainThread = true;
-        pipe(m_mainLooper_fd);
+        pipe2(m_mainLooper_fd, O_NONBLOCK | O_CLOEXEC);
         m_mainLooper = ALooper_forThread();
-        ALooper_acquire(m_mainLooper);
 
-        ALooper_addFd(m_mainLooper, m_mainLooper_fd[0], 0, ALOOPER_EVENT_INPUT, CallbackHandlers::RunOnMainThreadFdCallback, nullptr);
+       ALooper_acquire(m_mainLooper);
+
+        // try using 2MB
+        int ret = fcntl(m_mainLooper_fd[1], F_SETPIPE_SZ, 2 * (1024 * 1024));
+
+        // try using 1MB
+        if (ret != 0) {
+            ret = fcntl(m_mainLooper_fd[1], F_SETPIPE_SZ, 1 * (1024 * 1024));
+        }
+
+        // try using 512KB
+        if (ret != 0) {
+            ret = fcntl(m_mainLooper_fd[1], F_SETPIPE_SZ, (512 * 1024));
+        }
+
+        ALooper_addFd(m_mainLooper, m_mainLooper_fd[0], ALOOPER_POLL_CALLBACK, ALOOPER_EVENT_INPUT, CallbackHandlers::RunOnMainThreadFdCallback, nullptr);
 
         Local<FunctionTemplate> workerFuncTemplate = FunctionTemplate::New(isolate, CallbackHandlers::NewThreadCallback);
         Local<ObjectTemplate> prototype = workerFuncTemplate->PrototypeTemplate();
@@ -851,6 +867,10 @@ int Runtime::GetWriter(){
     return m_mainLooper_fd[1];
 }
 
+int Runtime::GetReader(){
+    return m_mainLooper_fd[0];
+}
+
 JavaVM* Runtime::s_jvm = nullptr;
 jmethodID Runtime::GET_USED_MEMORY_METHOD_ID = nullptr;
 map<int, Runtime*> Runtime::s_id2RuntimeCache;
@@ -860,4 +880,3 @@ v8::Platform* Runtime::platform = nullptr;
 int Runtime::m_androidVersion = Runtime::GetAndroidVersion();
 ALooper* Runtime::m_mainLooper = nullptr;
 int Runtime::m_mainLooper_fd[2];
-
diff --git a/test-app/runtime/src/main/cpp/Runtime.h b/test-app/runtime/src/main/cpp/Runtime.h
@@ -13,6 +13,7 @@
 #include "File.h"
 #include <mutex>
 #include <android/looper.h>
+#include <fcntl.h>
 
 namespace tns {
 class Runtime {
@@ -71,6 +72,10 @@ class Runtime {
         std::string ReadFileText(const std::string& filePath);
 
         static int GetWriter();
+        static int GetReader();
+        static ALooper* GetMainLooper() {
+            return m_mainLooper;
+        }
 
     private:
         Runtime(JNIEnv* env, jobject runtime, int id);

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@nativescript/android",`
`3`	`3`	`"description": "NativeScript Runtime for Android",`
`4`		`- "version": "8.2.4",`
	`4`	`+ "version": "8.3.1",`
`5`	`5`	`"repository": {`
`6`	`6`	`"type": "git",`
`7`	`7`	`"url": "https://github.com/NativeScript/android-runtime.git"`