-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
8.2.0 crashes test #14381
Comments
Fascinating, we rolled out a lot of fixes for this issue in 8.2.0. Did you get a core dump that you can share with us? If you don't like to share it, could you run the following and show us the output: npm install -g llnode
llnode /path/to/bin/node -c /path/to/coredump
v8 bt I hope I got that correct, see https://github.com/nodejs/llnode for more info. Also, it assumes that you have xCode installed. |
/cc @nodejs/async_hooks |
I have two files under /cores
and
|
Thanks, that is a huge help. If the issue is easily reproducible, could you add the following code such it runs before anything else? That should output a stack trace that I would like to see. Error.stackTraceLimit = Infinity;
const async_hooks = require('async_hooks');
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
if (triggerAsyncId === undefined || asyncId === undefined || triggerAsyncId <= 0 || asyncId <= 0) {
process._rawDebug('init', {asyncId, type, triggerAsyncId});
throw new Error('bad async id');
}
}
}).enable(); For @nodejs/async_hooks: This is fascinating, it appears that the |
Confirmed, same here:
The error comes from https://www.npmjs.com/package/shot, just running |
@AndreasMadsen this is the stacktrace you are looking for:
|
@mcollina there are so many different causes for this error, it is rarely the same issue. Could you open a new issue and also show the backtrace for the coredump. @crespowang I would still like to see the stack trace. For @nodejs/async_hooks: I'm suspecting the 0fd4c73 fix is showing some issues that were previously hidden. |
@AndreasMadsen same problem for me /usr/local/Cellar/node/8.2.0/bin/node[68190]: ../src/env-inl.h:131:void node::Environment::AsyncHooks::push_ids(double, double): Assertion `(trigger_id) >= (0)' failed.
1: node::Abort() [/usr/local/Cellar/node/8.2.0/bin/node]
2: node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, char const*, int, v8::Local<v8::Value>*, node::async_context) [/usr/local/Cellar/node/8.2.0/bin/node]
3: node::AsyncWrap::PushAsyncIds(v8::FunctionCallbackInfo<v8::Value> const&) [/usr/local/Cellar/node/8.2.0/bin/node]
4: 0x3720c4438525 Node installed with |
Hey @AndreasMadsen, did you want me to add the code in the test? If yes, then this is the output
|
OutgoingMessage needs a async-hooks enabled socket to work, but we support also basic streams. This PR init the async-hooks bits for the passed stream if it is needed. PR-URL: nodejs#14389 Fixes: nodejs#14386 Fixes: nodejs#14381
@crespowang Thanks. Looks like this revealed another (less significant issue). Given the number of issues regarding |
@crespowang if you happen to have a testcase that consistently fails (independent of any |
Fixes: #14386 Fixes: #14381 PR-URL: #14387 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
thanks for fixing this so quickly, v8.2.1 fixed the problem, as a node newbie can I ask how long does it take for a new version to appear in docker hub? |
Fixes: #14386 Fixes: #14381 PR-URL: #14387 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
After upgrading node to v8.2.0 from v7, some of my tests are failing
I see the discussion here #13325 and #13548, not sure if they are related.
The text was updated successfully, but these errors were encountered: