search

RobLoach / node-raylib

Node.js bindings for Raylib
https://robloach.github.io/node-raylib/
Other
233 stars 20 forks source link

[bug?] a memory leak? #155

Open DanielMazurkiewicz opened 2 years ago

DanielMazurkiewicz commented 2 years ago

https://github.com/RobLoach/node-raylib/blob/4887ee14392e01883921909de0382fb53f6c1f1f/tools/generate_templates/node-raylib-bindings.js#L222

I might be missing something, but I didn't see any place that does free it, is not that a memory leak?

RobLoach commented 2 years ago

Good question... There was a CleanUp process in the old bindings. Perhaps the new ones are missing that clean step.

konsumer commented 2 years ago

I could be wrong, but it's my understanding that NAPI manages all the FromValue/ToValue stuff. I was worried about memory-leaks at first, but it seems like it calls & frees them in the normal js GC lifecycle. It might be a good idea to try to auto-detect leaks just in case, but outside of these To/From calls nothing should be allocated, so I think we are mostly safe.

Are strings causing an actual problem?

twuky commented 2 years ago

I'm also really not an expert when it comes to working with memory in C but looking at it now, this probably does leak? The way the bindings are supposed to look, the string itself should be okay to discard once the function its used for is done (and it wouldnt affect anything the user has saved in JS, its just a copy). So we probably need to add another condition to the code generator for functions with string arguments:

I'm just not sure personally what it looks like to free that memory, i basically only know enough about C to have gotten the bindings to compile...

konsumer commented 2 years ago

Maybe a first step is to verify our idea of this (as we disagree on if it's a leak or not.) How do we determine it is? This seems like it might help detect them. So, what is the test for it? Do we run a bunch of node-raylib functions that pass strings, and see if we hit a leak?