I'm creating an NSOperation that executes a closure with a delay. The operations are added into a queue, every time before a new operation is added I cancel all existing ones in the queue:
let myOp = SomeOperation { [weak self] in /* do something */ }
queue.cancelAllOperations()
queue.addOperation(myOp)
final class SomeOperation: Operation {
private let closure: () -> Void
init(closure: @escaping () -> Void) {
self.closure = closure
}
override func main() {
if isCancelled {
return
}
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3, execute: doSomething)
}
private func doSomething() {
guard isCancelled == false else {
return
}
closure()
}
}
While the above code works, the code below doesn't. In the DispatchQueue closure, self is nil:
final class SomeOperation: Operation {
private let closure: () -> Void
init(closure: @escaping () -> Void) {
self.closure = closure
}
override func main() {
if isCancelled {
return
}
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in
guard let self = self else { return }
guard isCancelled == false else { return }
self.closure()
}
}
}
So I'm trying to learn a bit deeper:
self is nil because as soon as DispatchQueue.main.asyncAfter… is called, the main method finishes and the operation is thus released.execute: doSomething implicitly captures/retains a self, so even after asyncAfter, self is still there.So my questions are:
start, asynchronous, executing, finished, etc. In my case I just need to have a delay, not actually doing anything asynchronous, should I do it in main only or should I do it as an async operation by implementing those methods Apple suggested?self retained implicitly, which doesn't sound correct and can create retain cycle?Thanks!
You asked:
- In Apple's doc it says for concurrent operations I should rather be using
start,asynchronous,executing,finished, etc. In my case I just need to have a delay, not actually doing anything asynchronous, should I do it in main only or should I do it as an async operation by implementing those methods Apple suggested?
First, you are doing something asynchronous. I.e., asyncAfter is asynchronous.
Second, the motivating reason behind Apple’s concurrent operation discussion is that the operation should not finish until the asynchronous task it launched also finishes. You talk about canceling the operation, but that only makes sense if the operation is still running by the time you go to cancel it. This feature, the wrapping the asynchronous task in an object while never blocking a thread, is one of the key reasons we use operations rather than just GCD. It opens the door for all sorts of elegant dependencies between asynchronous tasks (dependencies, cancelation, etc.).
- Is my thinking correct that in Code 1 there's a self retained implicitly, which doesn't sound correct and can create retain cycle?
Regarding the strong reference cycle issues, let’s look at your first example. While it is prudent for the creator of the operation to use [weak self] capture list, it should not be required. Good design of the operation (or anything using asynchronously called closures) is to have it release the closure when it is no longer needed:
class SomeOperation2: Operation {
private var closure: (() -> Void)?
init(closure: @escaping () -> Void) {
self.closure = closure
}
override func main() {
if isCancelled {
return
}
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3, execute: doSomething)
}
override func cancel() {
closure = nil
super.cancel()
}
private func doSomething() {
guard !isCancelled else {
return
}
closure?()
closure = nil
}
}
It doesn’t mean that the caller shouldn’t use [weak self] capture list, only that the operation no longer requires it, and will resolve any strong reference cycles when it is done with the closure.
[Note, in the above, I omitted synchronization of the variable, to keep it simple. But you need to synchronize your access to it to ensure thread-safe design.]
But this design begs the question as to why would you would want to keep the asyncAfter scheduled, still firing even after you canceled the operation. It would be better to cancel it, by wrapping the closure in a DispatchWorkItem, which can be canceled, e.g.:
class SomeOperation: Operation {
private var item: DispatchWorkItem!
init(closure: @escaping () -> Void) {
super.init()
item = DispatchWorkItem { [weak self] in
closure()
self?.item = nil
}
}
override func main() {
if isCancelled { return }
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3, execute: item)
}
override func cancel() {
item?.cancel()
item = nil
super.cancel()
}
}
Having outlined the memory issues, we should note that this is all probably moot, as you probably should just make this a concurrent operation (with all that custom KVO) as you identified in the documentation. Besides, all this care we’ve put into cancelation logic only applies if the operation is alive until the asynchronous process finishes. So, we will make a concurrent operation. E.g.,
class SomeOperation: AsynchronousOperation {
private var item: DispatchWorkItem!
init(closure: @escaping () -> Void) {
super.init()
item = DispatchWorkItem { [weak self] in
closure()
self?.item = nil
self?.complete()
}
}
override func main() {
if isCancelled { return }
synchronized {
DispatchQueue.main.asyncAfter(deadline: .now() + 3, execute: item)
}
}
override func cancel() {
super.cancel()
synchronized {
item?.cancel()
item = nil
}
}
}
The above uses an asynchronous operation base class that (a) performs the necessary KVO notifications; and (b) is thread-safe. Here is one random example of how that could be implemented:
/// Asynchronous Operation base class
///
/// This class performs all of the necessary KVN of `isFinished` and
/// `isExecuting` for a concurrent `NSOperation` subclass. So, to developer
/// a concurrent NSOperation subclass, you instead subclass this class which:
///
/// - must override `main()` with the tasks that initiate the asynchronous task;
///
/// - must call `complete()` function when the asynchronous task is done;
///
/// - optionally, periodically check `self.cancelled` status, performing any clean-up
/// necessary and then ensuring that `complete()` is called; or
/// override `cancel` method, calling `super.cancel()` and then cleaning-up
/// and ensuring `complete()` is called.
public class AsynchronousOperation: Operation {
private let lock = NSLock()
private var _executing: Bool = false
override private(set) public var isExecuting: Bool {
get {
synchronized { _executing }
}
set {
willChangeValue(for: \.isExecuting)
synchronized { _executing = newValue }
didChangeValue(for: \.isExecuting)
}
}
private var _finished: Bool = false
override private(set) public var isFinished: Bool {
get {
synchronized { _finished }
}
set {
willChangeValue(for: \.isFinished)
synchronized { _finished = newValue }
didChangeValue(for: \.isFinished)
}
}
override public var isAsynchronous: Bool { return true }
/// Complete the operation
///
/// This will result in the appropriate KVN of `isFinished` and `isExecuting`
public func complete() {
if isExecuting {
isExecuting = false
isFinished = true
}
}
public override func cancel() {
super.cancel()
complete()
}
override public func start() {
if isCancelled {
isFinished = true
return
}
isExecuting = true
main()
}
override public func main() {
fatalError("subclasses must override `main`")
}
public func synchronized<T>(block: () throws -> T) rethrows -> T {
try lock.withLock { try block() }
}
}