Is it safe to initialize closure directly in an init?

Using Swift
1

What's the proper way to provide a default closure that gets set on a stored property? I'm investigating a crash which I think is related to a default closure value initialized in an init. I generally provide a default value on the parameter itself. I saw the default closure created within the init which doesn't feel right to me.

Example of the crash report.

MyModule
partial apply for closure #1 in DownloadOperation.getData ...
NSGenericException: Task created in a session that has been invalidated

0 CoreFoundation  __exceptionPreprocess +220
2 CFNetwork       CFURLRequestSetRequestPriority + 10688
3 Embrace         swizzledDataTaskWithRequest + 492
4 MyModule        DownloadOperation.swift line 22
                  partial apply for closure #1 in DownloadOperation.executeRequest(_:) + 11

Here's the gist of what the class structure looks like:

01  class DownloadOperation: Foundation.Operation {
02      typealias ResultType = Result<Data, Error>
03      typealias CompletionType = (ResultType) -> ()
04
05      let request: URLRequest
06      var onCompletion: CompletionType
07
08      init(request: URLRequest) {
09          ...
10          self.request = request
11          self.onCompletion = {_ in}
12          super.init()
13      }
14
15      override func execute() {
16          ...
17          executeRequest(self.request)
18      }
19
20      private func executeRequest(_ request: URLRequest) {
21          self.workQueue.async {
22              let task = self.session.dataTask(with: self.request)
23              task.resume()
24          } 
25      }
26  }
27
28  // gets used like so
29  let op = DownloadOperation()
30  op.onCompletion = { (result) in 
31     ...
32  }

I think it makes more sense to pass in an escaping closure to the init and provide a default if needed. The reason why I've narrowed it down to this is line 22 and 11 mentioned in the crash.

// make the onCompletion property private and a let.
private let onCompletion: CompletionType

init(request: URLRequest, completion: @escaping CompletionType = {_ in}) {
    ...
    self.onCompletion = completion
}

// then of course update the references
let op = DownloadOperation { (result) in
    ...
}
2

You've elided a key part here: the lifetime of your URLSession. Given the initial exception it seems that your URLSession has been invalidated by the time you call dataTask. So where's self.session?

(As an aside, you really don't want a URLSession per request. Instead, you'd pass a common session into your encapsulation.)

Also, subclassing Operation is generally a bad idea. Nowadays there are much better ways to encapsulate URLSession work. If you are going to subclass it you need to know about sync vs. async operations, which are a whole thing. And there are a lot of Operation bugs you may need to be aware of, like various KVO issues (whether those are still relevant I don't know).

2 Likes
3

The session is a lazy property.

private lazy var session: URLSession = { [unowned self] in
    let config = URLSessionConfiguration.default
    return URLSession(configuration: config, delegate: self, delegateQueue: nil)
}()
4

Yes, I do agree that there are better ways to encapsulate URLSession work. I'll take a look at the lifetime of the session. Thanks.

5

It's also no clear how you're using the operation. But ultimately what I think happened is that you're created a synchronous Operation, started async work in it, and so the Operation has completed and deinitialized before the task is created, leading to the exception you see. So moving your URLSession outside the Operation would fix that part but you'd still be stuck trying to create an async Operation subclass if you wanted it to work properly.