Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear kResourceStore in timers and intervals #53408

Open
mcollina opened this issue Jun 10, 2024 · 4 comments · May be fixed by #53443
Open

Clear kResourceStore in timers and intervals #53408

mcollina opened this issue Jun 10, 2024 · 4 comments · May be fixed by #53443
Labels
async_hooks Issues and PRs related to the async hooks subsystem.

Comments

@mcollina
Copy link
Member

As noted in this article, the timeout/interval objects keep a reference to the store even after they have been cleared.

I think it would be better to clear up that reference inside the clearTimeout() and clearInterval() functions. We might also want to consider exposing an API for end-users and explicit lifetime tracking.

@mcollina
Copy link
Member Author

mcollina commented Jun 10, 2024

cc @nodejs/diagnostics @anonrig

@Flarna Flarna added the async_hooks Issues and PRs related to the async hooks subsystem. label Jun 10, 2024
@Qard
Copy link
Member

Qard commented Jun 10, 2024

setTimeout should probably actually just clear immediately after running given that it only occurs once. But yes, also clearing in clearInterval makes sense to me too.

@Flarna
Copy link
Member

Flarna commented Jun 11, 2024

I guess this is not only relevant for timers and intervals, all resources are effected.

const { AsyncLocalStorage, AsyncResource } = require('node:async_hooks');
const als = new AsyncLocalStorage();
let r, p;
als.run("I consume a lot memory", () => {
  r = new AsyncResource("aResource", { requireManualDestroy: true })
  r.emitDestroy()
  p = Promise.resolve()
})
console.log(r, p);
 AsyncResource {
  [Symbol(async_id_symbol)]: 2,
  [Symbol(trigger_async_id_symbol)]: 1,
  [Symbol(kResourceStore)]: 'I consume a lot memory'
} Promise {
  undefined,
  [Symbol(async_id_symbol)]: 3,
  [Symbol(trigger_async_id_symbol)]: 1,
  [Symbol(kResourceStore)]: 'I consume a lot memory'
}

For timers/intervals it's likely easier to find the place to cleanup.
Maybe we can extend the cleanup to some more places?

@mcollina
Copy link
Member Author

Absolutely!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants