Fix Emitter memory leak

[RiskoZoSlovenska]

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).

[GitSparTV]

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

[RiskoZoSlovenska]

@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.

[GitSparTV]

Does :once create Listener object?

[RiskoZoSlovenska]

It does https://github.com/SinisterRectus/Discordia/blob/f6aea20be3050c55f005c8a990fcb2138fec5bbd/libs/utils/Emitter.lua#L50-L55

[GitSparTV]

Ok last question, when the emitter is not yet emitted, will the listener object live? What if GC cycle happens before emitting?

[RiskoZoSlovenska]

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).

[GitSparTV]

Nice

[SinisterRectus]

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.

[RiskoZoSlovenska]

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.

[SinisterRectus]

Listeners are intentionally marked for deletion instead of directly removed to avoid unexpected behavior due to table contraction while actively iterating that table. Marked listeners are cleaned only after the emit loop exits and all listeners have been visited. I now remember that this is also why I do not clean listeners in the removeListener method: This method can be called during an event callback, and therefore, during listener table iteration.

[SinisterRectus]

My emitter implementation in the sandbox branch does not use the "once" table, so that leak is fixed there and will eventually merge into an active 3.0 branch (probably dev).

I've also just committed a change to the sandbox so that listener tables are now correctly marked in the removeListener method.

There is still the issue where the listener table can theoretically grow unbounded if a user continuously adds and removes listeners. To fix this, I think I need to use a different data structure than a simple Lua table.