vm: recent contextify changes cause segmentation fault in node v5.9

[raymondfeng]

A recent change made to node_contextify.cc that was released in node 5.9.0 causes node to segfault.

The offending commit is this: bfff07b

The problem can be reproduced using the following test script:

var vm = require('vm');

function f() {
 var sandbox = {};
 vm.createContext(sandbox);

 return function(script, ctx) {
 var s = new vm.Script(script);
 for(var p in ctx) { sandbox[p] = ctx[p]; };
 var result = s.runInContext(sandbox);
 console.log(result, sandbox);
 for(var p in sandbox) { delete sandbox[p]; };
 }
}

for(var i=0; i<10000; i++) {
 f()('x = 3', {x : 1});
 f()('x = 4', {x : 2});
}

$ node -v
v5.9.0
$ uname -a
Darwin raymond-117.local 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64

cc @ofrobots and @bnoordhuis

[ofrobots]

I can reproduce this with 5.9.0. This doesn't seem to fail on master. I'm investigating.

[ofrobots]

This should fail on master too, but it is dependent on GC timing. I am working on a fix.

[ofrobots]

Here's a minimal test-case:

// Flags: --expose-gc
var sandbox = {x: 1};
vm.createContext(sandbox);
gc();
vm.runInContext("x = 2", sandbox);

Associating the newly created V8 context with the contextified sandbox in a gc-visible fashion would be the easiest way to solve this problem, and I am trying to figure out a way to do that.

In any case, I want to think through the solution carefully as the object graph here is a bit complex. In the meanwhile, if a fix is more urgently needed, it may be worth reverting commits from #5392 in 5.9.1 (/cc @evanlucas).

[evanlucas]

@ofrobots I almost think we should revert and push out v5.9.1 pretty quick. I'm open to other opinions though. /cc @nodejs/release

[ofrobots]

Agree (and sorry for the churn).

[cjihrig]

@ofrobots any idea how long a proper fix will take? I was considering making a known issue test with your reproduction from #5768 (comment)

[ofrobots]

Work-in-progress fix: ofrobots@95d45ce.

[cjihrig]

OK, looks like you've got it under control :-)