[SR-10281] URLSession crash while handling basic authentication

[saiHemak]

Basically the crash in URLSession code is due to the invocation of TaskRegistry.remove twice with the same taskIdentifier from urlProtocolDidFinishLoading. While handling basic authentication, if the response code is 401 and authentication headers are missing then call should fail with error urlProtocol(_ protocol: URLProtocol, didFailWithError error: Error) which a is performing the removal of task`` from TaskRegistry. Hence, adding the return``` statement after the above call should resolve the issue.

[pushkarnk]

@swift-ci test

[pushkarnk]

@saiHemak Can you please change the commit message so that it describes the problem/solution from a URLSession standpoint?

[saiHemak]

@pushkarnk please review ..

[pushkarnk]

This probably fixes the issue reported here: https://twitter.com/mxcl/status/1111073495017029635

[pushkarnk]

LGTM

[pushkarnk]

@swift-ci please test and merge

[spevans]

@saiHemak @pushkarnk Should this be back ported to 5.0?

[ianpartridge]

Yes definitely - PR coming v. soon :)

[finestructure]

Should this be fixed in docker image swift:5.0.1-bionic?

I've got an API testing tool that specifically triggers 401s for testing purposes and between 5.0 and 5.0.1 I see some change in the output but I still seem to be getting the same underlying error:

5.0:

🎬  attack_create_contract started ...

Fatal error: Trying to remove task, but it's not in the registry.: file /home/buildnode/jenkins/workspace/oss-swift-5.0-package-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/URLSession/TaskRegistry.swift, line 76
❌  Error: The operation could not be completed (URLError(_nsError: The operation could not be completed))

5.0.1:

🎬  attack_create_contract started ...

❌  Error: The operation could not be completed (URLError(_nsError: The operation could not be completed))

This worked fine with swift:4.2.1.

[saiHemak]

Raised #2063 against 5.0 branch

[ianpartridge]

I guess this isn't the full fix then... @finestructure can you provide your testcase?

[finestructure]

I'll try to isolate a test case!

[finestructure]

I've created a sample repo here: https://github.com/finestructure/SR-10281 and logged the test results to a file test.log

Something odd seems to be going on (and I hope it's not me ;) ):

• macOS-5.0.1 and linux-4.2.1 always return a response
• linux-5.0.1 always comes with a nil response
• linux-5.0 is jittery but over a few reruns I see it mostly printing both none (i.e. nil response) and an actual response

I hope that helps!

[ianpartridge]

Awesome, thanks so much. @pushkarnk is out today but we'll take a look ASAP.

[pushkarnk]

@finestructure Thanks for the test case.

With swift-5.0-RELEASE the Fatal error is expected.

With swift-5.0.1-RELEASE I do not see the Error: The operation could not be completed message that you mention? May be the test is unable to reproduce the error in my environment. I ran the test more than 500 times, but don't see any failure. I also ran the test on Ubuntu 18.04 and 16.04 boxes, but no luck.

I see that the images swift:5.0 and swift:5.0.1 both use swift-5.0.1-RELEASE, can you please confirm the swift version which causes the Error: The operation could not be completed failure? Also, can you please confirm if you see the Fatal error with swift-5.0.1-RELEASE ?

[finestructure]

Sorry, I meant to add to this yesterday and completely forgot:

It appears that 5.0.1 fixes the fatal error that 5.0 introduced (line 50 in the test.log) but it doesn't seem to restore the 4.2.1 behaviour or match the 5.0/5.0.1 behaviour on macOS, which is to yield a response that is not nil.

In other words, I would have expected line 33 in that log not to be none but to show the response, like line 5ff.

Does that make sense?

[finestructure]

Regarding your other comment

I see that the images swift:5.0 and swift:5.0.1 both use swift-5.0.1-RELEASE

that doesn't match what I'm seeing:

 ~/P/G/SR-10281  ⎇ master  docker run --rm -it swift:5.0
root@d6cce205a5e4:/# swift --version
Swift version 5.0 (swift-5.0-RELEASE)
Target: x86_64-unknown-linux-gnu
root@d6cce205a5e4:/# exit
 ~/P/G/SR-10281  ⎇ master  docker run --rm -it swift:5.0.1
root@e60c1b0da0ae:/# swift --version
Swift version 5.0.1 (swift-5.0.1-RELEASE)
Target: x86_64-unknown-linux-gnu

I can confirm that I see the fatal error only with swift-5.0-RELEASE

[finestructure]

When you say "I also ran the test on Ubuntu 18.04 and 16.04 boxes" it seems like we're running the test differently. It sounds like you're not using docker but running swift run directly on Linux boxes?

I'm running the tests via the official swift docker images. If you've got docker installed, you can run all the tests and recreate my exact scenario if you run make test. It will run the macOS as well as the Linux tests with different versions.

[pushkarnk]

Hi @finestructure I did run with the official docker images using make test. I additionally ran the test on the Linux boxes.

[pushkarnk]

Sorry, I meant to add to this yesterday and completely forgot:

It appears that 5.0.1 fixes the fatal error that 5.0 introduced (line 50 in the test.log) but it doesn't seem to restore the 4.2.1 behaviour or match the 5.0/5.0.1 behaviour on macOS, which is to yield a response that is not nil.

In other words, I would have expected line 33 in that log not to be none but to show the response, like line 5ff.

Does that make sense?

Ah , ok. So this seems to be the problem. Thanks for clarifying!

[finestructure]

The version reports correctly for me:

test-linux-5.0
docker run --rm -it -v /Users/sas/Projects/GitHub/SR-10281:/host -w /host swift:5.0 swift --version && swift run -c release | tee -a test.log
Swift version 5.0 (swift-5.0-RELEASE)
Target: x86_64-unknown-linux-gnu
Launching ...

But after clearing my image I also get 5.0.1 now. Has the official version be retagged in docker hub?!

 ~/P/G/SR-10281  ⎇ master *~  docker rmi swift:5.0
Untagged: swift:5.0
Untagged: swift@sha256:ccaef3f936bd3cabd184a0caf7c2455eb861182b51e77970be4be72bea116a26
 ~/P/G/SR-10281  ⎇ master *~  make test-linux-5.0
rm -rf .build
echo "\ntest-linux-5.0" | tee -a test.log

test-linux-5.0
docker run --rm -it -v /Users/sas/Projects/GitHub/SR-10281:/host -w /host swift:5.0 swift --version && swift run -c release | tee -a test.log
Unable to find image 'swift:5.0' locally
5.0: Pulling from library/swift
Digest: sha256:dc9d8cb34457f9d10035caaefc465812ed387e8dfca506eb49bb5533fd137f0d
Status: Downloaded newer image for swift:5.0
Swift version 5.0.1 (swift-5.0.1-RELEASE)
Target: x86_64-unknown-linux-gnu
[2/2] Linking ./.build/x86_64-apple-macosx/release/SR-10281
Launching ...
Sending request ...

[ianpartridge]

5.0 is the Docker tag for the latest version of the Swift 5.0 release. So, if you did docker pull swift:5.0 when Swift 5.0 was released, you got Swift 5.0.0. If you do docker pull swift:5.0 now you get Swift 5.0.1. So that explains why you are seeing different versions.

[pushkarnk]

Ok, so the problem now is the inconsistency between versions 4.2.1 and 5.0.1, which is easily reproducible with your test case @finestructure . Would you mind filing a bug report on bugs.swift.org? Thanks.

[finestructure]

@ianpartridge I see, thanks! Since there is no 5.0.0 image I'd assumed that's the one that's frozen. Maybe there should be one?

[ianpartridge]

Yes probably we should have done a swift:5.0.0 tag when we released the 5.0 Docker image, so people can depend on that if they don't want 5.0.1, 5.0.2 etc. Unfortunately we didn't and that ship has sailed!

[finestructure]

Ok, I've filed it here: https://bugs.swift.org/browse/SR-10602