Discordia
Discordia copied to clipboard
SinisterRectus
Fix Emitter memory leak
Listeners created by the Emitter:once() method get put into an internal table, but are never removed from said table, causing them to never be GC'd. This PR fixes this issue by making the internal table's keys be weak.
(It also slightly simplifies the code used to clean the listener tables).
I have a feeling this won't work. Was this tested? Because numbers are not a subject of being weak for GC as I remember
@GitSparTV The table is not array-like; keys are Listener objects, which should work fine here.
I stumbled upon this issue when I noticed that some of my objects (which used Listeners created by Emitter:once()) weren't being GC'd, and this fix solved that.
It does https://github.com/SinisterRectus/Discordia/blob/f6aea20be3050c55f005c8a990fcb2138fec5bbd/libs/utils/Emitter.lua#L50-L55
Ok last question, when the emitter is not yet emitted, will the listener object live? What if GC cycle happens before emitting?
There should also be a reference to the listener in the self._listeners[listener.eventName] table (from which it is removed correctly when the event is fired/the listener is removed).
Looking at my code, it looks like I never clean false emitters from the table unless the emission hits the "once" code? I think I meant to add a call to clean somewhere else, like after emitter removal, but never went back to review it. I wonder if we should do that here.
https://github.com/SinisterRectus/Discordia/blob/4736f7e54ffcdbdd2537d3eae6a4b186b42e82e0/libs/utils/Emitter.lua#L83-L86 Pretty sure the listeners[i] = false could be replaced with a table.remove(listeners, i) call (of course, you'd have to iterate backwards).
https://github.com/SinisterRectus/Discordia/blob/4736f7e54ffcdbdd2537d3eae6a4b186b42e82e0/libs/utils/Emitter.lua#L54-L63 Same here. You wouldn't even need clean() if you did that unless you were planning to somehow optimize it.