nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.03k stars 29.29k forks source link

AsyncLocalStore looses context in MakeCallback #43038

Open Flarna opened 2 years ago

Flarna commented 2 years ago

Version

18.1.0, 14.19.1 (and most likely a lot others)

Platform

Tested on Microsoft Windows NT 10.0.22000.0 x64 but very likely platform independent.

Subsystem

async_hooks

What steps will reproduce the bug?

Use node::MakeCallback() with a receiver different to the AsyncResource.

See e.g. snappy 6.x which uses Nan::AsyncWorker.

In this setup the complete callback is not called with the AsyncResource as target and this results in setting the wrong object as current resource in MakeCallback.

Calling callbacks with undefined/null/global as target is not that uncommon.

The before/after callbacks are called with the correct asyncId but AsyncLocalStore relies on the current resource.

const { executionAsyncId, AsyncLocalStorage } = require("async_hooks")
const snappy = require("snappy")
const als = new AsyncLocalStorage()

als.run(15, () => {
  snappy.compress("Hello World!", () => {
    const cbId = executionAsyncId()
    console.log(`compressed: ${cbId}, als: ${als.getStore()}`)
  })
})

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

Always if MakeCallback is used with a receiver different then the AsyncResource object.

What is the expected behavior?

no context loss

What do you see instead?

context loss

Additional information

No response

Flarna commented 2 years ago

fyi @nodejs/async_hooks

RafaelGSS commented 2 years ago

It's happening only in Windows?

Flarna commented 2 years ago

I haven't tried any other OS but I assume it's os independent. Just filled the form.

Also node down to at least 14 is effected.