FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: runtime lifetime — deferred leaks, cross-isolate sharing, startup robustness by edusperoni · Pull Request #2013 · NativeScript/android · GitHub

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .cpp  (9) .h  (8) .md  (1) .txt  (1) All 4 file types selected
Deleted files Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
1 change: 0 additions & 1 deletion test-app/runtime/CMakeLists.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ add_library(
src/main/cpp/FieldAccessor.cpp
src/main/cpp/File.cpp
src/main/cpp/Interop.cpp
src/main/cpp/IsolateDisposer.cpp
src/main/cpp/IsolateTracked.cpp
src/main/cpp/JEnv.cpp
src/main/cpp/DesugaredInterfaceCompanionClassNameResolver.cpp
Expand Down
68 changes: 24 additions & 44 deletions test-app/runtime/src/main/cpp/BuiltinLoader.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "ArgConverter.h"
#include "NsBuiltinModules.h"
#include "RuntimeState.h"
#include "robin_hood.h"

using namespace v8;
Expand Down Expand Up @@ -36,13 +37,14 @@ constexpr const char* kPrimordialsParamName = "primordials";
constexpr size_t kParamCount = 5;

/*
* Per-isolate intrinsics snapshot and builtin require. Worker runtimes
* initialize on their own threads, so every access is under the mutex.
* This runtime's intrinsics snapshot and builtin require. Per-runtime state
* rather than an isolate-keyed shared map, so reaching it needs no lock and it
* is released with the runtime, while the isolate is still alive.
*/
std::mutex primordialsMutex;
robin_hood::unordered_map<Isolate*, Persistent<Object>*> isolateToPrimordials;
std::mutex builtinRequireMutex;
robin_hood::unordered_map<Isolate*, Persistent<v8::Function>*> isolateToBuiltinRequire;
struct BuiltinRealm {
v8::Global<v8::Object> primordials;
v8::Global<v8::Function> builtinRequire;
};

/*
* The `require` every builtin receives: builtin specifiers only, so a builtin
Expand Down Expand Up @@ -70,12 +72,13 @@ void BuiltinRequireCallback(const FunctionCallbackInfo<Value>& info) {
MaybeLocal<v8::Function> GetBuiltinRequire(Local<Context> context) {
Isolate* isolate = v8::Isolate::GetCurrent();

{
std::lock_guard<std::mutex> lock(builtinRequireMutex);
auto it = isolateToBuiltinRequire.find(isolate);
if (it != isolateToBuiltinRequire.end()) {
return it->second->Get(isolate);
}
auto* realm = RuntimeState::For<BuiltinRealm>(isolate);
if (realm == nullptr) {
return MaybeLocal<v8::Function>();
}

if (!realm->builtinRequire.IsEmpty()) {
return realm->builtinRequire.Get(isolate);
}

Local<v8::Function> require;
Expand All @@ -85,10 +88,7 @@ MaybeLocal<v8::Function> GetBuiltinRequire(Local<Context> context) {
return MaybeLocal<v8::Function>();
}

{
std::lock_guard<std::mutex> lock(builtinRequireMutex);
isolateToBuiltinRequire.emplace(isolate, new Persistent<v8::Function>(isolate, require));
}
realm->builtinRequire.Reset(isolate, require);
return require;
}

Expand Down Expand Up @@ -191,12 +191,13 @@ MaybeLocal<Value> CallBuiltin(Local<Context> context, BuiltinId id, Local<Value>
MaybeLocal<Object> GetPrimordials(Local<Context> context) {
Isolate* isolate = v8::Isolate::GetCurrent();

{
std::lock_guard<std::mutex> lock(primordialsMutex);
auto it = isolateToPrimordials.find(isolate);
if (it != isolateToPrimordials.end()) {
return it->second->Get(isolate);
}
auto* realm = RuntimeState::For<BuiltinRealm>(isolate);
if (realm == nullptr) {
return MaybeLocal<Object>();
}

if (!realm->primordials.IsEmpty()) {
return realm->primordials.Get(isolate);
}

Local<Value> result;
Expand All @@ -207,10 +208,7 @@ MaybeLocal<Object> GetPrimordials(Local<Context> context) {
}

Local<Object> primordials = result.As<Object>();
{
std::lock_guard<std::mutex> lock(primordialsMutex);
isolateToPrimordials.emplace(isolate, new Persistent<Object>(isolate, primordials));
}
realm->primordials.Reset(isolate, primordials);
return primordials;
}

Expand All @@ -226,22 +224,4 @@ MaybeLocal<Value> BuiltinLoader::RunBuiltin(Local<Context> context, BuiltinId id
return CallBuiltin(context, id, binding, primordials);
}

void BuiltinLoader::onDisposeIsolate(Isolate* isolate) {
{
std::lock_guard<std::mutex> lock(primordialsMutex);
auto it = isolateToPrimordials.find(isolate);
if (it != isolateToPrimordials.end()) {
delete it->second;
isolateToPrimordials.erase(it);
}
}

std::lock_guard<std::mutex> lock(builtinRequireMutex);
auto it = isolateToBuiltinRequire.find(isolate);
if (it != isolateToBuiltinRequire.end()) {
delete it->second;
isolateToBuiltinRequire.erase(it);
}
}

} // namespace tns
2 changes: 0 additions & 2 deletions test-app/runtime/src/main/cpp/BuiltinLoader.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ class BuiltinLoader {
static v8::MaybeLocal<v8::Value> RunBuiltin(
v8::Local<v8::Context> context, BuiltinId id,
v8::Local<v8::Value> binding = v8::Local<v8::Value>());

static void onDisposeIsolate(v8::Isolate* isolate);
};

} // namespace tns
Expand Down
28 changes: 0 additions & 28 deletions test-app/runtime/src/main/cpp/IsolateDisposer.cpp

This file was deleted.

40 changes: 0 additions & 40 deletions test-app/runtime/src/main/cpp/IsolateDisposer.h

This file was deleted.

Loading

Back | FazBrowse Home | New Git URL