Memory leak issues preventing destroying client

[SpencerCDixon]

Which version of the Ably SDK are you using?

E.g. 1.1.19

On which platform does the issue happen?

E.g. macOS 10.14

Are you using Carthage?

0.34.0

Are you using Cocoapods?

No

Which version of Xcode are you using?

11.3

What did you do?

Based on advice given in a previous ticket (#984) @tcard recommended completely destroying my ARTRealtime object and creating new ones when computers go to sleep and wake. I'm having trouble accomplishing this.

The latest release 1.1.19 introduced a new bug where after a computer being asleep for about 60-80 seconds they would all of sudden appear online in the presence set of computer B (even though computer A is completely asleep). EDIT: this was with a TTL of tokens set at 2 minutes.

I'm unable to get a simple RealTime client to create and destruct without setting any callbacks or channels without seeing memory leaks. Here is an example of code that can re-create the problem and fix it:

Broken Memory Leak

let options = ARTClientOptions()
        options.authCallback = { [unowned self] _, complete in
            self.apiService.authenticateAbly { [unowned self] (response, error) in
                if let response = response {
                    do {
                        let parsed = try ARTTokenRequest.fromJson(response.tokenRequest as ARTJsonCompatible)
                        complete(parsed, nil)
                        TKLog.debug("got ably token")
                    } catch let error as NSError {
                        TKLog.error("TokenRequest parsing error: \(error)")
                        complete(nil, error)
                    }
                } else {
                    TKLog.error("failed to get ably token")
                    complete(nil, error)
                }
            }
        }

Assuming I'm using that callback for the authCallback when I destroy the client on sleep and recreate when waking up I get this memory leak:

It appears that there may be issues with the scheduled block handler retaining things incorrectly.

If I update my auth callback to non-functioning code I can eliminate the issue:

No Memory Leak

options.authCallback = { [unowned self] _, complete in
            complete(nil, nil)
}

Or even parsing the TokenRequest and assigning it to var on self:

No Memory Leak

options.authCallback = { [unowned self] _, complete in
            self.apiService.authenticateAbly { [unowned self] (response, error) in
                self.parsed = try! ARTTokenRequest.fromJson(response.tokenRequest as ARTJsonCompatible)
            }
}

Passing your code a reference --> memory leak

options.authCallback = { [unowned self] _, complete in
            self.apiService.authenticateAbly { [unowned self] (response, error) in
                self.parsed = try! ARTTokenRequest.fromJson(response.tokenRequest as ARTJsonCompatible)
                complete(self.parsed!, nil)
            }
}

It's only when I pass the complete block handler a reference to the parsed ARTTokenRequest that things start failing.

It's possible I'm doing something very obviously wrong or dumb so please let me know if I am 😄 .

Are there tests of creating and destroying real-time clients without memory leaks??

Edit:

Just wanted to let you know I am calling close before setting client to nil to remove. Here are what the logs look like when sleeping:

2020-03-02 16:13:24.941288-0500 TupleDev[82819:1601341] DEBUG: R:0x600003d0c690 realtime is transitioning from 1 - Connecting to 2 - Connected
03/02/2020 16:13:30 [DEBUG] - willSleep OS notification	
03/02/2020 16:13:30 [DEBUG] - screensDidSleep OS notification	
2020-03-02 16:13:30.652684-0500 TupleDev[82819:1601001] INFO: Reachability: stopped listening for host realtime.ably.io
2020-03-02 16:13:30.653210-0500 TupleDev[82819:1601001] DEBUG: R:0x600003d0c690 realtime is transitioning from 2 - Connected to 5 - Closing
2020-03-02 16:13:30.653569-0500 TupleDev[82819:1601001] INFO: Reachability: stopped listening for host (null)
2020-03-02 16:13:30.653954-0500 TupleDev[82819:1601001] DEBUG: (ARTWebSocketTransport.m:109) R:0x600003d0c690 WS:0x60000296c380 websocket sending action 7 - Close
2020-03-02 16:13:30.654364-0500 TupleDev[82819:1601001] DEBUG: RS:0x600003031710 ARTJsonLikeEncoder<msgpack> encoding '{
    action = 7;
}'; got: <81a66163 74696f6e 07>
2020-03-02 16:13:30.685334-0500 TupleDev[82819:1601375] DEBUG: RS:0x600003031710 ARTJsonLikeEncoder<msgpack> decoding '<81a66163 74696f6e 08>'; got: {
    action = 8;
}
2020-03-02 16:13:30.686694-0500 TupleDev[82819:1601375] INFO: R:0x600003d0c690 Realtime closed
2020-03-02 16:13:30.687930-0500 TupleDev[82819:1601375] DEBUG: R:0x600003d0c690 realtime is transitioning from 5 - Closing to 6 - Closed
2020-03-02 16:13:30.688513-0500 TupleDev[82819:1601375] INFO: Reachability: stopped listening for host (null)
2020-03-02 16:13:30.689713-0500 TupleDev[82819:1601375] DEBUG: (ARTAuth.m:637) RS:0x600003031710 authorization cancelled with (null)
2020-03-02 16:13:30.691208-0500 TupleDev[82819:1601375] INFO: Reachability: stopped listening for host (null)
2020-03-02 16:13:30.691780-0500 TupleDev[82819:1601375] DEBUG: (ARTWebSocketTransport.m:281) R:0x0 WS:0x60000296c380 websocket did disconnect (code 1000) (null)

[QuintinWillison]

Hi @SpencerCDixon ... we're taking a look for you. Thanks for the detailed report.

[SpencerCDixon]

Awesome, let me know if you need me to create a separate reproducible app example or anything. I REALLY wanna get Ably testing in our prod environment asap.

Alternatively, if you have an example app of Ably client working with presence channels and computers going to sleep and waking up maybe I'm just using the client completely wrong?

[QuintinWillison]

Thanks, @SpencerCDixon. I've asked @tcard to take a look but he's a little bit stacked at the moment so may not have had time to look in depth. I'm able to check in again on Friday so will endeavour to have a look myself if Toni's not found time or made progress by then.

[tcard]

Thanks for the detailed report!

Based on advice given in a previous ticket (#984) @tcard recommended completely destroying my ARTRealtime object and creating new ones when computers go to sleep and wake.

Just to clarify, I meant that as a workaround while #984 was pending release. It should be completely OK to keep the same instance across sleeps.

About the memory leak issue:

I've found a reference cycle which is fixed at #999. While this should definitely be causing a leak on your app, I don't know how what you describe about authCallback would affect that.

#1000 fixes another possible situation, which your code may be triggering, but I don't think it's likely. For this to happen, you would have to hold to a reference to complete indefinitely.

Please test if branch 997-tentative-fix, which integrates both fixes, solves your problem. Otherwise, we'll have to look deeper into what's going on in your app.

The latest release 1.1.19 introduced a new bug where after a computer being asleep for about 60-80 seconds they would all of sudden appear online in the presence set of computer B (even though computer A is completely asleep). EDIT: this was with a TTL of tokens set at 2 minutes.

That's weird, but this seems to be an unrelated issue, isn't it? We'll look into it too.

[SpencerCDixon]

The latest release 1.1.19 introduced a new bug where after a computer being asleep for about 60-80 seconds they would all of sudden appear online in the presence set of computer B (even though computer A is completely asleep). EDIT: this was with a TTL of tokens set at 2 minutes.

I'm starting to think this was due to this setting:

When I turn wake from sleep off it no longer happens. It's not ideal as the presence will reconcile eventually but I think it will be okay for now. Can let ya know once we have this deployed in the field if it becomes an issue.

Just to clarify, I meant that as a workaround while #984 was pending release. It should be completely OK to keep the same instance across sleeps.

I apologize for misunderstanding you! I am now no longer tearing down the entire RealTime object.

I've found a reference cycle which is fixed at #999. While this should definitely be causing a leak on your app, I don't know how what you describe about authCallback would affect that.

I'm going to push forward without deallocating these objects but good work on finding some leak fixes!

[QuintinWillison]

Now that #984 is closed and both #999 and #1000 have been merged, I think this issue can also be closed.