Leaked Channels

[Lankester]

My application joins and leaves different channels depending on where the user is in the application.

During some profiling tests I noticed some leaked objects, that appeared to be generated following calls to channel.leave(). Originally I assumed it was my code but in the end I managed to find the following in the memory debugger which seems to indicate an orphaned channel, several closure contexts, and cyclic references to PhxTimer and Push. Essentially the channel is never deallocated and however many channels the application joins they always persist in memory.

I have managed to solve the issue with the following changes Lankester@0c5c263 but despite the tests passing I am not sure it is the best way.

If it turns out to be a reasonable solution, I'd be happy to submit a PR.

[dsrees]

Hmmm looks like a reasonable solution, but I'm curious if there's a better way we can manage channel instances. I'll take a look into it. Thanks for reporting this!

[darrenclark]

Ran into this issue as well.

I wondering an approach like @Lankester's Lankester@0c5c263 , but using weak references would be a good option?

I think there is a potential for some subtle crashes with using unowned. For example, changing Push.channel to unowned could cause a crash along the lines of:

1. a Push object is created and stored on a Channel
2. the Push.send() function is called, starting the timeout timer here
3. The parent Channel gets deinit'd (maybe the socket is closed, etc..)
4. Reference to Push object is still strongly held by the timeoutTimer NSTimer here
5. When the timeoutTimer fires, it calls into trigger(_:payload:), which uses self.channel here, causing a crash (since it was deinit'd in step 3)

Thoughts on using weak instead? I can try to put together a PR for using weak

[dsrees]

I would be happy to review a PR if you could put one together @darrenclark. I was thinking along the same lines. I didn't think simply unowned would solve the problem.

[darrenclark]

Cool, i'll take a stab at it

[Lankester]

Sorry, I missed the timeoutTimer closure calling self in Push. weak should do it. Thank you @darrenclark.

[dsrees]

Not a problem @Lankester! We still appreciate you reporting the issue! I'll be sure to get a release out as soon as a fix is approved.