Skip to content

Commit c5aa3f4

Browse files
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`. Backport-PR-URL: #35241 PR-URL: #30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
1 parent 808dedc commit c5aa3f4

File tree

6 files changed

+99
-16
lines changed

6 files changed

+99
-16
lines changed

src/api/environment.cc

+3-4
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,12 @@ NODE_EXTERN std::unique_ptr<InspectorParentHandle> GetInspectorParentHandle(
409409
}
410410

411411
void LoadEnvironment(Environment* env) {
412-
USE(LoadEnvironment(env, {}));
412+
USE(LoadEnvironment(env, nullptr, {}));
413413
}
414414

415415
MaybeLocal<Value> LoadEnvironment(
416416
Environment* env,
417+
StartExecutionCallback cb,
417418
std::unique_ptr<InspectorParentHandle> inspector_parent_handle) {
418419
env->InitializeLibuv(per_process::v8_is_profiling);
419420
env->InitializeDiagnostics();
@@ -428,9 +429,7 @@ MaybeLocal<Value> LoadEnvironment(
428429
}
429430
#endif
430431

431-
// TODO(joyeecheung): Allow embedders to customize the entry
432-
// point more directly without using _third_party_main.js
433-
return StartExecution(env);
432+
return StartExecution(env, cb);
434433
}
435434

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

src/node.cc

+22-7
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ void MarkBootstrapComplete(const FunctionCallbackInfo<Value>& args) {
364364
performance::NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
365365
}
366366

367+
static
367368
MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
368369
EscapableHandleScope scope(env->isolate());
369370
CHECK_NOT_NULL(main_script_id);
@@ -384,17 +385,31 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
384385
->GetFunction(env->context())
385386
.ToLocalChecked()};
386387

387-
InternalCallbackScope callback_scope(
388-
env,
389-
Object::New(env->isolate()),
390-
{ 1, 0 },
391-
InternalCallbackScope::kSkipAsyncHooks);
392-
393388
return scope.EscapeMaybe(
394389
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
395390
}
396391

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

src/node.h

+12-3
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
#define NODE_MAKE_VERSION(major, minor, patch) \
7879
((major) * 0x1000 + (minor) * 0x100 + (patch))
@@ -417,12 +418,20 @@ NODE_EXTERN std::unique_ptr<InspectorParentHandle> GetInspectorParentHandle(
417418
ThreadId child_thread_id,
418419
const char* child_url);
419420

420-
// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload
421-
// and provide a more flexible approach than third_party_main.
421+
struct StartExecutionCallbackInfo {
422+
v8::Local<v8::Object> process_object;
423+
v8::Local<v8::Function> native_require;
424+
};
425+
426+
using StartExecutionCallback =
427+
std::function<v8::MaybeLocal<v8::Value>(const StartExecutionCallbackInfo&)>;
428+
429+
// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload.
422430
NODE_EXTERN void LoadEnvironment(Environment* env);
423431
NODE_EXTERN v8::MaybeLocal<v8::Value> LoadEnvironment(
424432
Environment* env,
425-
std::unique_ptr<InspectorParentHandle> inspector_parent_handle);
433+
StartExecutionCallback cb,
434+
std::unique_ptr<InspectorParentHandle> inspector_parent_handle = {});
426435
NODE_EXTERN void FreeEnvironment(Environment* env);
427436

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

src/node_internals.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,10 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
301301
v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
302302
uv_loop_t* event_loop,
303303
MultiIsolatePlatform* platform);
304-
v8::MaybeLocal<v8::Value> StartExecution(Environment* env);
304+
// This overload automatically picks the right 'main_script_id' if no callback
305+
// was provided by the embedder.
305306
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
306-
const char* main_script_id);
307+
StartExecutionCallback cb = nullptr);
307308
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
308309
v8::MaybeLocal<v8::Value> ExecuteBootstrapper(
309310
Environment* env,

src/node_worker.cc

+1
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ void Worker::Run() {
333333
CreateEnvMessagePort(env_.get());
334334
Debug(this, "Created message port for worker %llu", thread_id_.id);
335335
if (LoadEnvironment(env_.get(),
336+
nullptr,
336337
std::move(inspector_parent_handle_))
337338
.IsEmpty()) {
338339
return;

test/cctest/test_environment.cc

+58
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,6 +217,35 @@ static void at_exit_js(void* arg) {
188217
called_at_exit_js = true;
189218
}
190219

220+
TEST_F(EnvironmentTest, SetImmediateCleanup) {
221+
int called = 0;
222+
int called_unref = 0;
223+
224+
{
225+
const v8::HandleScope handle_scope(isolate_);
226+
const Argv argv;
227+
Env env {handle_scope, argv};
228+
229+
node::LoadEnvironment(*env,
230+
[&](const node::StartExecutionCallbackInfo& info)
231+
-> v8::MaybeLocal<v8::Value> {
232+
return v8::Object::New(isolate_);
233+
});
234+
235+
(*env)->SetImmediate([&](node::Environment* env_arg) {
236+
EXPECT_EQ(env_arg, *env);
237+
called++;
238+
});
239+
(*env)->SetUnrefImmediate([&](node::Environment* env_arg) {
240+
EXPECT_EQ(env_arg, *env);
241+
called_unref++;
242+
});
243+
}
244+
245+
EXPECT_EQ(called, 1);
246+
EXPECT_EQ(called_unref, 0);
247+
}
248+
191249
static char hello[] = "hello";
192250

193251
TEST_F(EnvironmentTest, BufferWithFreeCallbackIsDetached) {

0 commit comments

Comments
 (0)