[swift-corelibs-dev] A URLSession crash and potential solutions

[Tony Parker]

Hi Pushkar,

> On Sep 13, 2016, at 12:20 PM, Pushkar N Kulkarni <pushkar.nk at in.ibm.com> wrote:
> 
> Hi Tony, Philippe, other onlookers, 
> 
> Long note. Kindly bear with me. 
> 
> I've created SR-2631 <https://bugs.swift.org/browse/SR-2631> to report a crash in a very basic usage of URLSession. I'm providing a general synopsis of the problem below and proposing two approaches to solve it.
> 
> The test case attached to the report creates a URLSession object locally inside a function run(), which also creates a dataTask with a completion handler, resumes the task and exits. On exiting run(), the URLSession object has no references pointing to it. It is cleaned up and we crash while trying to resume the task (the resume code and the subsequent callbacks are all asynchronous). 
> 
> This brings us to a basic flaw in the current NSURLSession implementation. Sessions and tasks are either created with delegates or callbacks, which are invoked asynchronously. Both delegates and callbacks need the URLSession objects they were registered with. Delegates need them until the last delegate method invocation returns. Completion handlers need them just until before they are invoked. The current implementation does not attempt to keep the URLSession object alive for the subsequent asynchronous code invocations. Hence the crash. 
> 
> Two questions arise when we think of a solution:
> 1. How can we keep the live URLSession objects alive?
> 2. What trigger can we use to clean them up?
> 
> I try to answer (2) first. The URLSession programming guide <https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/URLLoadingSystem/NSURLSessionConcepts/NSURLSessionConcepts.html> says:
> 
> "When you no longer need a session, invalidate it by calling either invalidateAndCancel (to cancel outstanding tasks) or finishTasksAndInvalidate(to allow outstanding tasks to finish before invalidating the object). *** The session object keeps a strong reference to the delegate until your app explicitly invalidates the session. If you do not invalidate the session, your app leaks memory.*** "
> 

Interesting, this is counter to the usual pattern of delegation but I can see why it’s required here. Usually we want our delegates to be zeroing weak references.

> The URLSession references also mention that "invalidateAndCancel" and "finishTasksAndInvalidate" break references to the callback and delegate objects and make the session object unusable. *** So, it is apparent that these methods are used to trigger a clean up of the session object ***
> 
> 
> Now coming to possible answers to (1). I can think of two ways of retaining the session object:
> 
> 1. We intentionally maintain a retain cycle between the session and task objects (URLSession <---> URLSessionTask)
> Currently the task object keeps an unowned reference to the session object. We could make it a strong reference.  And break it when invalidateAndCancel or finishTasksAndInvalidate are called. Alternatively, in the case of a completion handler, we could break the cycle just before invoking the handler. As long as we break the cycle eventually, I'm not sure if it could have side effects. 
> 

This is probably the most reasonable approach.

Thanks for looking into this!

- Tony

> 2. We maintain all live sessions in a static array or dictionary in the URLSession class. We remove them on session invalidation or callback invocation. Doing this bookkeeping and searching may involve a cost and scalability may be a problem here.
> 
> 
> Do you have any other approaches to consider? Can you please let me know your opinion on the ones detailed above ? 
> 
> Thank you! 
> 
> Pushkar N Kulkarni,
> IBM Runtimes
> 
> Simplicity is prerequisite for reliability - Edsger W. Dijkstra
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-corelibs-dev/attachments/20160913/ec4942c6/attachment.html>