Async hooks cause programs with frozen promises to crash

Version

v16.13.1

Platform

Darwin MacBook-Pro 21.3.0 Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

Type the following interactively at the Node shell.

~$ node
Welcome to Node.js v16.13.1.
Type ".help" for more information.
> const p = Promise.resolve(8);
undefined
> const names = Reflect.ownKeys(p);
undefined
> names
[
  'domain',
  Symbol(async_id_symbol),
  Symbol(trigger_async_id_symbol),
  Symbol(destroyed)
]
> for (const name of names) {
...   delete p[name];
... }
true
> Object.freeze(p);
Promise { 8 }
> const q = p.then(x => console.log(x));
Uncaught:
TypeError: Cannot add property Symbol(async_id_symbol), object is not extensible
    at getOrSetAsyncId (node:internal/async_hooks:424:34)
    at trackPromise (node:internal/async_hooks:314:35)
    at promiseInitHook (node:internal/async_hooks:322:3)
    at promiseInitHookWithDestroyTracking (node:internal/async_hooks:329:3)
    at Promise.then (<anonymous>)
> undefined
> 8

The 'domain' property is not the main concern here. Rather it is the other three symbols. Hardened JS (aka SES) avoid loading the module that would add 'domain'. However, at the vscode debug terminal, we get a result much like the above but without 'domain' showing up in the list of names. Other differences between the following and the previous section are probably due to the differences caused by Hardened JS and might or might now be relevant. Other than the expected absence of 'domain', the underlying Node bug is the same:

In the debug console:

p = Promise.resolve(8);
Promise {[[PromiseState]]: 'fulfilled', [[PromiseResult]]: 8, Symbol(async_id_symbol): 3543, Symbol(trigger_async_id_symbol): 1783, Symbol(destroyed): {…}}
names = Reflect.ownKeys(p);
(3) [Symbol(async_id_symbol), Symbol(trigger_async_id_symbol), Symbol(destroyed)]
for (const name of names) {
  delete p[name];
}
true
Object.freeze(p);
Promise {[[PromiseState]]: 'fulfilled', [[PromiseResult]]: 8}
q = p.then(x => console.log(x));

That last line will terminate the debug session, with the following error appearing on vscode's JavaScript Debug Terminal

Debugger attached.
  - range queries

  Uncaught exception in test/test-rankOrder.js

  TypeError: Cannot add property Symbol(async_id_symbol), object is not extensible
    at Promise.then (<anonymous>)
    at eval (eval at <anonymous> (packages/store/test/test-rankOrder.js:94:7), <anonymous>:1:13)
    at packages/store/test/test-rankOrder.js:94:7
    at packages/store/test/test-rankOrder.js:90:12
    at async Promise.all (index 7)

  › Promise.then (<anonymous>)
  › eval (eval at <anonymous> (packages/store/test/test-rankOrder.js:94:7), <anonymous>:1:13)
  › packages/store/test/test-rankOrder.js:94:7
  › packages/store/test/test-rankOrder.js:90:12

(TypeError#1)
Waiting for the debugger to disconnect...
TypeError#1: Cannot add property Symbol(async_id_symbol), object is not extensible
  at Promise.then (<anonymous>)
  at eval (eval at <anonymous> (packages/store/test/test-rankOrder.js:94:7), <anonymous>:1:13)
  at packages/store/test/test-rankOrder.js:94:7
  at packages/store/test/test-rankOrder.js:90:12
  at async Promise.all (index 7)

  ✖ test/test-rankOrder.js exited with a non-zero exit code: 1
  ─

  1 test skipped
  1 uncaught exception
Waiting for the debugger to disconnect...
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Waiting for the debugger to disconnect...
Waiting for the debugger to disconnect...

Putting the same code in a .js file and executing it non-interactively does not trigger the bug. The names array in that case is empty, as it should be.

How often does it reproduce? Is there a required condition?

It reproduces reliably. The explanation above should be adequate to reliably reproduce it. Please let me know if you have any trouble reproducing it.

What is the expected behavior?

names should be empty.
q should be defined as a promise that eventually fulfills to p's fulfillment, 8.

What do you see instead?

As presented above:

~$ node
Welcome to Node.js v16.13.1.
Type ".help" for more information.
> const p = Promise.resolve(8);
undefined
> const names = Reflect.ownKeys(p);
undefined
> names
[
  'domain',
  Symbol(async_id_symbol),
  Symbol(trigger_async_id_symbol),
  Symbol(destroyed)
]
> for (const name of names) {
...   delete p[name];
... }
true
> Object.freeze(p);
Promise { 8 }
> const q = p.then(x => console.log(x));
Uncaught:
TypeError: Cannot add property Symbol(async_id_symbol), object is not extensible
    at getOrSetAsyncId (node:internal/async_hooks:424:34)
    at trackPromise (node:internal/async_hooks:314:35)
    at promiseInitHook (node:internal/async_hooks:322:3)
    at promiseInitHookWithDestroyTracking (node:internal/async_hooks:329:3)
    at Promise.then (<anonymous>)
> undefined
> 8

Additional information

Googling led me to
https://stackoverflow.com/questions/70742387/where-can-i-find-any-docs-on-why-node-14-changed-promise-into-promise-undefi
which led me to
f37c26b
and
#36394

However, that PR shows Closed rather than Merged, so it may not actually be the source of the bug.

See also
endojs/endo#627
and
endojs/endo#126

@bmeck

Attn @bmeck

See also microsoft/vscode#144493

However, that PR shows Closed rather than Merged, so it may not actually be the source of the bug.

The commit was landed manually, see #36394 (comment).

Hello I am new to open source but familiar with Mern stack and thus would be really helpful if you provide me resources to contribute to this bug

I'm not really sure why this should be a bug. If you use async hooks (which happens behind the scenes here by using domain) but tinker with the objects in a way that async hooks can't work anymore it's reasonable that a crash happens.

If you don't use async hooks then it's fine:

const ah = require("async_hooks");

ah.createHook({ init: () => {}}).enable(); // remove this line and it works for you

const p = Promise.resolve(8);
const names = Reflect.ownKeys(p);
console.log(names);
for (const name of names) {
  delete p[name];
}
Object.freeze(p);
const q = p.then(x => console.log(x));

There are a lot other cases where tinkering with objects result in crashes, e.g see following http sample:

const http = require("http");

const server = http.createServer((req, res) => {});
Object.freeze(server);
server.listen();

results in

node:net:1317
    this._handle = rval;
                 ^

TypeError: Cannot assign to read only property '_handle' of object '#<Server>'
    at Server.setupListenHandle [as _listen2] (node:net:1317:18)
    at listenInCluster (node:net:1378:12)
    at Server.listen (node:net:1465:7)
    at Object.<anonymous> (C:\work\node-42229\http.js:5:8)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47

The two lines you mention

const ah = require("async_hooks");

ah.createHook({ init: () => {}}).enable(); // remove this line and it works for you

do not appear in my source code. Where would I remove them from? Put another way, how can I omit Node's async_hooks?

If I understand your initial posting correct you use repl to execute your code.
repl uses domain module see here
domain uses async hooks see here

So even the code is not visible it's executed as your setup depends on it.

Edit: As far as I know domain is the only module within nodejs itself which uses async-hooks (besides AsyncLocalStorage which is part of async_hooks module so it's quite oblivious that is it used there).
Additionally I think repl is the only internal module which uses domain. So don't use them and you should be fine.

I don't think there is any switch to remove existence of domain, async_hooks and repl from node.
Clearly once you use any 3rd party package you have to look if they rely on domain and/or async_hooks.

I'm still trying to track down what entrains async_hook, but from what I've gathered so far:

Connecting with Chrome devtools or VS Code JS debugger does trigger this behavior
Connecting with node inspect 127.0.0.1:9229 does not

Here is the test program that I'm using:

const p = Promise.resolve(8);
const names = Reflect.ownKeys(p);
console.log("p own keys", names);
for (const name of names) {
  delete p[name];
}
Object.freeze(p);
debugger;
const q = p.then((x) => console.log(x));
console.log(q);

If the devtool/VSCode are attached either at start, or as late as the debugger statement, then the following line will throw during the .then with the stack reported above.

Regardless of what triggers the creation of the hooks, I believe this is a bug in the async_hooks implementation which should be resilient to the extensibility state of objects it's attempting to modify. In particular here getOrSetAsyncId should not perform an assignment without try/catch. There may be similar cases. Is there actually a reason async_hooks tries to modify objects instead of using a WeakMap to store its internal information?

I think the VS code debugger uses async_hooks internally.

There may be similar cases. Is there actually a reason async_hooks tries to modify objects instead of using a WeakMap to store its internal information?

I guess performance but not 100% sure if this is the only reason.

Adding a try/catch in getOrSetAsyncId would avoid the exception above but it would break the underlying functionality. I think you should not use async hooks if you don't want them.

Regardless of what triggers the creation of the hooks, I believe this is a bug in the async_hooks implementation which should be resilient to the extensibility state of objects it's attempting to modify. In particular here getOrSetAsyncId should not perform an assignment without try/catch. There may be similar cases. Is there actually a reason async_hooks tries to modify objects instead of using a WeakMap to store its internal information?

Node.js will attach async context metadata to any object that can be tracked with async_hooks regardless of whether async_hooks is actively being used or not within code and yes, finding a way of tracking the information without modifying the objects themselves would be ideal. I'd certainly +1 a refactor on that -- though it's likely to be a non-trivial change.

I think you should not use async hooks if you don't want them.

We do not want to use async_hooks. If I could prevent it from loading I would (is there a way without modifying node?). However, we and our users need to be able to attach a debugger, without it crashing the program being debugged.

Node.js will attach async context metadata to any object that can be tracked with async_hooks regardless of whether async_hooks is actively being used or not within code

That is not my observation. If nothing triggers async_hooks (like a debugger), promise objects are not modified and everything runs fine.

I think for the VS code debugger it's possible to disable use of async hooks by adding "showAsyncStacks": false, into the launch config of type "type": "pwa-node".

Thanks, that indeed does it for VS Code!

I would like to re-iterate that:

This was not necessary before async_hooks: use new v8::Context PromiseHook API #36394 landed (aka 14.17.6 and 16.5.0 work fine, but 14.18.0 and 16.6.0 crash)
async_hooks shouldn't cause a program crash if it can't modify an object (promise in this case)

I'll rename the issue to capture this better.

Of course even better would be for async_hooks to not modify objects it doesn't own at all.

you could modify GetAssignedPromiseAsyncId and friends to fall back to the weakmap perhaps?

I'm sure someone could, but I'm not able to myself (it's been 15 years since I touched C code)

These get measured various times all over every year, most recent one on social media was : https://github.com/RafaelGSS/nodejs-bench-operations/blob/main/bench/private-property.js some tweaks to add WeakMap backed checks w/ grouping to ease cost of lookup for example give my local machine:

Node.js Benchmark Operations

Machine: darwin x64 | 8 vCPUs | 32.0GB Mem
Node: v17.2.0
Run: Wed Mar 09 2022 13:59:32 GMT-0600 (Central Standard Time)

private-property.js

Raw usage private field x 13,704,157 ops/sec ±1.53% (86 runs sampled)
Raw usage underscore usage x 9,562,654 ops/sec ±1.74% (88 runs sampled)
Raw usage weakmap usage x 2,224,267 ops/sec ±3.02% (68 runs sampled)
Manipulating private properties using # x 3,715,253 ops/sec ±1.82% (90 runs sampled)
Manipulating "private" properties using underscore(_) x 797,982,230 ops/sec ±0.66% (91 runs sampled)
Manipulating "private" properties using Symbol x 797,110,945 ops/sec ±0.76% (93 runs sampled)
Manipulating private properties using PrivateSymbol x 44,934,479 ops/sec ±1.55% (86 runs sampled)
Manipulating private properties using WeakMap x 979,550 ops/sec ±22.24% (24 runs sampled)
Manipulating private properties using Return Override and private fields x 9,728,517 ops/sec ±3.49% (88 runs sampled)

Newer V8 has fixes for private fields:

Node.js Benchmark Operations

Machine: darwin x64 | 8 vCPUs | 32.0GB Mem
Node: v18.0.0-pre
Run: Wed Mar 09 2022 13:56:45 GMT-0600 (Central Standard Time)

private-property.js

Raw usage private field x 779,883,070 ops/sec ±1.63% (85 runs sampled)
Raw usage underscore usage x 799,159,114 ops/sec ±0.85% (91 runs sampled)
Raw usage weakmap usage x 2,254,865 ops/sec ±4.79% (72 runs sampled)
Manipulating private properties using # x 792,309,165 ops/sec ±0.59% (93 runs sampled)
Manipulating "private" properties using underscore(_) x 797,127,594 ops/sec ±0.43% (92 runs sampled)
Manipulating "private" properties using Symbol x 790,864,710 ops/sec ±1.13% (93 runs sampled)
Manipulating private properties using PrivateSymbol x 46,016,836 ops/sec ±1.23% (87 runs sampled)
Manipulating private properties using WeakMap x 796,352 ops/sec ±37.79% (22 runs sampled)
Manipulating private properties using Return Override and private fields x 53,340,341 ops/sec ±3.59% (86 runs sampled)

WeakMaps are > 600 times slower even with grouping to reduce cost and Return Override is also hellish at ~15 times slower. I don't think they should be used in normal operation.

I'll just leave this (monkeypatch) workaround here. No need to tell the developer to turn off async stacktraces manually

const a = require('async_hooks');
const bkp = a.createHook
const noop = _ => { }
a.createHook = function () {
    //process._rawDebug(Error().stack)
    if (Error().stack.includes('node:internal/inspector_async_hook')) {
        return { enable: noop, disable: noop }
    }
    return bkp.apply(this, arguments)
}

@phoddie

WeakMaps are > 600 times slower [...]

FWIW, this is ridiculous. We should explain the brilliant weakmap representation and algorithm Moddable recently invented and implemented, which avoids the difficult choice between transposed and non-transposed representation. With one adjustment, it could provide reasonable efficiency and complexity measure under all scenarios. Having thought about ephemeron algorithms for a long time, the Moddable solution is by far the best solution I've seen. We should encourage v8 to adopt it.

@phoddie @patrick-soquet , we should talk about publishing your weakmap gc algorithm!

Our implementation of weakmaps in XS is in our repository for all to see. It could be a little challenging to infer the details from the code though. Perhaps we should write something about that.

@phoddie

@phoddie to see if i'm understanding this correctly, wm.set(key, value) basically stores value in a slot on key, and then stores key in a slot on map, so then you can either collect individual entries by visiting key or all entries by visiting map?

Here's my first step to understanding it.

Prior to any form of weakness, normal gc only has OR joins: If A points at C and B points at C then C is reachable if either A is reachable OR B is reachable.

From an API and authority perspective, WeakMaps are asymmetric. But from a GC algorithm perspective, WeakMaps introduce symmetric AND joins into GC. After M[K] = V, V is reachable if M is reachable AND K is reachable. AND is commutative.

The Moddable GC algorithm might notice first either that reachable M at K points at V or that reachable K at M points at V. Whichever it notices first, it then transitions into a representation such that iff it notices the other one is also reachable, then it decides that V is reachable.

While I find the discussions regarding WeakMap algorithms here really interesting I doubt that this will result in a fast fix.

May I ask what are the exact requirements for SES?
If you omit deleting the keys added by async hooks the problem you see vanishes. Is deleting the keys a requirement for SES?

Please note that async hooks expose quite some internals in general. On every init hook you get a reference to the object created. Some of them are node internal objects which end user may never see via the public APIs (e.g. an HTTP Parser,...). This is one of the main reasons why async hooks never exited experimental status.

Therefore I wonder if SES can accept the existence of an installed async hook at all - independent of implementation details like WeakMap/private symbols,...

@Flarna

@Flarna SES guarantees could be broken by the program running in a hardened compartment accessing hooks. That won't be allowed. The issue here is developers using SES should still be able to use devtools/debugger.

@naugtur

To expand on @naugtur's point:

async hooks are probably powerful enough to break out of a locked down Compartment. However that's not relevant, as it wouldn't be a power that should be endowed to untrusted compartments.
The modifications to promises that async hooks cause when enabled leak information to the program on execution across the realm, which poses a confidentiality issue. As such, async_hooks should not be enabled by trusted code, unless it accepts those risks. Using private symbols, private fields, or weakmap by default would remove this confidentiality issue.

In the case of async hooks triggered by debugging, the hooks themselves are never exposed to the program, so I believe there is no endowment issue. Also the action of debugging can only be taken if you are already in a privileged position.

I have some workarounds we can put in place to mitigate this debugging use case, avoiding crashes on frozen promises, while maintaining debugger async traces. However they are not optimal, and I would like to know if some of the following could be considered:

Relax the follwing hasOwn check (it's inconsistent with simple assignment anywhere else):

node/lib/internal/async_hooks.js

Line 422 in 4aae536

if (ObjectPrototypeHasOwnProperty(object, async_id_symbol)) {

Background: the current workaround involves installing accessors on the Promise.prototype to transform into a defineProperty or storing in a WeakMap instance, depending on the extensibility of the promise object. This is probably the easiest change.
Add an internal WeakMap fallback if promise objects are not extensible. I am not sure how to deal with the native code expecting the presence of those symbols tho, as I mentioned above.
Remove the destroyed object bag logic for promises. From what I can tell, it no longer serves any purpose: the native side only ever reads that the destroyed prop is false, and always emits destroy. There isn't really any confidentiality issue or side channel created by this destroyed object bag, since it's simply attached to the promise object and can be frozen just as well, it's just a wart that doesn't seem to have any purpose anymore.

I'd still like to find a solution to avoid leaking the async ids for all promise objects, or at least an option to put the async hooks into a mode where it doesn't create those props, but as I mentioned, there is the issue of native code requiring these symbols's presence.

This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

Navigation Menu

Repository navigation

Async hooks cause programs with frozen promises to crash #42229

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

17 remaining items

Node.js Benchmark Operations

private-property.js

Node.js Benchmark Operations

private-property.js

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Async hooks cause programs with frozen promises to crash #42229

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

Activity

erights commented on Mar 6, 2022

erights commented on Mar 6, 2022

erights commented on Mar 6, 2022

RaisinTen commented on Mar 6, 2022

Harsh-Singh-619 commented on Mar 6, 2022

Flarna commented on Mar 7, 2022

erights commented on Mar 7, 2022

Flarna commented on Mar 7, 2022

mhofman commented on Mar 7, 2022

Flarna commented on Mar 7, 2022

jasnell commented on Mar 7, 2022

mhofman commented on Mar 7, 2022

Flarna commented on Mar 7, 2022

mhofman commented on Mar 7, 2022

17 remaining items

devsnek commented on Mar 9, 2022

mhofman commented on Mar 9, 2022

bmeck commented on Mar 9, 2022

Node.js Benchmark Operations

private-property.js

Node.js Benchmark Operations

private-property.js

naugtur commented on Mar 9, 2022

erights commented on Mar 10, 2022

phoddie commented on Mar 10, 2022

devsnek commented on Mar 11, 2022

erights commented on Mar 11, 2022

Flarna commented on Mar 11, 2022

naugtur commented on Mar 11, 2022

mhofman commented on Mar 14, 2022

github-actions commented on Jun 25, 2026

github-actions commented on Jul 26, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions