search

Stray / robotlegs-utilities-RelaxedEventMap

Just what it says - a robotlegs eventMap that lets you be a little more relaxed about race conditions and late-arriving views
17 stars 2 forks source link

Port to v2 & potential leak #4

Open creynders opened 11 years ago

creynders commented 11 years ago

Hey @Stray

Just to let you know I ported this to v2 and there was one thing I noticed however, even though the unmapRelaxedListener accepts an ownerObject parameter, it isn't used in the method. Am I right in assuming this can lead to potential memory leaks if a method of the owner object is used as a listener? The object itself is used as a weak key for the dictionary, but if one of its methods was used as a listener, there will be a reference to the object in the function method closure which obviously is stored as a value in the dictionary. I seem to remember this isn't counted as a circular reference and therefore isn't marked for GC, but could be I'm totally mistaken. What do you think?

Stray commented 11 years ago

Ooh, interesting - yes, because the RelaxedMediator uses the unmapListenersFor function it looks like I've not finished the unmapRelaxedListener behaviour internally. Looking through my code bases I've never used it - I obviously provided it for symmetry but then couldn't make that parameter meaningful.

I seem to recall that I found that it wasn't trivial to do it - it would involve some sort of key system for the anonymous functions that handle the unmappings (as they can't be inspected to find which one matches the other params).

I probably meant to come back to it.

It possibly could cause a memory leak unless you do unmapListenersFor when disposing of an object - I think my RelaxedMediator was the pragmatic solution for that as it does that bit for you. I also think that if the listener is added weakly then the memory issue should be ok (I think I tested that but I can't swear to it now).

If you solve the issue of determining what needs to be unmapped, without ending up with multiple cross referencing dictionaries, I'll back-fit your solution into this one. Thanks for spotting.

creynders commented 11 years ago

I think I got it fixed, but I have no idea on how to test this, since it's completely encapsulated. If you got an idea, let me know!