Remove manual task clearing in URLSessionClient invalidate

[philfi]

This resolves assertion crashes that are still occurring as described in this issue #1376 (comment) despite the fix introduced in #1383.
In theory since URLSession.shared is not being used, there shouldn't be a need for any manual task clearing. In my testing calling invalidateAndCancel prior to cleaning up tasks seems to consistently do the trick.

[apollo-cla]

@philfi: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[designatednerd]

The problem is that while we're not using urlSession.shared directly, we're still passing in the shared config, which means in most cases I believe that generally winds up having the same problem as actually using the shared session.

That said, if you want to break it out so that the manual cancellation only happens when the shared session config is the config, that could probably work.

[philfi]

ah had missed that the URLSessionConfiguration.default was being used as a default argument, thanks for pointing that out.

I'll do some testing with that case to get familiar but my hope is this logic should cover both that and the case of an injected config, which pretty consistently triggers this assertion failure for me at least. Will also take a look into the test failures on macOS, had them passing locally running 10.15.4.

[designatednerd]

I wouldn't worry too much about the test failures, the tests that are failing are flaky as hell

[designatednerd]

@philfi Have you had a chance to try to update this or do you mind if I take it over?

[philfi]

@designatednerd apologies I have not, feel free to take it over!

[designatednerd]

Ooh! OK so working on this when I set it up to do the extra work if session.configuration == URLSessionConfiguration.default was true, the work got done. BUT when I set it up for session == URLSession.shared, it was never hit.

The docs for invalidateAndCancel say:

Calling this method on the session returned by the sharedSession method has no effect.

So I think your approach is actually the correct one, and we don't need to bother with manual task cancellation. Poking CI, will merge if it passes.

[philfi]

Thanks @designatednerd!

[RolandasRazma]

Just FIY still happening in 0.36.0 (iOS12 simulator)