lavalink-devs / lavaplayer

Lavaplayer fork maintained by Lavalink
Apache License 2.0
187 stars 51 forks source link

Fix thread leak due to race condition #125

Closed freyacodes closed 5 months ago

freyacodes commented 5 months ago

https://github.com/lavalink-devs/lavaplayer/commit/f602c4794f80d01ee1b57326aabd50097f992a86 fixes a thread leak in lavaplayer that appears to be caused by a race condition. I was able to reproduce the issue with some test code, with the leak no longer occurring after that commit. I can provide the test code used if relevant.

Specifically, the leak concerns LocalAudioTrackExecutor, which each control a thread. The audio buffer is leaked as well. This first commit makes the executor no longer reusable, meaning that once stopped a new one must be created, which happens automatically now.

However, I am still observing a similar leak of LocalAudioTrackExecutor when using Lavalink in a real working environment. https://github.com/lavalink-devs/lavaplayer/commit/895b7cda7f65a7a6a186e81b4369f07257ca8a88 was an attempt at eliminating places where the executor could leak. Heap analysis shows that the stop() function of LocalAudioTrackExecutor is not being called in rare cases, leading to a leak. Any help in resolving this second leak would be greatly appreciated. I suspect that these two leaks are separate issues, so merging this PR would likely still reduce the leak to an extent.

devoxin commented 5 months ago

LGTM