From 68abe0fe122131dc0d8f55a5ad48e2fce25add07 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 2 May 2018 14:05:42 +0200 Subject: [PATCH 1/3] deps: cherry-pick ca76392 from upstream V8 Original commit message: Correctly run before/after hooks for await. This fixes a bug where we didn't run before/after hooks for await when the debugger is not active, as reported downstream in https://github.com/nodejs/node/issues/20274 Change-Id: I1948d1884c591418d87ffd1d0ccb2bebf4e908f1 Reviewed-on: https://chromium-review.googlesource.com/1039386 Commit-Queue: Benedikt Meurer Reviewed-by: Yang Guo Cr-Commit-Position: refs/heads/master@{#52909} Refs: v8/v8@ca76392 Fixes: #20274 --- common.gypi | 2 +- deps/v8/src/builtins/builtins-async-gen.cc | 2 +- deps/v8/test/cctest/test-api.cc | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/common.gypi b/common.gypi index d1304a14478d2c..3bb10d611fb1d5 100644 --- a/common.gypi +++ b/common.gypi @@ -27,7 +27,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.5', + 'v8_embedder_string': '-node.6', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/src/builtins/builtins-async-gen.cc b/deps/v8/src/builtins/builtins-async-gen.cc index 7958afba007d5e..ff4d8f654ba9cf 100644 --- a/deps/v8/src/builtins/builtins-async-gen.cc +++ b/deps/v8/src/builtins/builtins-async-gen.cc @@ -44,7 +44,7 @@ void AsyncBuiltinsAssembler::Await(Node* context, Node* generator, Node* value, // When debugging, we need to link from the {generator} to the // {outer_promise} of the async function/generator. Label done(this); - GotoIfNot(IsDebugActive(), &done); + GotoIfNot(IsPromiseHookEnabledOrDebugIsActive(), &done); CallRuntime(Runtime::kSetProperty, native_context, generator, LoadRoot(Heap::kgenerator_outer_promise_symbolRootIndex), outer_promise, SmiConstant(LanguageMode::kStrict)); diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index fe1aca5a0fb440..8d00dfd9b53d71 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -18107,6 +18107,12 @@ TEST(PromiseHook) { CHECK_EQ(v8::Promise::kFulfilled, GetPromise("p")->State()); CHECK_EQ(9, promise_hook_data->promise_hook_count); + promise_hook_data->Reset(); + source = "(async() => await p)();\n"; + + CompileRun(source); + CHECK_EQ(11, promise_hook_data->promise_hook_count); + delete promise_hook_data; isolate->SetPromiseHook(nullptr); } From 6326ada0d8f66acec543e08440958e606335babc Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 3 May 2018 01:18:14 +0200 Subject: [PATCH 2/3] test: add regression test for async_hooks This makes sure the hooks are properly called in V8. Refs: https://github.com/nodejs/node/issues/20274 --- ...test-async-hooks-async-await-regression.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/parallel/test-async-hooks-async-await-regression.js diff --git a/test/parallel/test-async-hooks-async-await-regression.js b/test/parallel/test-async-hooks-async-await-regression.js new file mode 100644 index 00000000000000..c44c79963522ee --- /dev/null +++ b/test/parallel/test-async-hooks-async-await-regression.js @@ -0,0 +1,25 @@ +'use strict'; + +const common = require('../common'); + +const async_hooks = require('async_hooks'); +const assert = require('assert'); + +common.crashOnUnhandledRejection(); + +const asyncIds = []; + +async_hooks.createHook({ + init: (asyncId, type, triggerAsyncId, resource) => { + asyncIds.push(`${triggerAsyncId} => ${asyncId}`); + } +}).enable(); + +async function main() { + console.log('hello'); + await null; + console.log('world'); +} + +main().then(() => assert.deepStrictEqual( + asyncIds, [ '1 => 3', '1 => 4', '1 => 5', '3 => 6', '3 => 7' ])); From da7eaa8ef0fac2444f6892753c9db5cae39f6e1a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 9 May 2018 15:52:37 +0200 Subject: [PATCH 3/3] fixup: test --- .../test-async-hooks-async-await-regression.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-async-hooks-async-await-regression.js b/test/parallel/test-async-hooks-async-await-regression.js index c44c79963522ee..722c89662de0a7 100644 --- a/test/parallel/test-async-hooks-async-await-regression.js +++ b/test/parallel/test-async-hooks-async-await-regression.js @@ -11,7 +11,7 @@ const asyncIds = []; async_hooks.createHook({ init: (asyncId, type, triggerAsyncId, resource) => { - asyncIds.push(`${triggerAsyncId} => ${asyncId}`); + asyncIds.push([triggerAsyncId, asyncId]); } }).enable(); @@ -21,5 +21,14 @@ async function main() { console.log('world'); } -main().then(() => assert.deepStrictEqual( - asyncIds, [ '1 => 3', '1 => 4', '1 => 5', '3 => 6', '3 => 7' ])); +main().then(() => { + // Verify correct relationship between ids, e.g.: + // '1 => 3', '1 => 4', '1 => 5', '3 => 6', '3 => 7' + assert.strictEqual(asyncIds[0][0], asyncIds[1][0]); + assert.strictEqual(asyncIds[0][0], asyncIds[2][0]); + assert.strictEqual(asyncIds[0][1], asyncIds[3][0]); + assert.strictEqual(asyncIds[0][1], asyncIds[4][0]); + assert.strictEqual(asyncIds[0][1] + 1, asyncIds[1][1]); + assert.strictEqual(asyncIds[0][1] + 2, asyncIds[2][1]); + assert.strictEqual(asyncIds[3][1] + 1, asyncIds[4][1]); +});