SwiftPhoenixClient icon indicating copy to clipboard operation
SwiftPhoenixClient copied to clipboard

Minor memory leak in Pushes that have not timed out/cancelled timeout when Channel closes

Open jatindervrify opened this issue 5 years ago • 1 comments

When performing a Push and then closing a channel, the timeout work item never releases the Push due to the strong retaining of self on Line 240 of Push.swift

/// Setup and start the Timeout timer.
let workItem = DispatchWorkItem { in
    self.trigger("timeout", payload: [:])
}

changing this closure to use [weak self] resolves the issue

/// Setup and start the Timeout timer.
let workItem = DispatchWorkItem { [weak self] in
    self?.trigger("timeout", payload: [:])
}

Screenshots:

image

image

Happy to investigate further if I'm in the wrong here. Cheers

jatindervrify avatar Jun 20 '19 18:06 jatindervrify

Finally getting around to looking into this. Turns out if you add [weak self] to the callback, then the callback is deallocated regardless unless you hold on to a reference of the Push when sending it, meaning a legit timeout won't be handled successfully.

channel
    .push("event", payload: ["foo": "bar"])
    .receive("timeout", callback: { (_) in
        timeoutCallsCount += 1
    })
                
fakeClock.tick(channel.timeout / 2)
expect(timeoutCallsCount).to(equal(0))
                
fakeClock.tick(channel.timeout)
expect(timeoutCallsCount).to(equal(1)) // <- this will be 0 if [weak self] is added

unless you hold reference to the Push

let push = channel
    .push("event", payload: ["foo": "bar"])
    .receive("timeout", callback: { (_) in
        timeoutCallsCount += 1
    })

I'll have to keep looking into this

dsrees avatar Jul 18 '19 00:07 dsrees