-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
vm: recent contextify changes cause segmentation fault in node v5.9 #5768
Comments
I can reproduce this with 5.9.0. This doesn't seem to fail on |
This should fail on |
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). |
@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 |
Agree (and sorry for the churn). |
@ofrobots any idea how long a proper fix will take? I was considering making a known issue test with your reproduction from #5768 (comment) |
Work-in-progress fix: ofrobots@95d45ce. |
OK, looks like you've got it under control :-) |
When the previous set of changes (bfff07b) it was possible to have the context get garbage collected while sandbox was still live. We need to tie the lifetime of the context to the lifetime of the sandbox. Fixes: nodejs#5768 PR-URL: nodejs#5786
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:
$ 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
The text was updated successfully, but these errors were encountered: