Skip to content

src: add public API to create isolate and context #20639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
helloshuangzi wants to merge 4 commits into from
Closed

src: add public API to create isolate and context #20639

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c48b52
src: add public API to create isolate and context
helloshuangzi May 11, 2018
32f988b
src: use unique_ptr to save Free calls
helloshuangzi May 11, 2018
e658fb7
src: use custom deleter for allocator and isolateData
helloshuangzi May 14, 2018
666c25f
src: polish indention
helloshuangzi May 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: use unique_ptr to save Free calls
  • Loading branch information
@helloshuangzi
helloshuangzi committed May 11, 2018
commit 32f988b856352bffa0fc381646ff376e0db5b21d
16 changes: 8 additions & 8 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4426,7 +4426,7 @@ int EmitExit(Environment* env) {


ArrayBufferAllocator* CreateArrayBufferAllocator() {
return new ArrayBufferAllocator;
return new ArrayBufferAllocator();
}


Expand Down Expand Up @@ -4623,8 +4623,8 @@ Isolate* NewIsolate(ArrayBufferAllocator* allocator) {
inline int Start(uv_loop_t* event_loop,
int argc, const char* const* argv,
int exec_argc, const char* const* exec_argv) {
ArrayBufferAllocator* allocator = CreateArrayBufferAllocator();
Isolate* const isolate = NewIsolate(allocator);
std::unique_ptr<ArrayBufferAllocator> allocator(CreateArrayBufferAllocator());

@bnoordhuis bnoordhuis May 12, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer simply ArrayBufferAllocator allocator; if you're not using a custom deleter, no point in heap-allocating it. Likewise for IsolateData.

@helloshuangzi helloshuangzi May 14, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detail description. I just want to reuse the APIs to help keep consistency between node-main-thread and addons when the relevant logic gets updated in the future.
Updated with custom deleter for both ArrayBufferAllocator and IsolateData.

Isolate* const isolate = NewIsolate(allocator.get());
if (isolate == nullptr)
return 12; // Signal internal error.

Expand All @@ -4639,16 +4639,17 @@ inline int Start(uv_loop_t* event_loop,
Locker locker(isolate);
Isolate::Scope isolate_scope(isolate);
HandleScope handle_scope(isolate);
IsolateData* isolate_data = CreateIsolateData(
std::unique_ptr<IsolateData> isolate_data(
CreateIsolateData(
isolate,
event_loop,
v8_platform.Platform(),
allocator);
allocator.get()));
if (track_heap_objects) {
isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
}
exit_code = Start(isolate, isolate_data, argc, argv, exec_argc, exec_argv);
FreeIsolateData(isolate_data);
exit_code =
Start(isolate, isolate_data.get(), argc, argv, exec_argc, exec_argv);

@bnoordhuis bnoordhuis May 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, four spaces.

}

{
Expand All @@ -4658,7 +4659,6 @@ inline int Start(uv_loop_t* event_loop,
}

isolate->Dispose();
FreeArrayBufferAllocator(allocator);

return exit_code;
}
Expand Down