neon: A possible memory leak in callbacks/root
Hi,
I was evaluating the new N-API codebase, and found quite fast greatly leaking piece of code. Here’s the test repo that will show the leak:
https://github.com/pimeys/hello-neon
Now, I did some digging with Valgrind:
As you can see the culprit is in napi_create_reference, called from Root::new. You can dig into the results from the included massif dump! I was not able to collect this data, even by forcing global.gc(), meaning we probably do something wrong in the native side.
I’d expect this loop to have a steady flat memory profile, the data should be collected in the young-space already…
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 1
- Comments: 17 (6 by maintainers)
Commits related to this issue
- fix(neon-runtime): Fix a leak from references not being deleted Fixes https://github.com/neon-bindings/neon/issues/746 — committed to neon-bindings/neon by kjvalencik 3 years ago
- fix(neon-runtime): Fix a leak from references not being deleted Fixes https://github.com/neon-bindings/neon/issues/746 — committed to neon-bindings/neon by kjvalencik 3 years ago
I found the issue and I’ll get a PR up soon with a fix. The issue boils down to a misunderstanding of how N-API references work.
I had interpreted that once the reference count hit
0, they would be automatically cleaned up. However, I must have missed the critical section of that docs that they also need to be deleted. The leak wasn’t showing up in the V8 heap or the Rust structs because it was a N-API struct leaking.https://nodejs.org/api/n-api.html#n_api_napi_delete_reference
tl;dr – It’s not a bug in
unsafecode. It’s a missing call tonapi_delete_referenceto fully drop the data structure.After this change, the process sits at 26MB indefinitely.
Confirmed there’s no leak in the V8 heap with the Chrome inspector but
topshows the steady process memory growth. @kjvalencik found a possible source of the leak but I have to run errands for a bit, we’ll test out fixes later this afternoon. I’ll keep the thread updated with our progress…