Inconsistent behaviour in `vm.runInNewContext()`

[alexlamsl]

• Version: 8.9.4 (also tested 8.6.0
• Platform: Windows 10 x64
• Subsystem: vm

The test case in https://github.com/mishoo/UglifyJS2/pull/2574 uncovers a difference in exceptional behaviour between node and node-chakracore - here's a reduced test case:

test.js

var code = [
    "try {",
    "    var a = A, b = 1;",
    "    throw a;",
    "} catch (e) {",
    "    console.log(b);",
    "}",
].join("\n");
eval(code);
require("vm").runInNewContext(code, { console: console });

node-chakracore

$ nvs use chakracore
$ node test.js
undefined
1

node

$ nvs use node/0.10
$ node test.js
undefined
undefined

$ nvs use node/0.12
$ node test.js
undefined
undefined

$ nvs use node/4
$ node test.js
undefined
undefined

$ nvs use node/6
$ node test.js
undefined
undefined

$ nvs use node/8
$ node test.js
undefined
undefined

$ nvs use node/9
$ node test.js
undefined
undefined

[MSLaguana]

Looks like this is because of how we try to create a v8-style context. Within the context, we create a proxy and assign it as the prototype of the global object, but this seems to mean that the A access goes to the proxy getter function and isn't detected as not-defined, and so no exception is thrown as part of var a = A.

While I was playing around with this, I also saw some strange behavior in node-v8:

> var o = {a: 1}
undefined
> vm.createContext(o)
{ a: 1 }
> vm.runInContext("Object.hasOwnProperty(this, 'a')", o)
false
> vm.runInContext("Object.getOwnPropertyDescriptor(this, 'a')", o)
{ value: 1, writable: true, enumerable: true, configurable: true }

I think we need to better understand what exactly these contexts do (and what they are supposed to do) and then we might need some upstream chakra changes to try and match v8's behavior.

[MSLaguana]

Ah nevermind, I was mis-using hasOwnProperty; should have been this.hasOwnProperty('a') and then it works as expected.

It seems that the main downfall of our current approach is the fact that the property interceptors are put on the prototype of the global object, rather than the global object itself (which is currently not possible in chakracore).

[MSLaguana]

I believe that for a proper fix here we'll need to add support for interceptors on the global object / having a proxy as a global object.

Without going that far, there are a few stop-gap options I've been considering to handle most of the use-cases:

1. Take a snapshot of the global object before and after every RunInContext, and apply the diff to the sandbox. This could be done via Object.getOwnPropertyDescriptors(theGlobal) before and after, then iterating through each descriptor and seeing what has changed. This doesn't give full compatibility though, since during execution things may differ (e.g. vm.runInContext("var x = 1; delete x; x", o) won't delete a property x from the sandbox object, since there isn't a diff between the before and after)
2. Do something with directHostObject. Chakra's global object has some support for a global which is backed by some other object. However, it isn't as flexible as a full proxy; there is a fixed order of where we look for whether a property exists and so on, and I don't think we can allow node's interceptor callbacks to do things like "if I have a property but it is read only, then do nothing".

[MSLaguana]

Just noticed that this issue isn't linked to https://github.com/nodejs/node-chakracore/issues/420 like it should be.

[alexlamsl]

Still an issue as of v10.0.0

[alexlamsl]

Still an issue as of v10.1.0

[alexlamsl]

Still an issue as of v10.6.0

[alexlamsl]

Problem persists as of v10.13.0