Repository navigation
AsyncLocalStorage.disable() strange behaviour #48480
Description
Activity
Sir , i don't use Linux Machines but still according to me for The steps that would reproduce the bug will be
1.Create a new TypeScript file (e.g., bug.ts) and copy the code into it.
2.Install the required dependencies by running the following command:
npm install @types/node3.Compile the TypeScript code to JavaScript using the TypeScript compiler (tsc) by running:
npx tsc bug.ts4.Run the generated JavaScript file using Node.js with the --experimental-async-hooks flag to enable the experimental async_hooks module:
node --experimental-async-hooks bug.js
When executing the code, it should run without any errors, and all assertions should pass.
Now this code that can be replaced with the above one 👍import { AsyncLocalStorage } from 'async_hooks';
import assert = require('assert');const als = new AsyncLocalStorage<{foo: string}>();
assert(als.getStore() === undefined);
als.enterWith({ foo: 'bar' });
assert(als.getStore()?.foo === 'bar');
als.disable();
assert(als.getStore() === undefined);
als.run({ foo: 'zoo' }, () => {
assert(als.getStore()?.foo === 'zoo');
assert(als.getStore() === undefined); // Move the assertion here
});// all assertions should pass without any issueThis might work ..... Just a small Thinking
Hi,
Thanks for your response.
The compiled JS code is as follows:
const { AsyncLocalStorage } = require('async_hooks'); const assert = require('assert'); const als = new AsyncLocalStorage(); assert(als.getStore() === undefined); als.enterWith({ foo: 'bar' }); assert(als.getStore()?.foo === 'bar'); als.disable(); assert(als.getStore() === undefined); als.run({ foo: 'zoo' }, () => { assert(als.getStore()?.foo === 'zoo'); }); assert(als.getStore() === undefined);
You can run it without the experimental flag (as I'm using node 18), and it still fails...
I don't see how the code you posted will work, as you assert that the store is not empty and then immediately assert that it is empty (I validated that claim)Can you explain what it is you are suggesting? Also changing the assertions order might get the code to work, but that doesn't change the fact that disable doesn't work as expected, as least as far as I understand what the
disablefunction should do.The doc says following:
Disables the instance of AsyncLocalStorage. All subsequent calls to asyncLocalStorage.getStore() will return undefined until asyncLocalStorage.run() or asyncLocalStorage.enterWith() is called again.==> the store is re-enabled by calling
.run()as expected.Hi, indeed, but it can be interpreted that it can continue to operate after you disable it, but not with the previously created contexts. i.e. instead of calling it
destroy, and then making any subsequent calls to fail, they call itdisable.The docs also say
When calling asyncLocalStorage.disable(), all current contexts linked to the instance will be exited.Which did not happen
In addition, it statesCalling asyncLocalStorage.disable() is required before the asyncLocalStorage can be garbage collectedThis makes you believe that once it is disabled, it severs all ties to the resources bound (same as the first quote I gave)
What I'm trying to say is that the docs are confusing, and maybe the current behavior is as intended and in that case, the docs need to be more explicit about it
Hi, indeed, but it can be interpreted that it can continue to operate after you disable it, but not with the previously created contexts.
Could you point to the location in your sample where an old context gets "reactivated"? I see a new
.run()call which re-enabled ALS and creates a new entry in the store which is then returned.Do you have a reproducer that the ALS instance is not garbage collected after calling
disableand not calling.run()afterwards?Hi, the last line demonstrates a reused context, I added comments in the code below I hope it is clear
const { AsyncLocalStorage } = require('async_hooks'); const assert = require('assert'); const als = new AsyncLocalStorage(); assert(als.getStore() === undefined); als.enterWith({ foo: 'bar' }); // here we enter context C1 assert(als.getStore()?.foo === 'bar'); als.disable(); assert(als.getStore() === undefined); // here context C1 doesn't exist as we called disdable als.run({ foo: 'zoo' }, () => { // We create a new context C2 but it is isolated for the callback run and does not affect the last line as expected assert(als.getStore()?.foo === 'zoo'); }); assert(als.getStore() === undefined); // This is not undefined and resuses context C1. If we never created C1 in the first place, this would be undefined and will not access C2
I know for a fact that it gets garbage collected because if I don't call disable, I get memory issues under certain situations. I'm not saying it is not garbage collected in this issue, but that the docs are a bit misleading - saying that it can be garbage collected infers that it has nothing holding on to it to prevent it from GC, which means it can't be reused.
als.run()is not isolated for the callback.The concrete store on the concrete resource is removed at the end of
run()but stillrun()enables the context propagation machinery used internally byAsyncLocalStore(as of now based on async hooks) - see here.wow ok this is enlighting - ok I understand what is going on now - I still believe that the docs are a bit misleading. thanks!
Version
v18.12.1
Platform
Linux machine 5.15.0-73-generic #80-Ubuntu SMP Mon May 15 15:18:26 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
No response
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
reproduces every time
What is the expected behavior? Why is that the expected behavior?
According to the docs
I would expect that contexts will be exited
What do you see instead?
if you keep using the storage after disable, the context is restored
Additional information
No response