Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1007a6a

Browse files
committed
src: make BuiltinLoader threadsafe and non-global
As discussed innodejs/node#45888, using aglobal `BuiltinLoader` instance is probably undesirable in a worldin which embedders are able to create Node.js Environments withdifferent sources and therefore mutually incompatible codecaching properties.This PR makes it so that `BuiltinLoader` is no longer a globalsingleton and instead only shared between `Environment`s thathave a direct relation to each other, and addresses a fewthread safety issues along with that.PR-URL:nodejs/node#45942Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parentb34433f commit1007a6a

File tree

14 files changed

+293
-173
lines changed

14 files changed

+293
-173
lines changed

‎graal-nodejs/src/api/environment.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ MaybeLocal<Value> LoadEnvironment(
492492
returnLoadEnvironment(
493493
env, [&](const StartExecutionCallbackInfo& info) -> MaybeLocal<Value> {
494494
std::string name ="embedder_main_" +std::to_string(env->thread_id());
495-
builtins::BuiltinLoader::Add(name.c_str(), main_script_source_utf8);
495+
env->builtin_loader()->Add(name.c_str(), main_script_source_utf8);
496496
Realm* realm = env->principal_realm();
497497

498498
return realm->ExecuteBootstrapper(name.c_str());
@@ -732,10 +732,17 @@ Maybe<bool> InitializePrimordials(Local<Context> context) {
732732
"internal/per_context/messageport",
733733
nullptr};
734734

735+
// We do not have access to a per-Environment BuiltinLoader instance
736+
// at this point, because this code runs before an Environment exists
737+
// in the first place. However, creating BuiltinLoader instances is
738+
// relatively cheap and all the scripts that we may want to run at
739+
// startup are always present in it.
740+
thread_local builtins::BuiltinLoader builtin_loader;
735741
for (constchar**module = context_files; *module !=nullptr;module++) {
736742
Local<Value> arguments[] = {exports, primordials};
737-
if (builtins::BuiltinLoader::CompileAndCall(
738-
context, *module,arraysize(arguments), arguments,nullptr)
743+
if (builtin_loader
744+
.CompileAndCall(
745+
context, *module,arraysize(arguments), arguments,nullptr)
739746
.IsEmpty()) {
740747
// Execution failed during context creation.
741748
return Nothing<bool>();

‎graal-nodejs/src/env-inl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,10 @@ inline std::vector<double>* Environment::destroy_async_id_list() {
388388
return &destroy_async_id_list_;
389389
}
390390

391+
inline builtins::BuiltinLoader*Environment::builtin_loader() {
392+
return &builtin_loader_;
393+
}
394+
391395
inlinedoubleEnvironment::new_async_id() {
392396
async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter] +=1;
393397
returnasync_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter];

‎graal-nodejs/src/env.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,15 @@ Environment::Environment(IsolateData* isolate_data,
676676
thread_id_(thread_id.id ==static_cast<uint64_t>(-1)
677677
? AllocateEnvironmentThreadId().id
678678
: thread_id.id) {
679+
#ifdef NODE_V8_SHARED_RO_HEAP
680+
if (!is_main_thread()) {
681+
CHECK_NOT_NULL(isolate_data->worker_context());
682+
// TODO(addaleax): Adjust for the embedder API snapshot support changes
683+
builtin_loader()->CopySourceAndCodeCacheReferenceFrom(
684+
isolate_data->worker_context()->env()->builtin_loader());
685+
}
686+
#endif
687+
679688
// We'll be creating new objects so make sure we've entered the context.
680689
HandleScopehandle_scope(isolate);
681690

‎graal-nodejs/src/env.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,8 @@ class Environment : public MemoryRetainer {
695695
// List of id's that have been destroyed and need the destroy() cb called.
696696
inline std::vector<double>*destroy_async_id_list();
697697

698+
builtins::BuiltinLoader*builtin_loader();
699+
698700
std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
699701
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
700702
std::unordered_map<uint32_t, contextify::ContextifyScript*>
@@ -1099,6 +1101,8 @@ class Environment : public MemoryRetainer {
10991101

11001102
std::unique_ptr<Realm> principal_realm_ =nullptr;
11011103

1104+
builtins::BuiltinLoader builtin_loader_;
1105+
11021106
// Used by allocate_managed_buffer() and release_managed_buffer() to keep
11031107
// track of the BackingStore for a given pointer.
11041108
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>

‎graal-nodejs/src/node.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,6 @@
132132

133133
namespacenode {
134134

135-
using builtins::BuiltinLoader;
136-
137135
using v8::EscapableHandleScope;
138136
using v8::Isolate;
139137
using v8::Local;
@@ -1231,9 +1229,6 @@ int LoadSnapshotDataAndRun(const SnapshotData** snapshot_data_ptr,
12311229
}
12321230
}
12331231

1234-
if ((*snapshot_data_ptr) !=nullptr) {
1235-
BuiltinLoader::RefreshCodeCache((*snapshot_data_ptr)->code_cache);
1236-
}
12371232
NodeMainInstancemain_instance(*snapshot_data_ptr,
12381233
uv_default_loop(),
12391234
per_process::v8_platform.Platform(),

‎graal-nodejs/src/node_binding.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,13 +663,13 @@ void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
663663
CHECK(exports->SetPrototype(context,Null(isolate)).FromJust());
664664
DefineConstants(isolate, exports);
665665
}elseif (!strcmp(*module_v,"natives")) {
666-
exports =builtins::BuiltinLoader::GetSourceObject(context);
666+
exports =realm->env()->builtin_loader()->GetSourceObject(context);
667667
// Legacy feature: process.binding('natives').config contains stringified
668668
// config.gypi
669669
CHECK(exports
670670
->Set(context,
671671
realm->isolate_data()->config_string(),
672-
builtins::BuiltinLoader::GetConfigString(isolate))
672+
realm->env()->builtin_loader()->GetConfigString(isolate))
673673
.FromJust());
674674
}else {
675675
returnTHROW_ERR_INVALID_MODULE(isolate,"No such binding: %s", *module_v);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp