From 9d064439e5716df250363ed6f88198c86eb63521 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 8 Mar 2019 16:42:21 +0100 Subject: [PATCH] async_hooks: remove deprecated emitBefore and emitAfter AsyncResource.emitBefore and AsyncResource.emitAfter have been deprecated in https://github.com/nodejs/node/pull/18632. This PR removes it all. This commit also updates some embedder tests to use internal APIs. The conditions are still possible for Node.js core developers but not for end users. PR-URL: https://github.com/nodejs/node/pull/26530 Reviewed-By: Ruben Bridgewater Reviewed-By: Anna Henningsen Reviewed-By: Ali Ijaz Sheikh Reviewed-By: Benedikt Meurer Reviewed-By: Yang Guo Reviewed-By: Andreas Madsen Reviewed-By: James M Snell --- doc/api/async_hooks.md | 36 ------------------ doc/api/deprecations.md | 5 ++- lib/async_hooks.js | 23 ----------- test/async-hooks/test-callback-error.js | 2 +- .../test-embedder.api.async-resource.js | 24 ++++++------ ...yed.js => test-emit-after-on-destroyed.js} | 38 +++++++++++++------ ...ed.js => test-emit-before-on-destroyed.js} | 38 +++++++++++++------ ...proper-order.js => test-improper-order.js} | 35 ++++++++++++----- ...oper-unwind.js => test-improper-unwind.js} | 34 +++++++++++------ .../test-async-hooks-recursive-stack.js | 16 ++++---- .../test-emit-after-uncaught-exception.js | 12 +++--- 11 files changed, 130 insertions(+), 133 deletions(-) rename test/async-hooks/{test-embedder.api.async-resource.after-on-destroyed.js => test-emit-after-on-destroyed.js} (61%) rename test/async-hooks/{test-embedder.api.async-resource.before-on-destroyed.js => test-emit-before-on-destroyed.js} (61%) rename test/async-hooks/{test-embedder.api.async-resource.improper-order.js => test-improper-order.js} (63%) rename test/async-hooks/{test-embedder.api.async-resource.improper-unwind.js => test-improper-unwind.js} (65%) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index fe4f87c9eff1b9..8878c03aa73b28 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -659,41 +659,6 @@ of the async resource. This will establish the context, trigger the AsyncHooks before callbacks, call the function, trigger the AsyncHooks after callbacks, and then restore the original execution context. -#### asyncResource.emitBefore() - -> Stability: 0 - Deprecated: Use [`asyncResource.runInAsyncScope()`][] instead. - -Call all `before` callbacks to notify that a new asynchronous execution context -is being entered. If nested calls to `emitBefore()` are made, the stack of -`asyncId`s will be tracked and properly unwound. - -`before` and `after` calls must be unwound in the same order that they -are called. Otherwise, an unrecoverable exception will occur and the process -will abort. For this reason, the `emitBefore` and `emitAfter` APIs are -considered deprecated. Please use `runInAsyncScope`, as it provides a much safer -alternative. - -#### asyncResource.emitAfter() - -> Stability: 0 - Deprecated: Use [`asyncResource.runInAsyncScope()`][] instead. - -Call all `after` callbacks. If nested calls to `emitBefore()` were made, then -make sure the stack is unwound properly. Otherwise an error will be thrown. - -If the user's callback throws an exception, `emitAfter()` will automatically be -called for all `asyncId`s on the stack if the error is handled by a domain or -`'uncaughtException'` handler. - -`before` and `after` calls must be unwound in the same order that they -are called. Otherwise, an unrecoverable exception will occur and the process -will abort. For this reason, the `emitBefore` and `emitAfter` APIs are -considered deprecated. Please use `runInAsyncScope`, as it provides a much safer -alternative. - #### asyncResource.emitDestroy() * Returns: {AsyncResource} A reference to `asyncResource`. @@ -713,7 +678,6 @@ never be called. `AsyncResource` constructor. [`after` callback]: #async_hooks_after_asyncid -[`asyncResource.runInAsyncScope()`]: #async_hooks_asyncresource_runinasyncscope_fn_thisarg_args [`before` callback]: #async_hooks_before_asyncid [`destroy` callback]: #async_hooks_destroy_asyncid [`init` callback]: #async_hooks_init_asyncid_type_triggerasyncid_resource diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index eeafab91dac398..1383c279c14496 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1905,6 +1905,9 @@ should start using the `async_context` variant of `MakeCallback` or ### DEP0098: AsyncHooks Embedder AsyncResource.emitBefore and AsyncResource.emitAfter APIs -Type: Runtime +Type: End-of-Life The embedded API provided by AsyncHooks exposes `.emitBefore()` and `.emitAfter()` methods which are very easy to use incorrectly which can lead diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 7ba5f40f18ebc4..107c92c97d670c 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -127,17 +127,6 @@ function createHook(fns) { const destroyedSymbol = Symbol('destroyed'); -let emitBeforeAfterWarning = true; -function showEmitBeforeAfterWarning() { - if (emitBeforeAfterWarning) { - process.emitWarning( - 'asyncResource.emitBefore and emitAfter are deprecated. Please use ' + - 'asyncResource.runInAsyncScope instead', - 'DeprecationWarning', 'DEP0098'); - emitBeforeAfterWarning = false; - } -} - class AsyncResource { constructor(type, opts = {}) { validateString(type, 'type'); @@ -169,18 +158,6 @@ class AsyncResource { } } - emitBefore() { - showEmitBeforeAfterWarning(); - emitBefore(this[async_id_symbol], this[trigger_async_id_symbol]); - return this; - } - - emitAfter() { - showEmitBeforeAfterWarning(); - emitAfter(this[async_id_symbol]); - return this; - } - runInAsyncScope(fn, thisArg, ...args) { emitBefore(this[async_id_symbol], this[trigger_async_id_symbol]); let ret; diff --git a/test/async-hooks/test-callback-error.js b/test/async-hooks/test-callback-error.js index 07293e0315931c..06d8cb7224ccad 100644 --- a/test/async-hooks/test-callback-error.js +++ b/test/async-hooks/test-callback-error.js @@ -19,7 +19,7 @@ switch (arg) { onbefore: common.mustCall(() => { throw new Error(arg); }) }).enable(); const resource = new async_hooks.AsyncResource(`${arg}_type`); - resource.emitBefore(); + resource.runInAsyncScope(() => {}); return; case 'test_callback_abort': diff --git a/test/async-hooks/test-embedder.api.async-resource.js b/test/async-hooks/test-embedder.api.async-resource.js index 74d6c478c835bb..19c1b7187a9e2c 100644 --- a/test/async-hooks/test-embedder.api.async-resource.js +++ b/test/async-hooks/test-embedder.api.async-resource.js @@ -47,16 +47,16 @@ assert.strictEqual(typeof alcaEvent.asyncId(), 'number'); assert.notStrictEqual(alcaEvent.asyncId(), alcaTriggerId); assert.strictEqual(alcaEvent.triggerAsyncId(), alcaTriggerId); -alcaEvent.emitBefore(); -checkInvocations(alcazares, { init: 1, before: 1 }, - 'alcazares emitted before'); -alcaEvent.emitAfter(); +alcaEvent.runInAsyncScope(() => { + checkInvocations(alcazares, { init: 1, before: 1 }, + 'alcazares emitted before'); +}); checkInvocations(alcazares, { init: 1, before: 1, after: 1 }, 'alcazares emitted after'); -alcaEvent.emitBefore(); -checkInvocations(alcazares, { init: 1, before: 2, after: 1 }, - 'alcazares emitted before again'); -alcaEvent.emitAfter(); +alcaEvent.runInAsyncScope(() => { + checkInvocations(alcazares, { init: 1, before: 2, after: 1 }, + 'alcazares emitted before again'); +}); checkInvocations(alcazares, { init: 1, before: 2, after: 2 }, 'alcazares emitted after again'); alcaEvent.emitDestroy(); @@ -75,11 +75,11 @@ function tick1() { assert.strictEqual(typeof poblado.uid, 'number'); assert.strictEqual(poblado.triggerAsyncId, pobTriggerId); checkInvocations(poblado, { init: 1 }, 'poblado constructed'); - pobEvent.emitBefore(); - checkInvocations(poblado, { init: 1, before: 1 }, - 'poblado emitted before'); + pobEvent.runInAsyncScope(() => { + checkInvocations(poblado, { init: 1, before: 1 }, + 'poblado emitted before'); + }); - pobEvent.emitAfter(); checkInvocations(poblado, { init: 1, before: 1, after: 1 }, 'poblado emitted after'); diff --git a/test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js b/test/async-hooks/test-emit-after-on-destroyed.js similarity index 61% rename from test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js rename to test/async-hooks/test-emit-after-on-destroyed.js index a389eddf421cc8..b015dacddba506 100644 --- a/test/async-hooks/test-embedder.api.async-resource.after-on-destroyed.js +++ b/test/async-hooks/test-emit-after-on-destroyed.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter, emitDestroy +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,20 +22,29 @@ if (process.argv[2] === 'child') { // Once 'destroy' has been emitted, we can no longer emit 'after' // Emitting 'before', 'after' and then 'destroy' - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); - event1.emitDestroy(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + emitDestroy(asyncId); + } // Emitting 'after' after 'destroy' - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - event2.emitDestroy(); - - console.log('heartbeat: still alive'); - event2.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + emitDestroy(asyncId); + console.log('heartbeat: still alive'); + emitAfter(asyncId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js b/test/async-hooks/test-emit-before-on-destroyed.js similarity index 61% rename from test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js rename to test/async-hooks/test-emit-before-on-destroyed.js index d6465a8445282d..36a50a050f9324 100644 --- a/test/async-hooks/test-embedder.api.async-resource.before-on-destroyed.js +++ b/test/async-hooks/test-emit-before-on-destroyed.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter, emitDestroy +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,20 +22,29 @@ if (process.argv[2] === 'child') { // Once 'destroy' has been emitted, we can no longer emit 'before' // Emitting 'before', 'after' and then 'destroy' - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); - event1.emitDestroy(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + emitDestroy(asyncId); + } // Emitting 'before' after 'destroy' - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - event2.emitDestroy(); - - console.log('heartbeat: still alive'); - event2.emitBefore(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + emitDestroy(asyncId); + console.log('heartbeat: still alive'); + emitBefore(asyncId, triggerId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.improper-order.js b/test/async-hooks/test-improper-order.js similarity index 63% rename from test/async-hooks/test-embedder.api.async-resource.improper-order.js rename to test/async-hooks/test-improper-order.js index 4a38d87ce21e1f..a9d46917df59e0 100644 --- a/test/async-hooks/test-embedder.api.async-resource.improper-order.js +++ b/test/async-hooks/test-improper-order.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -17,18 +22,28 @@ if (process.argv[2] === 'child') { // Async hooks enforce proper order of 'before' and 'after' invocations // Proper ordering - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - event1.emitBefore(); - event1.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event1', triggerId, {}); + emitBefore(asyncId, triggerId); + emitAfter(asyncId); + } // Improper ordering // Emitting 'after' without 'before' which is illegal - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); - - console.log('heartbeat: still alive'); - event2.emitAfter(); + { + const asyncId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(asyncId, 'event2', triggerId, {}); + + console.log('heartbeat: still alive'); + emitAfter(asyncId); + } } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/async-hooks/test-embedder.api.async-resource.improper-unwind.js b/test/async-hooks/test-improper-unwind.js similarity index 65% rename from test/async-hooks/test-embedder.api.async-resource.improper-unwind.js rename to test/async-hooks/test-improper-unwind.js index cb9e338905c386..a62512db974ac6 100644 --- a/test/async-hooks/test-embedder.api.async-resource.improper-unwind.js +++ b/test/async-hooks/test-improper-unwind.js @@ -1,13 +1,18 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); const assert = require('assert'); -const async_hooks = require('async_hooks'); -const { AsyncResource } = async_hooks; +const internal_async_hooks = require('internal/async_hooks'); const { spawn } = require('child_process'); const corruptedMsg = /async hook stack has become corrupted/; const heartbeatMsg = /heartbeat: still alive/; +const { + newAsyncId, getDefaultTriggerAsyncId, + emitInit, emitBefore, emitAfter +} = internal_async_hooks; + const initHooks = require('./init-hooks'); if (process.argv[2] === 'child') { @@ -21,23 +26,28 @@ if (process.argv[2] === 'child') { // The first test of the two below follows that rule, // the second one doesn't. - const event1 = new AsyncResource('event1', async_hooks.executionAsyncId()); - const event2 = new AsyncResource('event2', async_hooks.executionAsyncId()); + const eventOneId = newAsyncId(); + const eventTwoId = newAsyncId(); + const triggerId = getDefaultTriggerAsyncId(); + emitInit(eventOneId, 'event1', triggerId, {}); + emitInit(eventTwoId, 'event2', triggerId, {}); // Proper unwind - event1.emitBefore(); - event2.emitBefore(); - event2.emitAfter(); - event1.emitAfter(); + emitBefore(eventOneId, triggerId); + emitBefore(eventTwoId, triggerId); + emitAfter(eventTwoId); + emitAfter(eventOneId); // Improper unwind - event1.emitBefore(); - event2.emitBefore(); + emitBefore(eventOneId, triggerId); + emitBefore(eventTwoId, triggerId); console.log('heartbeat: still alive'); - event1.emitAfter(); + emitAfter(eventOneId); } else { - const args = process.argv.slice(1).concat('child'); + const args = ['--expose-internals'] + .concat(process.argv.slice(1)) + .concat('child'); let errData = Buffer.from(''); let outData = Buffer.from(''); diff --git a/test/parallel/test-async-hooks-recursive-stack.js b/test/parallel/test-async-hooks-recursive-stack.js index 7ab73dc1bf4538..bc4ac86e7f1ca1 100644 --- a/test/parallel/test-async-hooks-recursive-stack.js +++ b/test/parallel/test-async-hooks-recursive-stack.js @@ -7,14 +7,14 @@ const async_hooks = require('async_hooks'); function recurse(n) { const a = new async_hooks.AsyncResource('foobar'); - a.emitBefore(); - assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); - assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); - if (n >= 0) - recurse(n - 1); - assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); - assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); - a.emitAfter(); + a.runInAsyncScope(() => { + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + if (n >= 0) + recurse(n - 1); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + }); } recurse(1000); diff --git a/test/parallel/test-emit-after-uncaught-exception.js b/test/parallel/test-emit-after-uncaught-exception.js index 368099d48355be..5003972e9984aa 100644 --- a/test/parallel/test-emit-after-uncaught-exception.js +++ b/test/parallel/test-emit-after-uncaught-exception.js @@ -26,12 +26,12 @@ setImmediate(common.mustCall(() => { // Create a stack of async ids that will need to be emitted in the case of // an uncaught exception. const ar1 = new async_hooks.AsyncResource('Mine'); - ar1.emitBefore(); - - const ar2 = new async_hooks.AsyncResource('Mine'); - ar2.emitBefore(); - - throw new Error('bye'); + ar1.runInAsyncScope(() => { + const ar2 = new async_hooks.AsyncResource('Mine'); + ar2.runInAsyncScope(() => { + throw new Error('bye'); + }); + }); // TODO(trevnorris): This test shows that the after() hooks are always called // correctly, but it doesn't solve where the emitDestroy() is missed because