AsyncResource behaves differently w/ AsyncLocalStorage in v24.0.0

[bengl]

Version

v24.0.0

Platform

Darwin [computer name] 24.4.0 Darwin Kernel Version 24.4.0: Fri Apr 11 18:33:47 PDT 2025; root:xnu-11417.101.15~117/RELEASE_ARM64_T6000 arm64 arm Darwin

Subsystem

async_hooks

What steps will reproduce the bug?

const { AsyncLocalStorage, AsyncResource } = require('async_hooks')
const { strictEqual } = require('assert')

const storage = new AsyncLocalStorage()

storage.enterWith(1)
const ar = new AsyncResource('test')
ar.runInAsyncScope(() => {
  storage.enterWith(2)
  ar.runInAsyncScope(() => strictEqual(storage.getStore(), 2))
})

Running this in Node.js 24.0.0 breaks, and it passes on all earlier versions.

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

This is reproducible as long as --no-async-context-frame is not set.

What is the expected behavior? Why is that the expected behavior?

This should pass, as it does on previous versions. If a storage is entered with a given store in a runAsyncContext() block of an AsyncResource, then future invocations of getStore() should return store as long as no other enterWith() has been called on that storage.

What do you see instead?

node:assert:95
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 2

    at /path/to/broken-async.js:10:28
    at AsyncResource.runInAsyncScope (node:async_hooks:214:14)
    at /path/to/broken-async.js:10:6
    at AsyncResource.runInAsyncScope (node:async_hooks:214:14)
    at Object.<anonymous> (/path/to/broken-async.js:8:4)
    at Module._compile (node:internal/modules/cjs/loader:1734:14)
    at Object..js (node:internal/modules/cjs/loader:1899:10)
    at Module.load (node:internal/modules/cjs/loader:1469:32)
    at Module._load (node:internal/modules/cjs/loader:1286:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 1,
  expected: 2,
  operator: 'strictEqual'
}

Node.js v24.0.0

It looks like it's reverting to the store that was active when the AsyncResource was created.

Additional information

The test case provided is a whittled-down version of something we do in legacy instrumentations in dd-trace-js that pre-date tracingChannel. Re-implementing those instrumentations using tracingChannel solves this, since we're then not doing anything too strange with AsyncResources like we are here.

[Qard]

That seems expected, actually. AsyncResource is meant to capture its value at construction and restore that value around each run. It's a bit weird that the value could be mutated by an enterWith within itself. It should persist into new descending tasks, but AsyncResource should not mutate like this.

The difference seems to be that with old async_hooks the captured value is accidentally getting mutated by the enterWith somehow when it is active, probably because of how it stored the context values directly on the resource objects. I'd have to take a closer look at why it was behaving that way.

I would say from the semantics though that it seems to me like a bug that it ever behaved this way. 🤔

[jasnell]

I'd argue that enterWith(...) is the realy bug here ;-)

Snark aside, while I generally agree that the new behavior is likely preferable we should consider this a regression.

[Qard]

The prior implementation also suffers from a time travel bug of sorts. It can mutate already inflight async branches which results in strange behaviours like this:

const { AsyncLocalStorage, AsyncResource } = require('async_hooks')
const storage = new AsyncLocalStorage()

storage.enterWith("foo")
const ar = new AsyncResource('test')
ar.runInAsyncScope(() => {
  // The "foo" value flows into this asynchronous branch.
  setImmediate(() => {
    // However, this restores the AsyncResource temporally _after_ the enterWith below,
    // and so it uses that mutated value of the AsyncResource.
    ar.runInAsyncScope(() => {
      console.log(storage.getStore()) // "bar"
    })
  })

  // Generally one would assume this would change only the _following_ execution in this scope
  // and not modify the value of already created branches which lead to a restore of the AsyncResource.
  storage.enterWith("bar")
})

Basically AsyncResource is supposed to be a snapshot of state at the point at which it is constructed, which it then sets as the store state when it is run. But because of the previous implementation of ALS stamping the store value directly on the current resource, these behaviours form a circular dependency of sorts which results in a modification loop which could actually leak out into everything which uses AsyncResource.

That is to say, yep...enterWith(...) is kind of the bug here, though equally as much so is AsyncResource. Neither is particular great as neither are protected well or specified clearly enough.

As for calling this a regression, I think what makes the most sense here is to document the difference. Trying to adapt the new code to behave the same as the original would be not only quite complicated, as it would need to start keeping track of constructed AsyncResource instances to modify whenever an AsyncContextFrame change happens (which would be expensive), but it would also be objectively worse than the new behaviour. AsyncResource is essentially equivalent to AsyncContext.Snapshot in that it's supposed to capture state at construction time to restore later for some scope. By making it accidentally mutable, it violates a whole lot of the flow guarantees which context management is supposed to provide.

[jasnell]

Hmm.. yeah, I agree with that also... hmm... given that enterWith(...) is still technically experimental and given the issues, I think I agree that the best approach here is to document. I want to be sensitive, however, to anything this may have broken ... which I assume is why @bengl opened the issue. I wonder if there's an approach to telling AsyncResource to update its internal snapshot, something like...

const storage = new AsyncLocalStorage()

storage.enterWith(1)
const ar = new AsyncResource('test')
ar.runInAsyncScope(() => {
  storage.enterWith(2);
  ar.refreshSnapshot();  // <-- tells the AsyncResource to take a new snapshot of the async context
  ar.runInAsyncScope(() => strictEqual(storage.getStore(), 2))
})

[bengl]

@jasnell

I want to be sensitive, however, to anything this may have broken ... which I assume is why @bengl opened the issue.

For my purposes, I have a workaround, which is to re-implement the relevant code using far better primitives like tracingChannel. That said, I think anyone wanting to be that deliberate can just make another AsyncResource instance locking in the new state. 🤔

[Flarna]

The problem with using AsyncResource to capture/activate a context is the global scope:

const { AsyncLocalStorage, AsyncResource } = require("node:async_hooks");

const als1 = new AsyncLocalStorage();
const als2 = new AsyncLocalStorage();

als1.run("one", () => {
  const ar = new AsyncResource("ar1");
  als2.run("two", () => {
    console.log(als1.getStore()); // one
    console.log(als2.getStore()); // two
    ar.runInAsyncScope(() => {
      console.log(als1.getStore()); // one
      console.log(als2.getStore()); // undefined
    });
  });
});

As you can see both als instances are effected from the AsyncResource.

While this is fine and expected if used by library programmers being friendly to ALS users (e.g. some DB client) it's usually an unintended side effect if done by monitoring tools like OpenTelemetry.

I'd argue that enterWith(...) is the realy bug here ;-)

Yes and no. It's the combination of enterWith() and run(). Using only enterWith() or only run() is usually easier to understand then combining it.
Using tracingChannel is effective using only run so you are back on the safe track.