labstreaminglayer / liblsl-Java

Java bindings for liblsl
MIT License
5 stars 10 forks source link

Repeated calls to LSL.StreamInlet .info().channel_count() leaks memory #9

Open boulder-on opened 4 months ago

boulder-on commented 4 months ago

The following code will leak memory

while (true) {
   float[] sample = new float[inlet.info().channel_count()];
   inlet.pull_sample(sample);
}

Whereas this works properly

float[] sample = new float[inlet.info().channel_count()];
while (true) {
   inlet.pull_sample(sample);
}

I my experiment, I was collecting data at about 1 kHz. After an hour using the first snippet, my Java process was using about 20 GB. This obviously has an easy work around, but it does indicate that there is memory leaking somewhere (inside the native code?).

cboulay commented 4 months ago

I'm not very good with Java, but the difference between the two snippets is that you're calling new to create new memory inside the loop vs outside. I don't know if you need to do manual cleanup here or if Java is expected to do garbage cleanup when you exit scope. This doesn't seem like an LSL issue to me... unless you got the first snippet from a provided example?

cboulay commented 4 months ago

If you found the pattern in the first snippet (new inside a while loop) in any example code or liblsl documentation then please open a documentation-focused issue.

boulder-on commented 4 months ago

The new float[], whether inside or outside the loop isn't a problem, The garbage collector can handle the extra allocations without problem. Allocating in the loop isn't great form. I've tested the following and it will show the memory leak as well.

while (true) {
   inlet.info().channel_count();
}

Looking deeper into the code, the StreamInfo needs to be destroyed in order to avoid the leak.

while (true) {
   var info = inlet.info();
   info.channel_count();
   info.destroy();
}

A better idiom here would be to have StreamInfo implement AutoCloseable. That way the IDE can let you know that you are not closing the resource.

As the examples are written, they all leak memory by not destroying the StreamInfo. Albeit, they leak very little. A super quick look over the codebase shows there are Pointer objects in many places, all of which could represent a memory leak if they are not destroyed.

cboulay commented 4 months ago

Ah, I understand better now, thanks. Well, I am not a Java developer and I doubt @tstenner is using liblsl on Android anymore. So I think this can only be fixed with a community PR.