Track and cancel RequestChain when RequestChainNetworkTransport is released

[RolandasRazma]

Possible workaround for #1473 - I'm personally not that happy with it

[designatednerd]

I don't think this should be necessary - setting up multiple network transports that need to be repeatedly invalidated should be a rare thing outside of tests.

This also doesn't do anything to remove the chains from the array once they've completed, so every single chain you add is going to be put in here with no way to take it back out.

[RolandasRazma]

I don't think this should be necessary - setting up multiple network transports that need to be repeatedly invalidated

Thats not true. In our setup we don't have "API" singleton, we have "Cache" singleton and setup whole stack for every screen and share only cache.

This also doesn't do anything to remove the chains from the array

That's not true. Its weak reffed array and all Chains are removed as soon as they released.

[TizianoCoroneo]

Thats not true. In our setup we don't have "API" singleton, we have "Cache" singleton and setup whole stack for every screen and share only cache.

Why? That seems like a lot of overhead

[designatednerd]

That's not true. Its weak reffed array and all Chains are removed as soon as they released.

Ah I see, it's dropping down to NSHashTable and I wasn't familiar with the operator you're using.

I also have the same question as @TizianoCoroneo - what's the reasoning behind using different clients (with different network stacks) across different parts of your app?

[RolandasRazma]

there is probably no good reason anymore. I think reasons were "singletons are evil" :) and in the beginning there was no way of updating headers without recreating client. We could move to singleton "API" as we using singleton "cache" any way. On the other hand look how many bugs I reported with such set-up :P

In any case, you should be able to create and tear down whole stack without crashing whole app

[TizianoCoroneo]

I think reasons were "singletons are evil" :)

I 70% agree: only global access to singletons "is evil", because you cannot swap the object for testing purposes as easily. But having a single instance of the ApolloClient around, and properly injecting it in DI doesn't incur in the same issues.

An app I worked recently had a similar setup, where every screen was creating its own client from scratch, and when I got rid of that in favor of a single instance the result was a noticeable speedup in the whole app. We didn't measure it in Instruments unfortunately.

[designatednerd]

In any case, you should be able to create and tear down whole stack without crashing whole app

In principle I obviously agree with this, but you also don't want unexpected behavior from something that's been invalidated. I'll think this through.

[designatednerd]

i'm going to close this in favor of #1489, which returns an error rather than crashing on the URLSessionClient being invalidated.