From ba93c8d87deaa5943b50408bf89f7500fcf27bd8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 11 May 2020 02:30:02 +0200 Subject: [PATCH] async_hooks: clear async_id_stack for terminations in more places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Termination exceptions are similar to uncaught exceptions in that they should clear the async id stack, because no ongoing async callbacks will be brought to completion when execution terminates. Previously, there was a check that made sure that that happened when the termination occurred during the callback itself, but no such check was in place for the case that the termination occurred during microtasks started by them. This commit adds such a check, both for microtasks and the `nextTick` queue. The latter addition doesn’t fix a crash, but still makes sense conceptually. The condition here is also flipped from applying only on Worker threads to also applying on the main thread, and setting the `failed_` flag rather than reading it. The former makes sense because the public C++ `Stop(env)` API can have the same effect as worker thread termination, but on the main thread rather than a Worker thread. PR-URL: https://github.com/nodejs/node/pull/33347 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: David Carlier --- src/api/callback.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index f7e7ddedfae377..c8934e1cd33a36 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -84,9 +84,13 @@ void InternalCallbackScope::Close() { closed_ = true; if (!env_->can_call_into_js()) return; - if (failed_ && !env_->is_main_thread() && env_->is_stopping()) { - env_->async_hooks()->clear_async_id_stack(); - } + auto perform_stopping_check = [&]() { + if (env_->is_stopping()) { + MarkAsFailed(); + env_->async_hooks()->clear_async_id_stack(); + } + }; + perform_stopping_check(); if (!failed_ && async_context_.async_id != 0 && !skip_hooks_) { AsyncWrap::EmitAfter(env_, async_context_.async_id); @@ -109,6 +113,8 @@ void InternalCallbackScope::Close() { if (!tick_info->has_tick_scheduled()) { MicrotasksScope::PerformCheckpoint(env_->isolate()); + + perform_stopping_check(); } // Make sure the stack unwound properly. If there are nested MakeCallback's @@ -136,6 +142,7 @@ void InternalCallbackScope::Close() { if (tick_callback->Call(env_->context(), process, 0, nullptr).IsEmpty()) { failed_ = true; } + perform_stopping_check(); } MaybeLocal InternalMakeCallback(Environment* env,