Memory leak in 7.2.0

[frelars]

Upgrading from 7.1.2 to 7.2.0 caused a memory leak in my application. We use snappy heavily to compress/decompress (synchronously) massive amount of data.

I can enable/disable leak by changing snappy version and nothing else. Will slowly eat memory until app crashes in 7.2.0, while will continue forever in 7.1.2

[kurkle]

2nd that, but our leak happens with 7.1.2 too. Reported in #96. Which was not fixed by 7.2.0 (or 7.2.0 introduced a new leak).

[frelars]

Update; I may have upgrade from 7.1.1, not 7.1.2. I will recheck this later today or at least this week. I see I downgraded to 7.1.1 on the system affected.

[oyyd]

Can you help to confirm that if you were using the sync api, i.e. compressSync() and uncompressSync, with Node.js Buffer as input when you found memory leak? Because I can only reproduce when playing with such usage.

[frelars]

Can confirm sync ops with buffer

[kurkle]

2nd that

[Brooooooklyn]

Sorry I've been so busy with work lately. Start looking at this issue today

[oyyd]

This looks like a snappy issue (rather than napi itself):

snappy/src/lib.rs

Line 46 in 7116824

Either::B(b) => Ok(Data::Buffer(b.into_ref()?)),

Sync api doesn't release refereneces so buffers are kept in memory. Though it's a bit strange to me that async api does into_ref() and unref() only once.

[oyyd]

Incidentally, you should use the async apis if possible as it utilize the libuv threads of Node.js so it should be more performant and won't block Node.js's main thread.

[Brooooooklyn]

@frelars @kurkle please try 7.2.1

[frelars]

Hi, I'm currently burried in work and it is not trivial to test different version (must stop current service, reprocess massive amount of data). Hopefully I will get some time tomorrow or wednesday

[kurkle]

It will be couple of days here too, before I have the time to set up the test.

[kevcodez]

Hi, first off, a big thank you for finding the ghosts I've been trying to catch for weeks. We also heavily (de)-compress buffers synchronously. Upgrading to 7.2.1 unfortunately did not fix the memory leak, while downgrading to 7.1.1 did.

Node v16.18.0

Linux version 5.15.0-1021-aws (buildd@lcy02-amd64-045) (gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #25~20.04.1-Ubuntu SMP Thu Sep 22 13:59:08 UTC 2022

[Brooooooklyn]

@kevcodez please try 7.2.1

[kevcodez]

@Brooooooklyn That's what I did - the memory leak persists. We've had this memory leak shortly after upgrading to 7.1.2 for a few weeks now and could not pinpoint it due too many other changes. We ran 7.2.1 for a few hours and the memory leak was clearly not fixed. After downgrading to 7.1.1, we can clearly see, that the memory leak is gone.

Here's a screenshot of the average memory usage in the past 24h.

The hard drops are restarts due to out-of-memory exceptions or new deployments.

[oyyd]

Switch the code to v7.1.1 and rebuild, I could see that v7.1.1 do retains low memory usage(about 80~120MB) compared to v7.2.1 (about 360MB) in my Mac, with same napi version, Node.js version and test code.

But I can't see clear memory leaks of v7.2.1, compared to v7.2.0 of which memory usage increases to 3GB+ very quickly with sync api.