[Node Express] Memory Leak on >=v7.84

[rodolfoBee]

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?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

7.84.00

Framework Version

Express v4.19.2 Node.js v20.11.1

Link to Sentry event

No response

Reproduction Example/SDK Setup

A user is reporting Memory leak on Sentry SDK on an Express project. This issue is observed using v7.84.0, v7.102.0 and 8.17.0.

Steps to Reproduce

Configuration is as on the screenshots:

Expected Result

No Leaks

Actual Result

The Grafana dashboards shows the memory usage on the test Express server, you can see how shortly before 14:50 the first pod is handling the incoming requests and memory increases and continuous to increase up to ±1.25GB of memory and then stalls (~14:55). As more requests are coming in, 2 new pods are spun up until they stall (~14:55). More pods are started as other pods stall with memory usage increasing all the time. This pattern repeats a third time as pods stall at ~15:02 and ~15:03. After stopping the incoming requests, some garbage collection happens (~150MB), but never to the mem usage at startup (or close to).

Exact same code, just the Sentry config removed from the webserver. As you can see from the screenshot (red and orange lines), the memory usage is at or below the mem usage of the server running Sentry, but never higher and it stays at the same level of ~470MB throughout the test. After stopping the incoming requests to the server, some memory is released again (~100MB).

The below tests are from a local machine using the heap profiler. Snapshot 1 being made right after startup, snapshot 2 about a minute into the test, and snapshot 3 taken at 2 minutes after it. From snapshots 2 and 3 (screenshots below), it seems like the beforeExit event handler from sdk.js in the @sentry/core package contains a memory leak as it doesn’t seem to allow the array to be garbage collected.

All the tests were run using autocannon to generate ~230 requests per 10 second interval until I stopped the process using the command autocannon -r20 -f .

[lforst]

Hi, I don't have your github handle yet but thanks to whoever reported this. From the memory snapshot it looks like there is something called "beforeExit" holding a bunch of references to most likely Sentry Hub objects (can't say that with 100% confidence from looking at the screenshot).

We have one place in the SDK on the version you're using where we create "beforeExit" handlers and it matches with sdk.js:244:

sentry-javascript/packages/node/src/sdk.ts

Lines 248 to 264 in 4f4bd32

	function startSessionTracking(): void {
	const hub = getCurrentHub();
	hub.startSession();
	// Emitted in the case of healthy sessions, error of `mechanism.handled: true` and unhandledrejections because
	// The 'beforeExit' event is not emitted for conditions causing explicit termination,
	// such as calling process.exit() or uncaught exceptions.
	// Ref: https://nodejs.org/api/process.html#process_event_beforeexit
	process.on('beforeExit', () => {
	const session = hub.getScope().getSession();
	const terminalStates: SessionStatus[] = ['exited', 'crashed'];
	// Only call endSession, if the Session exists on Scope and SessionStatus is not a
	// Terminal Status i.e. Exited or Crashed because
	// "When a session is moved away from ok it must not be updated anymore."
	// Ref: https://develop.sentry.dev/sdk/sessions/
	if (session && !terminalStates.includes(session.status)) hub.endSession();
	});
	}

I don't think that place should leak in that way, unless startSessionTracking is called over and over again, which is only called from within Sentry.init() - which is per definition not allowed to be called over and over again and known to leak memory (more or less unavoidably unfortunately).

Can you verify that you are calling Sentry.init() only once per process? Thanks!

[stefanmirck]

Hi @lforst, I reported this issue to @rodolfoBee. Thanks for looking into the issue and your analysis of what may be causing the memory leak.

I’m currently preparing new tests with v8.27.0 of the SDK, and will also look into how often Sentry.init() is called.

[stefanmirck]

I’ve completed a new round of the tests using Sentry v8.27.0 and it seems like the memory leak issues have been resolved. Your analysis of the issue turned out to be spot on @lforst!

During the first test I ran with v8.27.0 and since you had mentioned it, I noticed from the logs that Sentry.init() was being called multiple times. On further investigation, I found some server middleware calling Sentry.init() on each function call. 😰 After removing it from the middleware and running a new test memory usage of the server remained stable (with just 2 active pods being able to handle the incoming requests).

(The heap profile didn’t show anything noticeable in regards to the Sentry SDK, so I haven’t attached it.)

As it seems the upgrade to v8.27.0 together with fixing the middleware do not show any signs of a memory leak, I’ve pushed the fix upstream and asked QA to take a look as well. Thanks for taking a closer look and helping out with this issue @lforst! And thanks @rodolfoBee for guiding the process! 🙇‍♂

[lforst]

@stefanmirck glad to hear you could figure it out! I will close this issue to keep our stream clean but please feel free to reach out any time if there are any more questions.