-
-
Couldn't load subscription status.
- Fork 33.6k
vm: fix crash on fatal error in debug context #1229
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just assign it? Is there any point in rolling it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two reasons:
- The environment doesn't really belong on the debug context. This is a hack to stop it from crashing until we have better ways to deal with this.
- It introduces a potential use-after-free because the pointer stays around when the non-debug context is disposed.
|
LGTM, with a small comment. |
|
LGTM, please land it ;) |
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. Fixes: nodejs#1190 PR-URL: nodejs#1229 Reviewed-By: Fedor Indutny <fedor@indutny.com>
c91855e to
cf081a4
Compare
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.
Fixes: #1190
R=@indutny?
https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/356/