Fix nsdata fails to access contents of url #1499
Conversation
|
@swift-ci please test |
| @@ -207,7 +207,7 @@ class _HTTPServer { | |||
| deadlineTime = .now() | |||
| } | |||
|
|
|||
| DispatchQueue.main.asyncAfter(deadline: deadlineTime) { | |||
| DispatchQueue.global().asyncAfter(deadline: deadlineTime) { | |||
There was a problem hiding this comment.
Can we split up the data fix itself from the change to the URLSession test code?
There was a problem hiding this comment.
No, we can't. Without this change, deadlock will occur in testing this PR. However If you can merge split PRs in order, I should be able to do that.
Would you like to split this?
There was a problem hiding this comment.
I think it should be merged as one PR as the HTTPServer.swift change is required for the test case to work.
There was a problem hiding this comment.
I don't think there is anything specific to HTTPServer about the change to NSData. How is it using NSData that triggers this bug?
There was a problem hiding this comment.
If the test case that was added (test_contentsOfURL) is calling NSData(contentsOf: url) and that is running on the main thread then won't the HTTP server have to run on another thread to service the request, or am I mis-understanding how DispatchQueue works? NSData doesn't trigger this bug, its just needed for the unit test.
There was a problem hiding this comment.
This is a code that reproduces this bug.
import Foundation
if let data = NSData(contentsOf: URL(string: "https://swift.org")!) {
print(data.length) // 0
}EDIT
As you says, changes to the HTTP server are necessary only for testing.
I add some explanations just in case we need it.
First, NSData(contentsOf:) uses URLSession.
Then, NSData(contentsOf:) uses NSCondition wait().
https://github.com/apple/swift-corelibs-foundation/blob/2ba2086405c864e0c8bdb223209cf9a87e74a1d2/Foundation/NSData.swift#L217
While the main thread is blocking with NSCondition wait(), The following test HTTP server code enqueued into the main thread queue is not executed.
Finally, Since there is no response from test HTTP server, deadlock will occur because the following code to resolve NSData's wait is not executed.
So, we need to change the HTTP server for testing.
There was a problem hiding this comment.
Ok, thanks for the additional info.
|
It makes sense to me that in the case the same thread is responding to the HTTP request initiated by NSData as the one that is blocked executing NSData(contentsOf: URL), then the test code should move its serving to a different thread. |
|
@swift-ci please test and merge |
NSData (contentsOf :)was always initialized with length 0.Also, Testing
NSData (contentsOf :)on a test HTTP server(_HTTPServer) will cause deadlock, so I changed it to work on sub thread.