Crash in URLSessionClient

[eelcokoelewijn]

I'm experiencing a crash(EXC_BAD_ACCESS) in the URLSessionClient when a pending GQL request gets deallocated. For example when a screen is loading data and dismissed before completing the request.

The crash is happening in sendRequest(_ request: URLRequest, rawTaskCompletionHandler: RawCompletion? = nil, completion: @escaping Completion) -> URLSessionTask, when performing self.rawCompletions[dataTask.taskIdentifier] = rawCompletion.

See also the attached crashlog.crash.zip.

In my case there are multiple GQL request performed while the screen gets deallocated. All pending requests are cancelled before the screen gets removed.

If I can provide more information or it needs more explanation, please let me know.

[designatednerd]

Looking at the stack, there's a bunch of Promise stuff going on before this gets enqueued, which indicates there's some kind of cache work going on before the call actually gets made.

Can you answer a few questions for me to help narrow down what might be going on:

• Are you setting up your own URLSessionClient or using the default one? Either subclassing or setting it up before setting up the HTTPNetworkTransport?
• Are you using the returnCacheDataElseFetch cache policy? This is the default one so if you're not doing anything to set it, this is what you'd be using. If not, which cache policy are you using?
• Are you using query watchers here or is this just individual requests?
• Is there any point at which the entire ApolloClient might be getting deallocated - for instance, are you using a separate Client instance per screen?

[sneakyness]

I'm getting this too!

Normal URLSessionClient. Default cache I think. Individual requests but a few of em. <Apollo.URLSessionClient: 0x6000037e9020> exists but that's as far as I managed to look, not doing anything wonky with client afaik? Project & Codegen are both v0.27

[designatednerd]

@sneakyness Also getting this when cancelling or are you seeing it at a different time?

[eelcokoelewijn]

@designatednerd, see answers below.

Are you setting up your own URLSessionClient or using the default one? Either subclassing or setting it up before setting up the HTTPNetworkTransport?

Using the default URLSessionClient, no sub-classing.

Are you using the returnCacheDataElseFetch cache policy? This is the default one so if you're not doing anything to set it, this is what you'd be using. If not, which cache policy are you using?

We are using the returnCacheDataAndFetch policy.

Are you using query watchers here or is this just individual requests?

Individual requests.

Is there any point at which the entire ApolloClient might be getting deallocated - for instance, are you using a separate Client instance per screen?

We use a singleton/ shared client for all the requests.

[pkluz]

seeing the same crash after updating. reverted back to previous version that was working for us (0.24.0).

The same client is used for all apollo requests and it is only re-allocated if the session configuration headers need to change (i.e. Bearer token has changed and needs to re-set.)

This requires a re-allocation of the Apollo client since the session configuration cannot easily be changed once passed.

[sneakyness]

@designatednerd Just normal requests, some of them nest fairly deep and we hit a few of those rapid fire up front, which is what seems to do it. Sometimes the promise chain is longer, other times it doesn't crash but the app is somehow up in the 100's for thread lifecycles (!)

I thought I might have been trying to be too concurrent, so I pulled all that out and it persisted. If I had to ship tomorrow, I'd follow pkluz and downgrade to 0.24.

Should I hoard crashlogs in the meantime?

[designatednerd]

Yes any crashlogs y'all can share would be a huge help.

I'll try to see if I can throw enough concurrent requests at it to make it crash locally.

[designatednerd]

Oh one more question: For those of you seeing this crash, are you seeing it on both simulator and device, or device only?

[pkluz]

tested on device only

[designatednerd]

OK as I was looking at the code it occurred to me that particularly if things are getting kicked off by Promises on random threads, I need to guard more defensively against concurrent access from multiple threads, which definitely is a recipe for EXC_BAD_ACCESS.

Before I go too far down other rabbit holes, can I ask the brave among you to see if this reasonably simple patch fixes the issue without totally tanking performance: fix/atomic-dicts?

[sneakyness]

I yeeted it, definitely seems better. Far less threads being burned. I can't get it to crash

🚀

[designatednerd]

OK - PR opened at #1184 - would appreciate any further feedback on this if anyone has it. If I don't hear anything else, I'll ship that as a patch release tomorrow.

[eelcokoelewijn]

For those of you seeing this crash, are you seeing it on both simulator and device, or device only?

Both, I can reproduce the crash on the device and simulator.

Before I go too far down other rabbit holes, can I ask the brave among you to see if this reasonably simple patch fixes the issue without totally tanking performance: fix/atomic-dicts?

The patch seems to fix the crash, I can't get the app to crash anymore.

[designatednerd]

Fix shipped with 0.27.1! Thank you all for your help in chasing this one down.