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

lib,src: remove vm.runInDebugContext() #13295

Merged
merged 2 commits into from
Nov 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
lib/internal/v8.js
Copy link
Member Author

Choose a reason for hiding this comment

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

The file is whitelisted because the V8 builtins don't lint. They're not legal JS syntax, eslint can't parse them.

Copy link
Member

Choose a reason for hiding this comment

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

You should be able add comments by starting lines with # – if you comment here in the PR, that sounds like a good indicator that it makes sense to do so. :)

Copy link
Member

Choose a reason for hiding this comment

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

There are other lib/internal/v8 files whitelisted. I don't think we need a comment. But either way is fine.

lib/internal/v8_prof_polyfill.js
lib/punycode.js
test/addons/??_*
Expand Down
5 changes: 2 additions & 3 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -600,10 +600,9 @@ a V8-inspector based CLI debugger available through `node inspect`.
<a id="DEP0069"></a>
### DEP0069: vm.runInDebugContext(string)

Type: Runtime
Type: End-of-Life

The DebugContext will be removed in V8 soon and will not be available in Node
10+.
DebugContext has been removed in V8 and is not available in Node 10+.

*Note*: DebugContext was an experimental API.

Expand Down
31 changes: 0 additions & 31 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,36 +334,6 @@ console.log(util.inspect(sandbox));
// { globalVar: 1024 }
```

## vm.runInDebugContext(code)
<!-- YAML
added: v0.11.14
deprecated: v8.0.0
changes:
- version: v9.0.0
pr-url: https://github.com/nodejs/node/pull/12815
description: Calling this function now emits a deprecation warning.
-->

> Stability: 0 - Deprecated. An alternative is in development.

* `code` {string} The JavaScript code to compile and run.

The `vm.runInDebugContext()` method compiles and executes `code` inside the V8
debug context. The primary use case is to gain access to the V8 `Debug` object:

```js
const vm = require('vm');
const Debug = vm.runInDebugContext('Debug');
console.log(Debug.findScript(process.emit).name); // 'events.js'
console.log(Debug.findScript(process.exit).name); // 'internal/process.js'
```

*Note*: The debug context and object are intrinsically tied to V8's debugger
implementation and may change (or even be removed) without prior warning.

The `Debug` object can also be made available using the V8-specific
`--expose_debug_as=` [command line option][].

## vm.runInNewContext(code[, sandbox][, options])
<!-- YAML
added: v0.3.1
Expand Down Expand Up @@ -517,7 +487,6 @@ associating it with the `sandbox` object is what this document refers to as
[`vm.runInContext()`]: #vm_vm_runincontext_code_contextifiedsandbox_options
[`vm.runInThisContext()`]: #vm_vm_runinthiscontext_code_options
[V8 Embedder's Guide]: https://github.com/v8/v8/wiki/Embedder's%20Guide#contexts
[command line option]: cli.html
[contextified]: #vm_what_does_it_mean_to_contextify_an_object
[global object]: https://es5.github.io/#x15.1
[indirect `eval()` call]: https://es5.github.io/#x10.4.2
15 changes: 15 additions & 0 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// do this good and early, since it handles errors.
setupProcessFatal();

setupV8();
setupProcessICUVersions();

setupGlobalVariables();
Expand Down Expand Up @@ -414,6 +415,20 @@
};
}

function setupV8() {
// Warm up the map and set iterator preview functions. V8 compiles
// functions lazily (unless --nolazy is set) so we need to do this
// before we turn off --allow_natives_syntax again.
const v8 = NativeModule.require('internal/v8');
v8.previewMapIterator(new Map().entries(), 1);
v8.previewSetIterator(new Set().entries(), 1);
// Disable --allow_natives_syntax again unless it was explicitly
// specified on the command line.
const re = /^--allow[-_]natives[-_]syntax$/;
if (!process.execArgv.some((s) => re.test(s)))
process.binding('v8').setFlagsFromString('--noallow_natives_syntax');
}

function setupProcessICUVersions() {
const icu = process.binding('config').hasIntl ?
process.binding('icu') : undefined;
Expand Down
21 changes: 21 additions & 0 deletions lib/internal/v8.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

function take(it, n) {
const result = [];
for (const e of it) {
if (--n < 0)
break;
result.push(e);
}
return result;
}

function previewMapIterator(it, n) {
return take(%MapIteratorClone(it), n);
}

function previewSetIterator(it, n) {
return take(%SetIteratorClone(it), n);
}

module.exports = { previewMapIterator, previewSetIterator };
4 changes: 0 additions & 4 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,10 +629,6 @@ Module.prototype._compile = function(content, filename) {
if (filename === resolvedArgv) {
delete process._breakFirstLine;
inspectorWrapper = process.binding('inspector').callAndPauseOnStart;
if (!inspectorWrapper) {
const Debug = vm.runInDebugContext('Debug');
Debug.setBreakPoint(compiledWrapper, 0, 0);
}
}
}
var dirname = path.dirname(filename);
Expand Down
39 changes: 19 additions & 20 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const { TextDecoder, TextEncoder } = require('internal/encoding');
const { isBuffer } = require('buffer').Buffer;

const { errname } = process.binding('uv');
const { previewMapIterator, previewSetIterator } = require('internal/v8');

const {
getPromiseDetails,
Expand Down Expand Up @@ -77,7 +78,6 @@ const dateToISOString = Date.prototype.toISOString;
const errorToString = Error.prototype.toString;

var CIRCULAR_ERROR_MESSAGE;
var Debug;

/* eslint-disable */
const strEscapeSequencesRegExp = /[\x00-\x1f\x27\x5c]/;
Expand Down Expand Up @@ -356,17 +356,6 @@ function stylizeNoColor(str, styleType) {
return str;
}

function ensureDebugIsInitialized() {
if (Debug === undefined) {
const runInDebugContext = require('vm').runInDebugContext;
// a workaround till this entire method is removed
const originalValue = process.noDeprecation;
process.noDeprecation = true;
Debug = runInDebugContext('Debug');
process.noDeprecation = originalValue;
}
}

function formatValue(ctx, value, recurseTimes, ln) {
// Primitive types cannot have properties
if (typeof value !== 'object' && typeof value !== 'function') {
Expand Down Expand Up @@ -474,10 +463,10 @@ function formatValue(ctx, value, recurseTimes, ln) {
formatter = formatTypedArray;
} else if (isMapIterator(value)) {
braces = ['MapIterator {', '}'];
formatter = formatCollectionIterator;
formatter = formatMapIterator;
} else if (isSetIterator(value)) {
braces = ['SetIterator {', '}'];
formatter = formatCollectionIterator;
formatter = formatSetIterator;
} else {
// Check for boxed strings with valueOf()
// The .valueOf() call can fail for a multitude of reasons
Expand Down Expand Up @@ -782,17 +771,27 @@ function formatMap(ctx, value, recurseTimes, keys) {
return output;
}

function formatCollectionIterator(ctx, value, recurseTimes, keys) {
ensureDebugIsInitialized();
const mirror = Debug.MakeMirror(value, true);
const vals = mirror.preview();
const output = [];
function formatCollectionIterator(preview, ctx, value, recurseTimes,
visibleKeys, keys) {
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1;
var vals = preview(value, 100);
var output = [];
for (const o of vals) {
output.push(formatValue(ctx, o, recurseTimes));
output.push(formatValue(ctx, o, nextRecurseTimes));
}
return output;
}

function formatMapIterator(ctx, value, recurseTimes, visibleKeys, keys) {
return formatCollectionIterator(previewMapIterator, ctx, value, recurseTimes,
visibleKeys, keys);
}

function formatSetIterator(ctx, value, recurseTimes, visibleKeys, keys) {
return formatCollectionIterator(previewSetIterator, ctx, value, recurseTimes,
visibleKeys, keys);
}

function formatPromise(ctx, value, recurseTimes, keys) {
var output;
const [state, result] = getPromiseDetails(value);
Expand Down
15 changes: 0 additions & 15 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const {

makeContext,
isContext,
runInDebugContext: runInDebugContext_
} = process.binding('contextify');

// The binding provides a few useful primitives:
Expand Down Expand Up @@ -105,19 +104,6 @@ function sigintHandlersWrap(fn, thisArg, argsArray) {
}
}

let runInDebugContextWarned = false;
function runInDebugContext(code) {
if (runInDebugContextWarned === false) {
runInDebugContextWarned = true;
process.emitWarning(
'DebugContext has been deprecated and will be removed in a ' +
'future version.',
'DeprecationWarning',
'DEP0069');
}
return runInDebugContext_(code);
}

function runInContext(code, contextifiedSandbox, options) {
if (typeof options === 'string') {
options = {
Expand Down Expand Up @@ -156,7 +142,6 @@ module.exports = {
Script,
createContext,
createScript,
runInDebugContext,
runInContext,
runInNewContext,
runInThisContext,
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
'lib/internal/http2/core.js',
'lib/internal/http2/compat.js',
'lib/internal/http2/util.js',
'lib/internal/v8.js',
'lib/internal/v8_prof_polyfill.js',
'lib/internal/v8_prof_processor.js',
'lib/internal/streams/lazy_transform.js',
Expand Down
6 changes: 6 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4288,6 +4288,12 @@ void Init(int* argc,
const char no_typed_array_heap[] = "--typed_array_max_size_in_heap=0";
V8::SetFlagsFromString(no_typed_array_heap, sizeof(no_typed_array_heap) - 1);

// Needed for access to V8 intrinsics. Disabled again during bootstrapping,
// see lib/internal/bootstrap_node.js.
const char allow_natives_syntax[] = "--allow_natives_syntax";
V8::SetFlagsFromString(allow_natives_syntax,
sizeof(allow_natives_syntax) - 1);

// We should set node_is_initialized here instead of in node::Start,
// otherwise embedders using node::Init to initialize everything will not be
// able to set it and native modules will not load for them.
Expand Down
33 changes: 0 additions & 33 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@
#include "node_internals.h"
#include "node_watchdog.h"
#include "base_object-inl.h"
#include "v8-debug.h"

namespace node {

using v8::Array;
using v8::ArrayBuffer;
using v8::Boolean;
using v8::Context;
using v8::Debug;
using v8::EscapableHandleScope;
using v8::External;
using v8::Function;
Expand Down Expand Up @@ -218,42 +216,11 @@ class ContextifyContext {
function_template->InstanceTemplate()->SetInternalFieldCount(1);
env->set_script_data_constructor_function(function_template->GetFunction());

env->SetMethod(target, "runInDebugContext", RunInDebugContext);
env->SetMethod(target, "makeContext", MakeContext);
env->SetMethod(target, "isContext", IsContext);
}


static void RunInDebugContext(const FunctionCallbackInfo<Value>& args) {
Local<String> script_source(args[0]->ToString(args.GetIsolate()));
if (script_source.IsEmpty())
return; // Exception pending.
Local<Context> debug_context = Debug::GetDebugContext(args.GetIsolate());
Environment* env = Environment::GetCurrent(args);
if (debug_context.IsEmpty()) {
// Force-load the debug context.
auto dummy_event_listener = [] (const Debug::EventDetails&) {};
Debug::SetDebugEventListener(args.GetIsolate(), dummy_event_listener);
debug_context = Debug::GetDebugContext(args.GetIsolate());
CHECK(!debug_context.IsEmpty());
// Ensure that the debug context has an Environment assigned in case
// a fatal error is raised. The fatal exception handler in node.cc
// is not equipped to deal with contexts that don't have one and
// can't easily be taught that due to a deficiency in the V8 API:
// there is no way for the embedder to tell if the data index is
// in use.
const int index = Environment::kContextEmbedderDataIndex;
debug_context->SetAlignedPointerInEmbedderData(index, env);
}

Context::Scope context_scope(debug_context);
MaybeLocal<Script> script = Script::Compile(debug_context, script_source);
if (script.IsEmpty())
return; // Exception pending.
args.GetReturnValue().Set(script.ToLocalChecked()->Run());
}


static void MakeContext(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down
4 changes: 0 additions & 4 deletions test/fixtures/vm-run-in-debug-context.js

This file was deleted.

6 changes: 3 additions & 3 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

// Flags: --expose_internals
'use strict';
const common = require('../common');
const assert = require('assert');
const JSStream = process.binding('js_stream').JSStream;
const util = require('util');
const vm = require('vm');
const { previewMapIterator } = require('internal/v8');

assert.strictEqual(util.inspect(1), '1');
assert.strictEqual(util.inspect(false), 'false');
Expand Down Expand Up @@ -442,11 +444,9 @@ assert.strictEqual(util.inspect(-0), '-0');

// test for Array constructor in different context
{
const Debug = vm.runInDebugContext('Debug');
const map = new Map();
map.set(1, 2);
const mirror = Debug.MakeMirror(map.entries(), true);
const vals = mirror.preview();
const vals = previewMapIterator(map.entries(), 100);
const valsOutput = [];
for (const o of vals) {
valsOutput.push(o);
Expand Down
Loading