Operation went isFinished=YES without being started by the queue it is in
Asked Answered
O

1

5

Overview

  • There is an asynchronous operation subclass
  • Added this operation to the queue.
  • I cancelled this operation before it starts.

Runtime Error / Warning:

SomeOperation went isFinished=YES without being started by the queue it is in

Question:

  1. Is this something that can be ignored or it is something serious ?
  2. How to resolve this ?
  3. Is the workaround / solution provided at the end valid ?

Code:

public class SomeOperation : AsyncOperation {

    //MARK: Start

    public override func start() {

        isExecuting = true

        guard !isCancelled else {
            markAsCompleted() //isExecuting = false, isFinished = true
            return
        }

        doSomethingAsynchronously { [weak self] in

            self?.markAsCompleted() //isExecuting = false, isFinished = true
        }
    }

    //MARK: Cancel

    public override func cancel() {

        super.cancel()
        markAsCompleted() //isExecuting = false, isFinished = true
    }
}

Adding to Queue and cancelling:

//someOperation is a property in a class
if let someOperation = someOperation {
    queue.addOperation(someOperation)
}

//Based on some condition cancelling it
someOperation?.cancel()

Is this valid a solution ?

public override func cancel() {

    isExecuting = true //Just in case the operation was cancelled before starting

    super.cancel()
    markAsCompleted()
}

Note:

  • markAsCompleted sets isExecuting = false and isFinished = true
  • isExecuting, isFinished are properties which are synchronised KVO
Overbuild answered 7/1, 2018 at 14:3 Comment(3)
Please show how you add the operation to the queue and how you cancel it.Ward
Updated answer with a new section called Adding to Queue and cancellingOverbuild
When you add an operation to a queue, it isn't started immediately. It is queued.. You need to check in your cancel operation if isCancelled or isFinished.. Then in your start operation, check the same..Cobbett
S
18

The key problem is that your markAsCompleted is triggering isFinished when the operation is not isExecuting. I'd suggest you just fix that markAsCompleted to only do this if isExecuting is true. This reduces the burden on subclasses doing any complicated state tests to figure out whether they need to transition to isFinished or not.

That having been said, I see three basic patterns when writing cancelable asynchronous operations:

  1. If I'm dealing with some pattern where the canceling of the task will prevent it from transitioning executing operations to a isFinished state.

    In that case, I must have cancel implementation manually finish the executing operations. For example:

    class FiveSecondOperation: AsynchronousOperation {
        var block: DispatchWorkItem?
    
        override func main() {
            block = DispatchWorkItem { [weak self] in
                self?.finish()
                self?.block = nil
            }
    
            DispatchQueue.main.asyncAfter(deadline: .now() + 5, execute: block!)
        }
    
        override func cancel() {
            super.cancel()
    
            if isExecuting {
                block?.cancel()
                finish()
            }
        }
    }
    

    Focusing on the cancel implementation, because if I cancel the DispatchWorkItem it won't finish the operation, I therefore need to make sure that cancel will explicitly finish the operation itself.

  2. Sometimes, when you cancel some asynchronous task, it will call its completion handler automatically for you, in which case cancel doesn't need to do anything other than cancel the that task and call super. For example:

    class GetOperation: AsynchronousOperation {
        var url: URL
        weak var task: URLSessionTask?
    
        init(url: URL) {
            self.url = url
            super.init()
        }
    
        override func main() {
            let task = URLSession.shared.dataTask(with: url) { data, _, error in
                defer { self.finish() }  // make sure to finish the operation
    
                // process `data` & `error` here
            }
            task.resume()
            self.task = task
        }
    
        override func cancel() {
            super.cancel()
            task?.cancel()
        }
    }
    

    Again, focusing on cancel, in this case we don't touch the "finished" state, but just cancel dataTask (which will call its completion handler even if you cancel the request) and call the super implementation.

  3. The third scenario is where you have some operation that is periodically checking isCancelled state. In that case, you don't have to implement cancel at all, as the default behavior is sufficient. For example:

    class DisplayLinkOperation: AsynchronousOperation {
        private weak var displayLink: CADisplayLink?
        private var startTime: CFTimeInterval!
        private let duration: CFTimeInterval = 2
    
        override func main() {
            startTime = CACurrentMediaTime()
            let displayLink = CADisplayLink(target: self, selector: #selector(handleDisplayLink(_:)))
            displayLink.add(to: .main, forMode: .commonModes)
            self.displayLink = displayLink
        }
    
        @objc func handleDisplayLink(_ displayLink: CADisplayLink) {
            let percentComplete = (CACurrentMediaTime() - startTime) / duration
    
            if percentComplete >= 1.0 || isCancelled {
                displayLink.invalidate()
                finish()
            }
    
            // now do some UI update based upon `elapsed`
        }
    }
    

    In this case, where I've wrapped a display link in an operation so I can manage dependencies and/or encapsulate the display link in a convenient object, I don't have to implement cancel at all, because the default implementation will update isCancelled for me, and I can just check for that.

Those are three basic cancel patterns I generally see. That having been said, updating markAsCompleted to only trigger isFinished if isExecuting is a good safety check to make sure you can never get the problem you described.


By the way, the AsynchronousOperation that I used for the above examples is as follows, adapted from Trying to Understand Asynchronous Operation Subclass. BTW, what you called markAsCompleted is called finish, and it sounds like you're triggering the isFinished and isExecuting KVO via a different mechanism, but the idea is basically the same. Just check the current state before you trigger isFinished KVO:

open class AsynchronousOperation: Operation {

    /// State for this operation.

    @objc private enum OperationState: Int {
        case ready
        case executing
        case finished
    }

    /// Concurrent queue for synchronizing access to `state`.

    private let stateQueue = DispatchQueue(label: Bundle.main.bundleIdentifier! + ".rw.state", attributes: .concurrent)

    /// Private backing stored property for `state`.

    private var rawState: OperationState = .ready

    /// The state of the operation

    @objc private dynamic var state: OperationState {
        get { return stateQueue.sync { rawState } }
        set { stateQueue.sync(flags: .barrier) { rawState = newValue } }
    }

    // MARK: - Various `Operation` properties

    open         override var isReady:        Bool { return state == .ready && super.isReady }
    public final override var isExecuting:    Bool { return state == .executing }
    public final override var isFinished:     Bool { return state == .finished }
    public final override var isAsynchronous: Bool { return true }

    // MARK: - KVN for dependent properties

    open override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
        if ["isReady", "isFinished", "isExecuting"].contains(key) {
            return [#keyPath(state)]
        }

        return super.keyPathsForValuesAffectingValue(forKey: key)
    }

    // MARK: - Foundation.Operation

    public final override func start() {
        if isCancelled {
            state = .finished
            return
        }

        state = .executing

        main()
    }

    /// Subclasses must implement this to perform their work and they must not call `super`. The default implementation of this function throws an exception.

    open override func main() {
        fatalError("Subclasses must implement `main`.")
    }

    /// Call this function to finish an operation that is currently executing

    public final func finish() {
        if isExecuting { state = .finished }
    }
}
Scoggins answered 7/1, 2018 at 19:0 Comment(13)
Awesome explanation, thanks a ton !!! I was under the impression that you needed to set isFinished = true for the dependencies to trigger. Now from your explanation I learn that only executing operations need to be set to isFininished = true. Just calling the super.cancel is sufficient for operations that haven't started. I will override markAsCompleted to check for isExecutingOverbuild
I also noticed that when an operation (in queue) that has hasn't started is cancelled, it starts. I suppose the check in start to check if isCancelled catches it and handles it. So for my example (like in your case 3) there is no need to override cancel. As you suggested would need to fix markAsCompleted to add the check for isExecuting.Overbuild
Just curious in your AsynchronousOperation.start would it be better to set state = executing before checking is isCancelled so that when operation is cancelled (that hasn’t started) and operation invokes start and executing is set before it is set to finished ?Overbuild
@Overbuild - If you go back to the original Concurrency Programming Guide, in their start implementation, they check if it was canceled. If canceled, they trigger the isFinished to false KVN and return immediately. They only do the isExecuting to true KVN if the operation wasn't canceled.Scoggins
@Overbuild - "I also noticed that when an operation (in queue) that hasn't been started is canceled, it starts." ... That's not strictly true. For example, if I add two operations to serial queue and then queue.cancelAllOperations() when half way through the first operation, I'll see the second op start and then immediately cancel. But that isn't always the case. For example, in the same scenario, if I just cancel the second op, then I'll see it cancel without ever starting. It just depends upon how you cancel the operations.Scoggins
Thanks a lot Rob, it has quite a few intricate details I didn't notice, think I need to test moreOverbuild
The operations completionBlock is not called if the OP was cancelled.Ward
Those examples of using cancel are great! Do you think it would be better to call super.cancel() after cancelling your own work (i.e.call it at the end of the function)? It's my understanding that super will call start() to immediately flush the operation from the queue.Sogdiana
@Scoggins In your AsynchronousOperation subclass, if the operation is cancelled before it starts, then it is later started, won't it fail to reach the .finished state? Because in start() you check if it is cancelled, and if so, call finish() and return. But in finish() you only move to the .finished state if the current state was .executing, which seemingly leaves no way to reach .finished. I'd assume this was just a sample-code oversight, but you've put such an emphasis on this finish() behavior (in this answer and the related one), that I feel I must have missed something?Ore
Hi @Scoggins in this if isCancelled { finish() ; return } why there is need of calling finish() as it wont do anything because it is checking for state isExecuting and here it won't be true. correct me if there is any scenario where this could be useful.Regulate
@Ore - You are correct. It should just set state to .finished in start. I’ve corrected that above.Scoggins
@Scoggins I have been using AsynOperation class for download some data from url which is asynchronous. I have read in the doc that asynchronous task is ignored if added to a queue, I am having hard time understanding this, Please help. developer.apple.com/documentation/foundation/operation . "When you add an operation to an operation queue, the queue ignores the value of the isAsynchronous property and always calls the start() method from a separate thread. Therefore, if you always run operations by adding them to an operation queue, there is no reason to make them asynchronous."Regulate
Apple has really made a mess of this. In short, they’re right, that they don’t care about isAsynchronous in conjunction w queues. At the same time, isAsynchronous says: “The value of this property is true for operations that run asynchronously with respect to the current thread or false for operations that run synchronously on the current thread.” So, while ignored when using operation queues, but I think it’s valuable for property to reflect the operation’s actual underlying behavior. Plus, it will also work if you ever use it in situation where you start it manually yourself.Scoggins

© 2022 - 2025 — McMap. All rights reserved.