Skip to content
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

embedding: allow NewContext to create inspectable context #31411

Closed
wants to merge 2 commits into from

Conversation

Dragiyski
Copy link

NodeJS should allow C++ users to create contexts, whose compiled script can be debugged through the inspector agent. Currently this functionality is only available for JavaScript modules through VM module. Unfortunately, VM creates a context whose global is completely new and hidden v8::FunctionTemplate. C++ users might already have v8::FunctionTemplate or v8::ObjectTemplate from which they create a context.

The only option now is to call V8 API directly: v8::Context::New(...). The created context is not associated with any node::Environment and code executed in that context cannot be debugged in the inspector agent (the context is missing debug_context_id as this is provided when the agent is notified by ContextCreated). This might actually be an intended behavior. The node::NewContext also create a context and do not report it to any agent (agent is a part of node::Environment and node::NewContext does not access any environments.

This is a proposal of a new feature to allow mitigate that gap. It provides another form of node::NewContext where node::Environment is accepted as the first parameter instead of v8::Isolate. When such an environment is available, it will create a context with a specific v8::ObjectTemplate and then call v8::Environment::AssignToContext. The latter will report the newly created context to the environment's inspector agent (if one is available) and add some embedders properties to allow proper handling of stack traces.

Without this feature, C++ modules can create v8::Context and use v8::ScriptCompiler to compile functions and scripts. Those scripts are not reported to the inspector agent and functions has [[FunctionLocation]] internal slot set to <unknown> even when v8::Source specify resource name, line and column offset, etc. If a context is reported to the inspector agent, it receives a debug_context_id. If this is available, compilation of new scripts and functions in that context will report to the inspector agent, which will send the source to the inspector frontend. Furthermore, the environment that has setup PrepareStackTraceCallback calls for Environment::GetCurrent(context). If an exception occurs into a context not assigned to environment, stack trace would be ignored, even when it is available.

This feature can be implemented using the following principles:

  • Additive: do not change any behavior and code of existing NodeJS implementation;
  • Minimal: do not add large amount of new code;
  • Efficient: does not require additional variables, objects, etc to be stored into memory; does not require additional processing when the feature is not used;
  • Useful: it allows access to huge amount of V8 API. C++ addons can use that API now, but with a limitation towards debugging and stack trace generation.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v13.x labels Jan 19, 2020
@richardlau
Copy link
Member

New features usually target master.

@codebytere codebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Jan 19, 2020
@Trott
Copy link
Member

Trott commented Jan 20, 2020

@Dragiyski Can you re-open this PR against the master branch rather than v13.x-staging?

@addaleax
Copy link
Member

(Or rebase & change the base branch for this PR?)

@Dragiyski
Copy link
Author

Re-created in #31424

@Dragiyski Dragiyski closed this Jan 20, 2020
@Dragiyski Dragiyski deleted the v13.x-staging branch January 20, 2020 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants