Since version 7.48.0 sentry leaks AsyncLocalStorage instance

[kamilchlebek]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/node

SDK Version

7.64.0

Steps to Reproduce

Since version 7.48.0 (till the latest version) we experience memory leaks while using @sentry/node.
After my investigation it's clear that this commit 4856685 is the root cause of our issues.

This is because for every execution of Sentry.init another AsyncLocalStorage is created and latter attached to global object:

sentry-javascript/packages/node/src/async/hooks.ts

Line 19 in 65df869

const asyncStorage = new (async_hooks as NewerAsyncHooks).AsyncLocalStorage<Hub>();

The most important part (and tricky one) is that we call Sentry.init multiple times.
Once when node.js bootstraps, then for every upcoming request that generates angular application (we use similar approach to this one: https://dev.to/raghavgarg1257/angular-universal-add-error-logger-like-sentry-1hdd, so CSR uses @sentry/browsers but for angular universal (SSR) we pass @sentry/node).

It used to work since 2019 without any issue.

If this issue can't be fixed we'll try to setup single sentry instance and then pass it to angular universal app to use it.

Expected Result

Don't leak AsyncLocalStorage instance. Maybe reuse AsyncLocalStorage when Sentry.init is called multiple times?

Actual Result

Our CPU usage since v7.48.0 (goes back to normal when rolledback to v7.47.0)

[lforst]

Thanks for writing in. Memory leaks are veeery bad and we hate them. Let me see if I can fix this.

Because memory leaks are very annoying to reproduce, would you mind applying a patch to your code and trying it out when I push a PR? If not, no big deal!

[kamilchlebek]

Thanks for immediate response! 🙏

would you mind applying a patch to your code and trying it out

No problem at all. I just applied your changes by editing node_modules directly.

It fixes the issue 🚀

[lforst]

@kamilchlebek Thank you so much for trying it out!

We will probably merge this but I will hold out just a tiny bit because I want to get some more opinions on this change if it could cause problems down the line. We will likely release this fix today if we don't see any issues with it.