Race Condition in CustomHTTPProtocol between -stopLoading and URLSessionDataDelegate callbacks

Originator:heath.borders
Number:rdar://21484589 Date Originated:22-Jun-2015 10:35 AM
Status:Open Resolved:
Product:Sample Code Product Version:
Classification:Serious Bug Reproducible:Rarely
 
Summary:
The CustomHTTPProtocol example code contains a race condition between the CustomHTTPProtocol.clientThread and QNSURLSessionDemux.sessionDelegateQueue. The following sequence of method calls is possible:

1. URLSession:dataTask:didReceiveData: on QNSURLSessionDemux.sessionDelegateQueue
2. stopLoading on CustomHTTPProtocol.clientThread
3. URLSession:dataTask:didReceiveData: on CustomHTTPProtocol.clientThread

This causes an assertion failure because stopLoading nils CustomHTTPProtocol.task, and all of the URLSessionDataDelegate callback methods `assert(self.task == dataTask)`.

Steps to Reproduce:
I was able to reproduce this by loading a page in UIWebView while CustomHTTPProtocol was active. Unfortunately, the page is internal, and I can't expose it to you. However, if should be easy to see in code that the above race condition is at least theoretically possible.

Expected Results:
CustomHTTPProtocol should expect this race condition and handle it.

Actual Results:
CustomHTTPProtocol traps when `assert(self.task == dataTask)` fails.

Comments

This happens regularly during my testing

I recently moved to this new version to try and future proof my solution.

I immediately found issues along the lines described during testing. stopLoading is regularly called before delegate methods didReceiveResponse, didReceiverData and willCacheResponse.

As a work around, I have done the following:

1) In all affected delegate methods, do not assert if self.task is nil. 2) In didReceiveResponse, added the following code at the top to cancel when nil 

// Added to deal with race condition: If self.task is nil, assume cancelled
if (self.task == nil){
    completionHandler(NSURLSessionResponseCancel);
    return;
}

3) For now I have left the other delegate handler code alone, except to ignore a nil self.task, as only didReceiveResponse relies on self.task.

So far so good and it is working for me at the moment. An official solution would be good as this is a complex area but essential.

By Rory.McKinnel at Sept. 4, 2015, 12:26 p.m. (reply...)

Please note: Reports posted here will not necessarily be seen by Apple. All problems should be submitted at bugreport.apple.com before they are posted here. Please only post information for Radars that you have filed yourself, and please do not include Apple confidential information in your posts. Thank you!