Skip to content

Commit c44edec

Browse files
addaleaxaddaleax
committed
src: provide a variant of LoadEnvironment taking a callback
This allows embedders to flexibly control how they start JS code rather than using `third_party_main`. PR-URL: #30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
1 parent a9fb51f commit c44edec

6 files changed

Lines changed: 77 additions & 20 deletions

File tree

‎src/api/environment.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,12 @@ NODE_EXTERN std::unique_ptr<InspectorParentHandle> GetInspectorParentHandle(
422422
}
423423

424424
void LoadEnvironment(Environment* env) {
425-
USE(LoadEnvironment(env, {}));
425+
USE(LoadEnvironment(env, nullptr, {}));
426426
}
427427

428428
MaybeLocal<Value> LoadEnvironment(
429429
Environment* env,
430+
StartExecutionCallback cb,
430431
std::unique_ptr<InspectorParentHandle> inspector_parent_handle) {
431432
env->InitializeLibuv(per_process::v8_is_profiling);
432433
env->InitializeDiagnostics();
@@ -441,9 +442,7 @@ MaybeLocal<Value> LoadEnvironment(
441442
}
442443
#endif
443444

444-
// TODO(joyeecheung): Allow embedders to customize the entry
445-
// point more directly without using _third_party_main.js
446-
return StartExecution(env);
445+
return StartExecution(env, cb);
447446
}
448447

449448
Environment* GetCurrentEnvironment(Local<Context> context) {

‎src/node.cc‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,7 @@ void MarkBootstrapComplete(const FunctionCallbackInfo<Value>& args) {
372372
performance::NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
373373
}
374374

375+
static
375376
MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
376377
EscapableHandleScope scope(env->isolate());
377378
CHECK_NOT_NULL(main_script_id);
@@ -392,17 +393,31 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
392393
->GetFunction(env->context())
393394
.ToLocalChecked()};
394395

395-
InternalCallbackScope callback_scope(
396-
env,
397-
Object::New(env->isolate()),
398-
{ 1, 0 },
399-
InternalCallbackScope::kSkipAsyncHooks);
400-
401396
return scope.EscapeMaybe(
402397
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
403398
}
404399

405-
MaybeLocal<Value> StartExecution(Environment* env) {
400+
MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
401+
InternalCallbackScope callback_scope(
402+
env,
403+
Object::New(env->isolate()),
404+
{ 1, 0 },
405+
InternalCallbackScope::kSkipAsyncHooks);
406+
407+
if (cb != nullptr) {
408+
EscapableHandleScope scope(env->isolate());
409+
410+
if (StartExecution(env, "internal/bootstrap/environment").IsEmpty())
411+
return {};
412+
413+
StartExecutionCallbackInfo info = {
414+
env->process_object(),
415+
env->native_module_require(),
416+
};
417+
418+
return scope.EscapeMaybe(cb(info));
419+
}
420+
406421
// To allow people to extend Node in different ways, this hook allows
407422
// one to drop a file lib/_third_party_main.js into the build
408423
// directory which will be executed instead of Node's normal loading.

‎src/node.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
#include "node_version.h" // NODE_MODULE_VERSION
7474

7575
#include <memory>
76+
#include <functional>
7677

7778
// We cannot use __POSIX__ in this header because that's only defined when
7879
// building Node.js.
@@ -438,12 +439,20 @@ NODE_EXTERN std::unique_ptr<InspectorParentHandle> GetInspectorParentHandle(
438439
ThreadId child_thread_id,
439440
const char* child_url);
440441

441-
// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload
442-
// and provide a more flexible approach than third_party_main.
442+
struct StartExecutionCallbackInfo {
443+
v8::Local<v8::Object> process_object;
444+
v8::Local<v8::Function> native_require;
445+
};
446+
447+
using StartExecutionCallback =
448+
std::function<v8::MaybeLocal<v8::Value>(const StartExecutionCallbackInfo&)>;
449+
450+
// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload.
443451
NODE_EXTERN void LoadEnvironment(Environment* env);
444452
NODE_EXTERN v8::MaybeLocal<v8::Value> LoadEnvironment(
445453
Environment* env,
446-
std::unique_ptr<InspectorParentHandle> inspector_parent_handle);
454+
StartExecutionCallback cb,
455+
std::unique_ptr<InspectorParentHandle> inspector_parent_handle = {});
447456
NODE_EXTERN void FreeEnvironment(Environment* env);
448457

449458
// This may return nullptr if context is not associated with a Node instance.

‎src/node_internals.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,10 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
298298
v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
299299
uv_loop_t* event_loop,
300300
MultiIsolatePlatform* platform);
301-
v8::MaybeLocal<v8::Value> StartExecution(Environment* env);
301+
// This overload automatically picks the right 'main_script_id' if no callback
302+
// was provided by the embedder.
302303
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
303-
const char* main_script_id);
304+
StartExecutionCallback cb = nullptr);
304305
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
305306
v8::MaybeLocal<v8::Value> ExecuteBootstrapper(
306307
Environment* env,

‎src/node_worker.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ void Worker::Run() {
323323
CreateEnvMessagePort(env_.get());
324324
Debug(this, "Created message port for worker %llu", thread_id_.id);
325325
if (LoadEnvironment(env_.get(),
326+
nullptr,
326327
std::move(inspector_parent_handle_))
327328
.IsEmpty()) {
328329
return;

‎test/cctest/test_environment.cc‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,35 @@ class EnvironmentTest : public EnvironmentTestFixture {
5151
// CHECK(result->IsString());
5252
// }
5353

54+
TEST_F(EnvironmentTest, LoadEnvironmentWithCallback) {
55+
const v8::HandleScope handle_scope(isolate_);
56+
const Argv argv;
57+
Env env {handle_scope, argv};
58+
59+
v8::Local<v8::Context> context = isolate_->GetCurrentContext();
60+
bool called_cb = false;
61+
node::LoadEnvironment(*env,
62+
[&](const node::StartExecutionCallbackInfo& info)
63+
-> v8::MaybeLocal<v8::Value> {
64+
called_cb = true;
65+
66+
CHECK(info.process_object->IsObject());
67+
CHECK(info.native_require->IsFunction());
68+
69+
v8::Local<v8::Value> argv0 = info.process_object->Get(
70+
context,
71+
v8::String::NewFromOneByte(
72+
isolate_,
73+
reinterpret_cast<const uint8_t*>("argv0"),
74+
v8::NewStringType::kNormal).ToLocalChecked()).ToLocalChecked();
75+
CHECK(argv0->IsString());
76+
77+
return info.process_object;
78+
});
79+
80+
CHECK(called_cb);
81+
}
82+
5483
TEST_F(EnvironmentTest, AtExitWithEnvironment) {
5584
const v8::HandleScope handle_scope(isolate_);
5685
const Argv argv;
@@ -188,9 +217,6 @@ static void at_exit_js(void* arg) {
188217
called_at_exit_js = true;
189218
}
190219

191-
// TODO(addaleax): Re-enable this test once it is possible to initialize the
192-
// Environment properly.
193-
/*
194220
TEST_F(EnvironmentTest, SetImmediateCleanup) {
195221
int called = 0;
196222
int called_unref = 0;
@@ -200,6 +226,12 @@ TEST_F(EnvironmentTest, SetImmediateCleanup) {
200226
const Argv argv;
201227
Env env {handle_scope, argv};
202228

229+
node::LoadEnvironment(*env,
230+
[&](const node::StartExecutionCallbackInfo& info)
231+
-> v8::MaybeLocal<v8::Value> {
232+
return v8::Object::New(isolate_);
233+
});
234+
203235
(*env)->SetImmediate([&](node::Environment* env_arg) {
204236
EXPECT_EQ(env_arg, *env);
205237
called++;
@@ -212,7 +244,7 @@ TEST_F(EnvironmentTest, SetImmediateCleanup) {
212244

213245
EXPECT_EQ(called, 1);
214246
EXPECT_EQ(called_unref, 0);
215-
}*/
247+
}
216248

217249
static char hello[] = "hello";
218250

0 commit comments

Comments
 (0)