Skip to content

Commit

Permalink
src: provide a variant of LoadEnvironment taking a callback
Browse files Browse the repository at this point in the history
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 <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
addaleax committed Mar 21, 2020
1 parent a9fb51f commit c44edec
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 20 deletions.
7 changes: 3 additions & 4 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,12 @@ NODE_EXTERN std::unique_ptr<InspectorParentHandle> GetInspectorParentHandle(
}

void LoadEnvironment(Environment* env) {
USE(LoadEnvironment(env, {}));
USE(LoadEnvironment(env, nullptr, {}));
}

MaybeLocal<Value> LoadEnvironment(
Environment* env,
StartExecutionCallback cb,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle) {
env->InitializeLibuv(per_process::v8_is_profiling);
env->InitializeDiagnostics();
Expand All @@ -441,9 +442,7 @@ MaybeLocal<Value> LoadEnvironment(
}
#endif

// TODO(joyeecheung): Allow embedders to customize the entry
// point more directly without using _third_party_main.js
return StartExecution(env);
return StartExecution(env, cb);
}

Environment* GetCurrentEnvironment(Local<Context> context) {
Expand Down
29 changes: 22 additions & 7 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ void MarkBootstrapComplete(const FunctionCallbackInfo<Value>& args) {
performance::NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
}

static
MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
EscapableHandleScope scope(env->isolate());
CHECK_NOT_NULL(main_script_id);
Expand All @@ -392,17 +393,31 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
->GetFunction(env->context())
.ToLocalChecked()};

InternalCallbackScope callback_scope(
env,
Object::New(env->isolate()),
{ 1, 0 },
InternalCallbackScope::kSkipAsyncHooks);

return scope.EscapeMaybe(
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
}

MaybeLocal<Value> StartExecution(Environment* env) {
MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
InternalCallbackScope callback_scope(
env,
Object::New(env->isolate()),
{ 1, 0 },
InternalCallbackScope::kSkipAsyncHooks);

if (cb != nullptr) {
EscapableHandleScope scope(env->isolate());

if (StartExecution(env, "internal/bootstrap/environment").IsEmpty())
return {};

StartExecutionCallbackInfo info = {
env->process_object(),
env->native_module_require(),
};

return scope.EscapeMaybe(cb(info));
}

// To allow people to extend Node in different ways, this hook allows
// one to drop a file lib/_third_party_main.js into the build
// directory which will be executed instead of Node's normal loading.
Expand Down
15 changes: 12 additions & 3 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#include "node_version.h" // NODE_MODULE_VERSION

#include <memory>
#include <functional>

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

// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload
// and provide a more flexible approach than third_party_main.
struct StartExecutionCallbackInfo {
v8::Local<v8::Object> process_object;
v8::Local<v8::Function> native_require;
};

using StartExecutionCallback =
std::function<v8::MaybeLocal<v8::Value>(const StartExecutionCallbackInfo&)>;

// TODO(addaleax): Deprecate this in favour of the MaybeLocal<> overload.
NODE_EXTERN void LoadEnvironment(Environment* env);
NODE_EXTERN v8::MaybeLocal<v8::Value> LoadEnvironment(
Environment* env,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle);
StartExecutionCallback cb,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle = {});
NODE_EXTERN void FreeEnvironment(Environment* env);

// This may return nullptr if context is not associated with a Node instance.
Expand Down
5 changes: 3 additions & 2 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,10 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
uv_loop_t* event_loop,
MultiIsolatePlatform* platform);
v8::MaybeLocal<v8::Value> StartExecution(Environment* env);
// This overload automatically picks the right 'main_script_id' if no callback
// was provided by the embedder.
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
const char* main_script_id);
StartExecutionCallback cb = nullptr);
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
v8::MaybeLocal<v8::Value> ExecuteBootstrapper(
Environment* env,
Expand Down
1 change: 1 addition & 0 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ void Worker::Run() {
CreateEnvMessagePort(env_.get());
Debug(this, "Created message port for worker %llu", thread_id_.id);
if (LoadEnvironment(env_.get(),
nullptr,
std::move(inspector_parent_handle_))
.IsEmpty()) {
return;
Expand Down
40 changes: 36 additions & 4 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,35 @@ class EnvironmentTest : public EnvironmentTestFixture {
// CHECK(result->IsString());
// }

TEST_F(EnvironmentTest, LoadEnvironmentWithCallback) {
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Env env {handle_scope, argv};

v8::Local<v8::Context> context = isolate_->GetCurrentContext();
bool called_cb = false;
node::LoadEnvironment(*env,
[&](const node::StartExecutionCallbackInfo& info)
-> v8::MaybeLocal<v8::Value> {
called_cb = true;

CHECK(info.process_object->IsObject());
CHECK(info.native_require->IsFunction());

v8::Local<v8::Value> argv0 = info.process_object->Get(
context,
v8::String::NewFromOneByte(
isolate_,
reinterpret_cast<const uint8_t*>("argv0"),
v8::NewStringType::kNormal).ToLocalChecked()).ToLocalChecked();
CHECK(argv0->IsString());

return info.process_object;
});

CHECK(called_cb);
}

TEST_F(EnvironmentTest, AtExitWithEnvironment) {
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Expand Down Expand Up @@ -188,9 +217,6 @@ static void at_exit_js(void* arg) {
called_at_exit_js = true;
}

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

node::LoadEnvironment(*env,
[&](const node::StartExecutionCallbackInfo& info)
-> v8::MaybeLocal<v8::Value> {
return v8::Object::New(isolate_);
});

(*env)->SetImmediate([&](node::Environment* env_arg) {
EXPECT_EQ(env_arg, *env);
called++;
Expand All @@ -212,7 +244,7 @@ TEST_F(EnvironmentTest, SetImmediateCleanup) {

EXPECT_EQ(called, 1);
EXPECT_EQ(called_unref, 0);
}*/
}

static char hello[] = "hello";

Expand Down

0 comments on commit c44edec

Please sign in to comment.