-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
src: improve embedder API #30467
Closed
Closed
src: improve embedder API #30467
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
b6738bc
src: make `FreeEnvironment()` perform all necessary cleanup
addaleax 7e2da59
src: fix memory leak in CreateEnvironment when bootstrap fails
addaleax 8a04c59
src: move worker_context from Environment to IsolateData
addaleax b732a01
src: associate is_main_thread() with worker_context()
addaleax 162b9ee
src: align worker and main thread code with embedder API
addaleax 2f7cda2
fixup! src: align worker and main thread code with embedder API
addaleax eb04cac
fixup! src: align worker and main thread code with embedder API
addaleax cf89ff0
fixup! src: align worker and main thread code with embedder API
addaleax 7336b7f
src: provide a variant of LoadEnvironment taking a callback
addaleax 61098fc
src: add LoadEnvironment() variant taking a string
addaleax 541d7f6
fixup! src: add LoadEnvironment() variant taking a string
addaleax c0b957c
test: re-enable cctest that was commented out
addaleax 9bd88e7
src: add unique_ptr equivalent of CreatePlatform
addaleax 99912a0
src: make InitializeNodeWithArgs() official public API
addaleax 23b9ace
src: add ability to look up platform based on `Environment*`
addaleax ededfc9
src: allow non-Node.js TracingControllers
addaleax 36ec54b
src: fix what a dispose without checking
718f91f
fixup! src: fix what a dispose without checking
addaleax 12bb53a
src: shutdown platform from FreePlatform()
addaleax 487df31
src,test: add full-featured embedder API test
addaleax 799c9b8
fixup! src,test: add full-featured embedder API test
addaleax ab5e211
doc: add basic embedding example documentation
addaleax 682e804
test: add extended embedder cctest
addaleax f191a72
fixup! doc: add basic embedding example documentation
addaleax 3ce279a
fixup! src,test: add full-featured embedder API test
addaleax f46c44c
fixup! fixup! doc: add basic embedding example documentation
addaleax 330f4a7
squash! test: add extended embedder cctest
addaleax 634a289
fixup! squash! test: add extended embedder cctest
addaleax File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,226 @@ | ||
# C++ Embedder API | ||
|
||
<!--introduced_in=REPLACEME--> | ||
|
||
Node.js provides a number of C++ APIs that can be used to execute JavaScript | ||
in a Node.js environment from other C++ software. | ||
|
||
The documentation for these APIs can be found in [src/node.h][] in the Node.js | ||
source tree. In addition to the APIs exposed by Node.js, some required concepts | ||
are provided by the V8 embedder API. | ||
|
||
Because using Node.js as an embedded library is different from writing code | ||
that is executed by Node.js, breaking changes do not follow typical Node.js | ||
[deprecation policy][] and may occur on each semver-major release without prior | ||
warning. | ||
addaleax marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Example embedding application | ||
|
||
The following sections will provide an overview over how to use these APIs | ||
to create an application from scratch that will perform the equivalent of | ||
`node -e <code>`, i.e. that will take a piece of JavaScript and run it in | ||
a Node.js-specific environment. | ||
|
||
The full code can be found [in the Node.js source tree][embedtest.cc]. | ||
|
||
### Setting up per-process state | ||
|
||
Node.js requires some per-process state management in order to run: | ||
|
||
* Arguments parsing for Node.js [CLI options][], | ||
* V8 per-process requirements, such as a `v8::Platform` instance. | ||
|
||
The following example shows how these can be set up. Some class names are from | ||
the `node` and `v8` C++ namespaces, respectively. | ||
|
||
```c++ | ||
int main(int argc, char** argv) { | ||
std::vector<std::string> args(argv, argv + argc); | ||
std::vector<std::string> exec_args; | ||
std::vector<std::string> errors; | ||
// Parse Node.js CLI options, and print any errors that have occurred while | ||
// trying to parse them. | ||
int exit_code = node::InitializeNodeWithArgs(&args, &exec_args, &errors); | ||
for (const std::string& error : errors) | ||
fprintf(stderr, "%s: %s\n", args[0].c_str(), error.c_str()); | ||
if (exit_code != 0) { | ||
return exit_code; | ||
} | ||
|
||
// Create a v8::Platform instance. `MultiIsolatePlatform::Create()` is a way | ||
// to create a v8::Platform instance that Node.js can use when creating | ||
// Worker threads. When no `MultiIsolatePlatform` instance is present, | ||
gireeshpunathil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Worker threads are disabled. | ||
gireeshpunathil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
std::unique_ptr<MultiIsolatePlatform> platform = | ||
MultiIsolatePlatform::Create(4); | ||
V8::InitializePlatform(platform.get()); | ||
V8::Initialize(); | ||
|
||
// See below for the contents of this function. | ||
int ret = RunNodeInstance(platform.get(), args, exec_args); | ||
|
||
V8::Dispose(); | ||
V8::ShutdownPlatform(); | ||
return ret; | ||
} | ||
``` | ||
|
||
### Per-instance state | ||
|
||
Node.js has a concept of a “Node.js instance”, that is commonly being referred | ||
to as `node::Environment`. Each `node::Environment` is associated with: | ||
|
||
* Exactly one `v8::Isolate`, i.e. one JS Engine instance, | ||
* Exactly one `uv_loop_t`, i.e. one event loop, and | ||
* A number of `v8::Context`s, but exactly one main `v8::Context`. | ||
* One `node::IsolateData` instance that contains information that could be | ||
shared by multiple `node::Environment`s that use the same `v8::Isolate`. | ||
Currently, no testing if performed for this scenario. | ||
|
||
In order to set up a `v8::Isolate`, an `v8::ArrayBuffer::Allocator` needs | ||
to be provided. One possible choice is the default Node.js allocator, which | ||
can be created through `node::ArrayBufferAllocator::Create()`. Using the Node.js | ||
allocator allows minor performance optimizations when addons use the Node.js | ||
C++ `Buffer` API, and is required in order to track `ArrayBuffer` memory in | ||
[`process.memoryUsage()`][]. | ||
|
||
Additionally, each `v8::Isolate` that is used for a Node.js instance needs to | ||
be registered and unregistered with the `MultiIsolatePlatform` instance, if one | ||
is being used, in order for the platform to know which event loop to use | ||
for tasks scheduled by the `v8::Isolate`. | ||
|
||
The `node::NewIsolate()` helper function creates a `v8::Isolate`, | ||
sets it up with some Node.js-specific hooks (e.g. the Node.js error handler), | ||
and registers it with the platform automatically. | ||
|
||
```c++ | ||
int RunNodeInstance(MultiIsolatePlatform* platform, | ||
const std::vector<std::string>& args, | ||
const std::vector<std::string>& exec_args) { | ||
int exit_code = 0; | ||
// Set up a libuv event loop. | ||
uv_loop_t loop; | ||
int ret = uv_loop_init(&loop); | ||
if (ret != 0) { | ||
fprintf(stderr, "%s: Failed to initialize loop: %s\n", | ||
args[0].c_str(), | ||
uv_err_name(ret)); | ||
return 1; | ||
} | ||
|
||
std::shared_ptr<ArrayBufferAllocator> allocator = | ||
ArrayBufferAllocator::Create(); | ||
|
||
Isolate* isolate = NewIsolate(allocator, &loop, platform); | ||
if (isolate == nullptr) { | ||
fprintf(stderr, "%s: Failed to initialize V8 Isolate\n", args[0].c_str()); | ||
return 1; | ||
} | ||
|
||
{ | ||
Locker locker(isolate); | ||
Isolate::Scope isolate_scope(isolate); | ||
|
||
// Create a node::IsolateData instance that will later be released using | ||
// node::FreeIsolateData(). | ||
std::unique_ptr<IsolateData, decltype(&node::FreeIsolateData)> isolate_data( | ||
node::CreateIsolateData(isolate, &loop, platform, allocator.get()), | ||
node::FreeIsolateData); | ||
|
||
// Set up a new v8::Context. | ||
HandleScope handle_scope(isolate); | ||
Local<Context> context = node::NewContext(isolate); | ||
if (context.IsEmpty()) { | ||
fprintf(stderr, "%s: Failed to initialize V8 Context\n", args[0].c_str()); | ||
return 1; | ||
} | ||
|
||
// The v8::Context needs to be entered when node::CreateEnvironment() and | ||
// node::LoadEnvironment() are being called. | ||
Context::Scope context_scope(context); | ||
|
||
// Create a node::Environment instance that will later be released using | ||
// node::FreeEnvironment(). | ||
std::unique_ptr<Environment, decltype(&node::FreeEnvironment)> env( | ||
node::CreateEnvironment(isolate_data.get(), context, args, exec_args), | ||
node::FreeEnvironment); | ||
|
||
// Set up the Node.js instance for execution, and run code inside of it. | ||
// There is also a variant that takes a callback and provides it with | ||
// the `require` and `process` objects, so that it can manually compile | ||
// and run scripts as needed. | ||
// The `require` function inside this script does *not* access the file | ||
// system, and can only load built-in Node.js modules. | ||
// `module.createRequire()` is being used to create one that is able to | ||
// load files from the disk, and uses the standard CommonJS file loader | ||
// instead of the internal-only `require` function. | ||
MaybeLocal<Value> loadenv_ret = node::LoadEnvironment( | ||
env.get(), | ||
"const publicRequire =" | ||
" require('module').createRequire(process.cwd() + '/');" | ||
"globalThis.require = publicRequire;" | ||
"require('vm').runInThisContext(process.argv[1]);"); | ||
gireeshpunathil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (loadenv_ret.IsEmpty()) // There has been a JS exception. | ||
return 1; | ||
|
||
{ | ||
// SealHandleScope protects against handle leaks from callbacks. | ||
SealHandleScope seal(isolate); | ||
bool more; | ||
do { | ||
uv_run(&loop, UV_RUN_DEFAULT); | ||
|
||
// V8 tasks on background threads may end up scheduling new tasks in the | ||
// foreground, which in turn can keep the event loop going. For example, | ||
// WebAssembly.compile() may do so. | ||
platform->DrainTasks(isolate); | ||
|
||
// If there are new tasks, continue. | ||
more = uv_loop_alive(&loop); | ||
if (more) continue; | ||
|
||
// node::EmitBeforeExit() is used to emit the 'beforeExit' event on | ||
// the `process` object. | ||
node::EmitBeforeExit(env.get()); | ||
|
||
// 'beforeExit' can also schedule new work that keeps the event loop | ||
// running. | ||
more = uv_loop_alive(&loop); | ||
} while (more == true); | ||
} | ||
|
||
// node::EmitExit() returns the current exit code. | ||
exit_code = node::EmitExit(env.get()); | ||
|
||
// node::Stop() can be used to explicitly stop the event loop and keep | ||
// further JavaScript from running. It can be called from any thread, | ||
// and will act like worker.terminate() if called from another thread. | ||
node::Stop(env.get()); | ||
} | ||
|
||
// Unregister the Isolate with the platform and add a listener that is called | ||
// when the Platform is done cleaning up any state it had associated with | ||
// the Isolate. | ||
bool platform_finished = false; | ||
platform->AddIsolateFinishedCallback(isolate, [](void* data) { | ||
*static_cast<bool*>(data) = true; | ||
}, &platform_finished); | ||
platform->UnregisterIsolate(isolate); | ||
isolate->Dispose(); | ||
|
||
// Wait until the platform has cleaned up all relevant resources. | ||
while (!platform_finished) | ||
uv_run(&loop, UV_RUN_ONCE); | ||
int err = uv_loop_close(&loop); | ||
assert(err == 0); | ||
|
||
return exit_code; | ||
} | ||
``` | ||
|
||
[`process.memoryUsage()`]: process.html#process_process_memoryusage | ||
[CLI options]: cli.html | ||
[deprecation policy]: deprecations.html | ||
[embedtest.cc]: https://github.com/nodejs/node/blob/master/test/embedding/embedtest.cc | ||
[src/node.h]: https://github.com/nodejs/node/blob/master/src/node.h |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently bypassing the
tools/test.py
machinery, so it does not play along too well with the CI which relies on tap output - is there a reason? (I think it should work if you just use the addon test config?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joyeecheung I’m not sure what this refers to … this test is run very differently from the addons?
So do the other cctests, too, if I’m not mistaken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the CI we use
gtest
's XML output forcctest
(gtest dropped tap support some time ago so we just upload the xml to Jenkins at the end of the test jobs):node/Makefile
Line 531 in f46c44c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardlau Any suggestions here? I don’t think we can really implement full TAP support for this test, but if it’s about failing/passing, I guess we could wrap the call to the embedding test in a shell script … I don’t really know what kind of integration that would require
(That being said, I don’t expect this test to be particularly platform-specific, so it’s most likely going to fail locally before we run into trouble in CI)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addon tests work by:
I think it's not too different from what's being done here other than that in this case the binary is an executable instead of a shared object (if you are using node.gyp to build it the dependency is handled just fine without extra modification), and we need to load the binary by spawning a process instead of dlopen, both are controllable via configs / source code.
If you use
AddonTestConfiguration
in thetest/folder/testcfg.py
, it would scan all files ending in.js
infolder
and run it without/${BUILDTYPE}/node(.exe)
with appropriate command line flags, whereas if you useSimpleTestCase
it needs the tests to be named liker'^test-.*\.m?js$'
, either is fine, depending on how you want the directory structure to be. Of course the ideal solution would be to add another config intest/__init__.py
and override e.g.GetCommand()
to locateout/${BUILDTYPE}/embedtest(.exe)
and run the test directly to save an extra indirection, but that's not a big deal (although if you already need that logic in a JS file, it's probably not that different to just do it in python)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(BTW I just noticed that the test is not run on Windows if you just use Makefile to run it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joyeecheung I understand how this would work conceptually, but that doesn’t mean that I know the concrete steps that would be necessary to implement this; in particular:
That would mean rebuilding all of Node.js, wouldn’t it?
Either way, if there’s an easy solution, I’m happy to see that implemented, but if this is a blocker for you then I’d prefer opening a separate PR for the test so that this PR can be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am talking about what's currently done in the PR is fine (i.e. you don't need extra config in vcbuild.bat to handle the dependency)
This should work though I have not tested it on Windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Note: you'd need to add an extra case e.g. read from fixtures, or just a plain string, for testing the module system as in the diff above the file is directly run with the node binary and only cases inside are run with the embedtest binary)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joyeecheung Done, added your changes + fixed up Makefile + an extra test that reads from fixtures in 330f4a7 – I’ll kick off CI and we’ll see how that goes.