Skip to content

Commit 70d887e

Browse files
joyeecheungjoyeecheung
committed
src: allow creating NodeMainInstance that does not own the isolate
Allows instantiating a NodeMainInstance with an isolate whose initialization and disposal are controlled by the caller. PR-URL: nodejs#27321 Refs: nodejs#17058 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent d66c7e3 commit 70d887e

5 files changed

Lines changed: 131 additions & 31 deletions

File tree

‎src/api/environment.cc‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -183,17 +183,33 @@ void SetIsolateCreateParamsForNode(Isolate::CreateParams* params) {
183183
#endif
184184
}
185185

186+
void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat) {
187+
switch (cat) {
188+
case IsolateSettingCategories::kErrorHandlers:
189+
isolate->AddMessageListenerWithErrorLevel(
190+
OnMessage,
191+
Isolate::MessageErrorLevel::kMessageError |
192+
Isolate::MessageErrorLevel::kMessageWarning);
193+
isolate->SetAbortOnUncaughtExceptionCallback(
194+
ShouldAbortOnUncaughtException);
195+
isolate->SetFatalErrorHandler(OnFatalError);
196+
break;
197+
case IsolateSettingCategories::kMisc:
198+
isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
199+
isolate->SetAllowWasmCodeGenerationCallback(
200+
AllowWasmCodeGenerationCallback);
201+
isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
202+
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
203+
break;
204+
default:
205+
UNREACHABLE();
206+
break;
207+
}
208+
}
209+
186210
void SetIsolateUpForNode(v8::Isolate* isolate) {
187-
isolate->AddMessageListenerWithErrorLevel(
188-
OnMessage,
189-
Isolate::MessageErrorLevel::kMessageError |
190-
Isolate::MessageErrorLevel::kMessageWarning);
191-
isolate->SetAbortOnUncaughtExceptionCallback(ShouldAbortOnUncaughtException);
192-
isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
193-
isolate->SetFatalErrorHandler(OnFatalError);
194-
isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback);
195-
isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
196-
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
211+
SetIsolateUpForNode(isolate, IsolateSettingCategories::kErrorHandlers);
212+
SetIsolateUpForNode(isolate, IsolateSettingCategories::kMisc);
197213
}
198214

199215
Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {

‎src/node.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -886,8 +886,12 @@ int Start(int argc, char** argv) {
886886
}
887887

888888
{
889-
NodeMainInstance main_instance(
890-
uv_default_loop(), result.args, result.exec_args);
889+
Isolate::CreateParams params;
890+
NodeMainInstance main_instance(&params,
891+
uv_default_loop(),
892+
per_process::v8_platform.Platform(),
893+
result.args,
894+
result.exec_args);
891895
result.exit_code = main_instance.Run();
892896
}
893897

‎src/node_internals.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,9 @@ struct InitializationResult {
319319
};
320320
InitializationResult InitializeOncePerProcess(int argc, char** argv);
321321
void TearDownOncePerProcess();
322+
enum class IsolateSettingCategories { kErrorHandlers, kMisc };
323+
void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat);
324+
void SetIsolateCreateParamsForNode(v8::Isolate::CreateParams* params);
322325

323326
#if HAVE_INSPECTOR
324327
namespace profiler {

‎src/node_main_instance.cc‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,70 @@ using v8::Local;
1212
using v8::Locker;
1313
using v8::SealHandleScope;
1414

15-
NodeMainInstance::NodeMainInstance(uv_loop_t* event_loop,
15+
NodeMainInstance::NodeMainInstance(Isolate* isolate,
16+
uv_loop_t* event_loop,
17+
MultiIsolatePlatform* platform,
18+
const std::vector<std::string>& args,
19+
const std::vector<std::string>& exec_args)
20+
: args_(args),
21+
exec_args_(exec_args),
22+
array_buffer_allocator_(nullptr),
23+
isolate_(isolate),
24+
platform_(platform),
25+
isolate_data_(nullptr),
26+
owns_isolate_(false) {
27+
isolate_data_.reset(new IsolateData(isolate_, event_loop, platform, nullptr));
28+
SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc);
29+
}
30+
31+
NodeMainInstance* NodeMainInstance::Create(
32+
Isolate* isolate,
33+
uv_loop_t* event_loop,
34+
MultiIsolatePlatform* platform,
35+
const std::vector<std::string>& args,
36+
const std::vector<std::string>& exec_args) {
37+
return new NodeMainInstance(isolate, event_loop, platform, args, exec_args);
38+
}
39+
40+
NodeMainInstance::NodeMainInstance(Isolate::CreateParams* params,
41+
uv_loop_t* event_loop,
42+
MultiIsolatePlatform* platform,
1643
const std::vector<std::string>& args,
1744
const std::vector<std::string>& exec_args)
1845
: args_(args),
1946
exec_args_(exec_args),
2047
array_buffer_allocator_(ArrayBufferAllocator::Create()),
2148
isolate_(nullptr),
22-
isolate_data_(nullptr) {
23-
// TODO(joyeecheung): when we implement snapshot integration this needs to
24-
// set params.external_references.
25-
Isolate::CreateParams params;
26-
params.array_buffer_allocator = array_buffer_allocator_.get();
27-
isolate_ =
28-
NewIsolate(&params, event_loop, per_process::v8_platform.Platform());
49+
platform_(platform),
50+
isolate_data_(nullptr),
51+
owns_isolate_(true) {
52+
params->array_buffer_allocator = array_buffer_allocator_.get();
53+
isolate_ = Isolate::Allocate();
2954
CHECK_NOT_NULL(isolate_);
30-
isolate_data_.reset(CreateIsolateData(isolate_,
31-
event_loop,
32-
per_process::v8_platform.Platform(),
33-
array_buffer_allocator_.get()));
55+
// Register the isolate on the platform before the isolate gets initialized,
56+
// so that the isolate can access the platform during initialization.
57+
platform->RegisterIsolate(isolate_, event_loop);
58+
SetIsolateCreateParamsForNode(params);
59+
Isolate::Initialize(isolate_, *params);
60+
61+
isolate_data_.reset(new IsolateData(
62+
isolate_, event_loop, platform, array_buffer_allocator_.get()));
63+
SetIsolateUpForNode(isolate_, IsolateSettingCategories::kMisc);
64+
SetIsolateUpForNode(isolate_, IsolateSettingCategories::kErrorHandlers);
65+
}
66+
67+
void NodeMainInstance::Dispose() {
68+
CHECK(!owns_isolate_);
69+
platform_->DrainTasks(isolate_);
70+
delete this;
3471
}
3572

3673
NodeMainInstance::~NodeMainInstance() {
74+
if (!owns_isolate_) {
75+
return;
76+
}
3777
isolate_->Dispose();
38-
per_process::v8_platform.Platform()->UnregisterIsolate(isolate_);
78+
platform_->UnregisterIsolate(isolate_);
3979
}
4080

4181
int NodeMainInstance::Run() {
@@ -120,6 +160,7 @@ std::unique_ptr<Environment> NodeMainInstance::CreateMainEnvironment(
120160
}
121161

122162
Local<Context> context = NewContext(isolate_);
163+
CHECK(!context.IsEmpty());
123164
Context::Scope context_scope(context);
124165

125166
std::unique_ptr<Environment> env = std::make_unique<Environment>(

‎src/node_main_instance.h‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

6+
#include <cstddef>
67
#include "node.h"
78
#include "util.h"
89
#include "uv.h"
@@ -14,29 +15,64 @@ namespace node {
1415
// We may be able to create an abstract class to reuse some of the routines.
1516
class NodeMainInstance {
1617
public:
17-
NodeMainInstance(const NodeMainInstance&) = delete;
18-
NodeMainInstance& operator=(const NodeMainInstance&) = delete;
19-
NodeMainInstance(NodeMainInstance&&) = delete;
20-
NodeMainInstance& operator=(NodeMainInstance&&) = delete;
18+
// To create a main instance that does not own the isoalte,
19+
// The caller needs to do:
20+
//
21+
// Isolate* isolate = Isolate::Allocate();
22+
// platform->RegisterIsolate(isolate, loop);
23+
// isolate->Initialize(...);
24+
// isolate->Enter();
25+
// NodeMainInstance* main_instance =
26+
// NodeMainInstance::Create(isolate, loop, args, exec_args);
27+
//
28+
// When tearing it down:
29+
//
30+
// main_instance->Cleanup(); // While the isolate is entered
31+
// isolate->Exit();
32+
// isolate->Dispose();
33+
// platform->UnregisterIsolate(isolate);
34+
//
35+
// After calling Dispose() the main_instance is no longer accessible.
36+
static NodeMainInstance* Create(v8::Isolate* isolate,
37+
uv_loop_t* event_loop,
38+
MultiIsolatePlatform* platform,
39+
const std::vector<std::string>& args,
40+
const std::vector<std::string>& exec_args);
41+
void Dispose();
2142

22-
NodeMainInstance(uv_loop_t* event_loop,
43+
// Create a main instance that owns the isolate
44+
NodeMainInstance(v8::Isolate::CreateParams* params,
45+
uv_loop_t* event_loop,
46+
MultiIsolatePlatform* platform,
2347
const std::vector<std::string>& args,
2448
const std::vector<std::string>& exec_args);
2549
~NodeMainInstance();
2650

2751
// Start running the Node.js instances, return the exit code when finished.
2852
int Run();
2953

30-
private:
3154
// TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h
3255
// and the environment creation routine in workers somehow.
3356
std::unique_ptr<Environment> CreateMainEnvironment(int* exit_code);
3457

58+
private:
59+
NodeMainInstance(const NodeMainInstance&) = delete;
60+
NodeMainInstance& operator=(const NodeMainInstance&) = delete;
61+
NodeMainInstance(NodeMainInstance&&) = delete;
62+
NodeMainInstance& operator=(NodeMainInstance&&) = delete;
63+
64+
NodeMainInstance(v8::Isolate* isolate,
65+
uv_loop_t* event_loop,
66+
MultiIsolatePlatform* platform,
67+
const std::vector<std::string>& args,
68+
const std::vector<std::string>& exec_args);
3569
std::vector<std::string> args_;
3670
std::vector<std::string> exec_args_;
3771
std::unique_ptr<ArrayBufferAllocator> array_buffer_allocator_;
3872
v8::Isolate* isolate_;
73+
MultiIsolatePlatform* platform_;
3974
std::unique_ptr<IsolateData> isolate_data_;
75+
bool owns_isolate_ = false;
4076
};
4177

4278
} // namespace node

0 commit comments

Comments
 (0)