Hey team! Over the past couple days, we noticed a small memory leak in one of our applications, as we enabled a new feature. It was very subtle, but memory usage increased slowly over the hours, quickly reaching our memory limit, which then caused our service to be OOM killed.
This new feature relies on napi-rs (love it ❤️ ). It isn't the first time we use it on that specific app, but it's the first time that we use both Reference and SharedReference, as the types we needed to expose required lifetimes.
From the investigation we conducted, using SharedReference lead to a leak of 32 bytes every time we used it. I made a little reproduction available at https://github.com/lbarthon/napi-leak.
My understanding is that the Reference.share_with method explicitly leaks memory with Box::leak, and then the memory should be freed later on in the drop. When debugging, I added logs in a couple places, and it seemed that the drop was never called - https://github.com/napi-rs/napi-rs/blob/main/crates/napi/src/bindgen_runtime/js_values/value_ref.rs#L140-L143
Also worth noting that when I commented the whole logic around finalize_callbacks, the leak was still there, and it didn't make anything worse.
I reproduced this on an arm64 MacOS machine, running NodeJS 20.11.1 (latest LTS). Given the memory usage we saw on Linux, it should be reproducible on it too.
I would happily send a PR with a patch, but my knowledge of napi & napi-rs is way too little to do so, efficiently. I hope we'll find a solution to that!
Hey team! Over the past couple days, we noticed a small memory leak in one of our applications, as we enabled a new feature. It was very subtle, but memory usage increased slowly over the hours, quickly reaching our memory limit, which then caused our service to be OOM killed.
This new feature relies on
napi-rs(love it ❤️ ). It isn't the first time we use it on that specific app, but it's the first time that we use bothReferenceandSharedReference, as the types we needed to expose required lifetimes.From the investigation we conducted, using
SharedReferencelead to a leak of 32 bytes every time we used it. I made a little reproduction available at https://github.com/lbarthon/napi-leak.My understanding is that the
Reference.share_withmethod explicitly leaks memory withBox::leak, and then the memory should be freed later on in the drop. When debugging, I added logs in a couple places, and it seemed that the drop was never called - https://github.com/napi-rs/napi-rs/blob/main/crates/napi/src/bindgen_runtime/js_values/value_ref.rs#L140-L143Also worth noting that when I commented the whole logic around
finalize_callbacks, the leak was still there, and it didn't make anything worse.I reproduced this on an arm64 MacOS machine, running NodeJS 20.11.1 (latest LTS). Given the memory usage we saw on Linux, it should be reproducible on it too.
I would happily send a PR with a patch, but my knowledge of napi & napi-rs is way too little to do so, efficiently. I hope we'll find a solution to that!