Browse Source

src: clear async id stack if bootstrap throws

If bootstrap throws and if ids are added to the async id stack and if
the exception wasn't handled by the fatal exception handler then the
AsyncCallbackScope destructor will cause the AsyncHooks::pop_ids() stack
check to fail. Causing the application to crash. So clear the async id
stack manually.

This is only possible if the user: 1) manually calls MakeCallback() or
2) uses async await in the top level. Which will cause _tickCallback()
to fire before bootstrap finishes executing.

The following example shows how the application can fail due to
exceeding the maximum call stack while using async await:

    async function fn() {
      fn();
      throw new Error();
    }
    (async function() { await fn(); })();

If this occurs during bootstrap then the application will pring the
following warning a number of times then exit with a status of 0:

    (node:*) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: *): Error

Here's the same recursive call done after enabling a new AsyncHook() the
following will print instead of the above warning and exit with a
non-zero code (currently it's 7 because of how node::FatalException
assigns error codes based on where the failure happened):

    script.js:25
    async function fn() {
                     ^
    RangeError: Maximum call stack size exceeded
        at <anonymous>
        at fn (script.js:25:18)
        at fn (script.js:26:3)
        ....

This has to do with how Promises lazily enable PromiseHook if an
AsyncHook() is enabled. Whether these need to be made uniform is outside
the scope of this commit

Fixes: https://github.com/nodejs/node/issues/15448
PR-URL: https://github.com/nodejs/node/pull/15553
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
canary-base
Trevor Norris 7 years ago
parent
commit
92d7e81b73
No known key found for this signature in database GPG Key ID: 251CA676820DC7F3
  1. 11
      src/node.cc
  2. 21
      test/parallel/test-async-wrap-pop-id-during-load.js

11
src/node.cc

@ -3753,7 +3753,16 @@ void LoadEnvironment(Environment* env) {
// who do not like how bootstrap_node.js sets up the module system but do
// like Node's I/O bindings may want to replace 'f' with their own function.
Local<Value> arg = env->process_object();
f->Call(Null(env->isolate()), 1, &arg);
auto ret = f->Call(env->context(), Null(env->isolate()), 1, &arg);
// If there was an error during bootstrap then it was either handled by the
// FatalException handler or it's unrecoverable (e.g. max call stack
// exceeded). Either way, clear the stack so that the AsyncCallbackScope
// destructor doesn't fail on the id check.
// There are only two ways to have a stack size > 1: 1) the user manually
// called MakeCallback or 2) user awaited during bootstrap, which triggered
// _tickCallback().
if (ret.IsEmpty())
env->async_hooks()->clear_async_id_stack();
}
static void PrintHelp() {

21
test/parallel/test-async-wrap-pop-id-during-load.js

@ -0,0 +1,21 @@
'use strict';
require('../common');
if (process.argv[2] === 'async') {
async function fn() {
fn();
throw new Error();
}
(async function() { await fn(); })();
// While the above should error, just in case it dosn't the script shouldn't
// fork itself indefinitely so return early.
return;
}
const assert = require('assert');
const { spawnSync } = require('child_process');
const ret = spawnSync(process.execPath, [__filename, 'async']);
assert.strictEqual(ret.status, 0);
assert.ok(!/async.*hook/i.test(ret.stderr.toString('utf8', 0, 1024)));
Loading…
Cancel
Save