-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Garbage Collection Crash #8216
Comments
/cc @indutny |
@sehyoc Thanks for reporting this! Is there any chance you can offer more information? Is the problem reproducible? Do you have something like a test script that can demonstrate the issue? |
Also: do you know if there is a node version where the same code was working, or is this the first version of node you tried? |
I have no other suspects but the AsyncWrap class. @trevnorris looks like calling JS functions is no longer allowed from the |
I agree with Fedor that the call into the VM from the AsyncWrap destructor is responsible. @sehyoc Something in your project (either your own code or a third-party module) is calling |
This problem is reproducible but not everytime. Crashes happen 1 of 5 times during startup. Before I was running v6.3.1 which also crashed but much less frequency. We are using babel with async plug-in. Since we are heavily relying on 'async' keyword, there is no way to turn this off. |
@sehyoc This is not about the |
Yes, I found who was using async_wrap. It was
And furthermore, async-hook was used by: I was using to get long stack trace. Current version of |
It is not allowed anymore to call JS code when collecting weakly persistent handles, it hits the assertion below: # Fatal error in ../deps/v8/src/execution.cc, line 103 # Check failed: AllowJavascriptExecution::IsAllowed(isolate). Remove the call into the VM from the AsyncWrap destructor. This commit breaks the destroy hook but that cannot be helped. Fixes: nodejs#8216
cc/ @nodejs/diagnostics |
any resolution so far? |
ping @trevnorris |
well just to add I see this is still happening in versions 7 too. Easy way for me to reproduce is to install any package using async hooks and fire few requests to make it crash, let me know if you need any help |
I have a fix ready, but don't have a reproducible test case for it. There a minimal one I can include? |
I easily reproduce using this package https://github.com/AndreasMadsen/trace/ (Uses async-hooks) If you give me the branch you are working on I can test my current scenario upon it |
@nakedcity link is #9753 |
Thanks @trevnorris I take a look this weekend |
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. PR-URL: nodejs#9753 Fixes: nodejs#8216 Fixes: nodejs#9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: nodejs#8216 Fixes: nodejs#9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. PR-URL: #9753 Fixes: #8216 Fixes: #9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: nodejs#8216 Fixes: nodejs#9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: nodejs#8216 Fixes: nodejs#9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: #8216 Fixes: #9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. PR-URL: #10096 Fixes: #8216 Fixes: #9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. PR-URL: nodejs#9753 Fixes: nodejs#8216 Fixes: nodejs#9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: #8216 Fixes: #9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Calling JS during GC is a no-no. So intead create a queue of all ids that need to have their destroy() callback called and call them later. Removed checking destroy() in test-async-wrap-uid because destroy() can be called after the 'exit' callback. Missing a reliable test to reproduce the issue that caused the FATAL_ERROR. Fixes: #8216 Fixes: #9465 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
I am also experiencing a similar crash in Node.js version: v6.11.0
|
@surajwy Can you open a new issue and include the contents of the registers and a disassembly of the top frame? Yours is probably a different issue than this bug report. edit: by the way, is that with a binary from https://nodejs.org/? |
@bnoordhuis I have opened a new issue here: #17097 |
Node crashed with following stack trace:
Process finished with exit code 132 (interrupted by signal 4: SIGILL)
The text was updated successfully, but these errors were encountered: