Operation Went Isfinished=Yes Without Being Started by the Queue It Is In

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

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 }
}
}

NSOperation subclass isCancelled isFinished isConcurrent

If you override ready, you must also override cancel. What happens in the abstract class is that when cancel is called it sets the operation as ready so that the queue may call start, the start method checks for the canceled flag, then aborts the operation and sets isFinished=YES. Then the operation queue dealloc's the operation. You can't have one without the other.

OperationQueue with custom `maxConcurrentOperationCount` does not pick up / execute all operations in the queue, after finishing first operation

The issue is that the methods are not KVC/KVO compliant. As the Operation documentation says:

The NSOperation class is key-value coding (KVC) and key-value observing (KVO) compliant for several of its properties.

If you provide custom implementations for any of the preceding properties, your implementations must maintain KVC and KVO compliance.

Constraints on the degree of concurrency (e.g., both maxConcurrentOperationCount and addDependency(_:)) rely upon KVO to know when the prior operation is complete. If you fail to perform the required KVO notifications, the queue will not know when subsequent operations may proceed.

See the latter part of Trying to Understand Asynchronous Operation Subclass for an example implementation.


FWIW, here is an asynchronous operation implementation:

public class AsynchronousOperation: Operation {

@Atomic @objc private dynamic var state: OperationState = .ready

// MARK: - Various `Operation` properties

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

// KVO for dependent properties

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

return super.keyPathsForValuesAffectingValue(forKey: key)
}

// Start

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 !isFinished { state = .finished }
}
}

private extension AsynchronousOperation {
/// State for this operation.

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

With the following:

@propertyWrapper
public class Atomic<T> {
private var _wrappedValue: T
private var lock = NSLock()

public var wrappedValue: T {
get { lock.synchronized { _wrappedValue } }
set { lock.synchronized { _wrappedValue = newValue } }
}

public init(wrappedValue: T) {
_wrappedValue = wrappedValue
}
}

extension NSLocking {
func synchronized<T>(block: () throws -> T) rethrows -> T {
lock()
defer { unlock() }
return try block()
}
}

With the above, I abstract the asynchronous Operation code into something I can subclass and inherit the asynchronous behaviors. E.g., here is an operation that performs the same asyncAfter as your example (but with some extra OSLog signposts so I can visually see the operations in Instruments):

import os.log

private let log = OSLog(subsystem: "Op", category: .pointsOfInterest)

class MyOperation: AsynchronousOperation {
var value: Int

init(value: Int) {
self.value = value
super.init()
}

override func main() {
let id = OSSignpostID(log: log)
os_signpost(.begin, log: log, name: "Operation", signpostID: id, "%d", value)

DispatchQueue.main.asyncAfter(deadline: .now() + 1) { [self] in
finish()
os_signpost(.end, log: log, name: "Operation", signpostID: id, "%d", value)
}
}
}

Then ...

let queue = OperationQueue()
queue.maxConcurrentOperationCount = 1

for i in 0..<5 {
queue.addOperation(MyOperation(value: i))
}

... yields a timeline of the operations like so:

Sample Image

Trying to Understand Asynchronous Operation Subclass

You said:

  1. What is the purpose of the stateQueue property? I see it being used by get and set of the state computed property, but I can't find any documentation that explains the sync:flags:execute and sync:execute methods that they use.

This code "synchronizes" access to a property to make it thread safe. Regarding why you need to do that, see the Operation documentation, which advises:

Multicore Considerations

... When you subclass NSOperation, you must make sure that any overridden methods remain safe to call from multiple threads. If you implement custom methods in your subclass, such as custom data accessors, you must also make sure those methods are thread-safe. Thus, access to any data variables in the operation must be synchronized to prevent potential data corruption. For more information about synchronization, see Threading Programming Guide.

Regarding the exact use of this concurrent queue for synchronization, this is known as the "reader-writer" pattern. This basic concept of reader-writer pattern is that reads can happen concurrent with respect to each other (hence sync, with no barrier), but writes must never be performed concurrently with respect to any other access of that property (hence async with barrier).

For example, you might implement a reader-writer for thread-safety on an array like so:

class ThreadSafeArray<T> {
private var values: [T]
private let queue = DispatchQueue(label: "...", attributes: .concurrent)

init(_ values: [T]) {
self.values = values
}

func reader<U>(block: () throws -> U) rethrows -> U {
return try queue.sync {
try block()
}
}

func writer(block: @escaping (inout [T]) -> Void) {
queue.async(flags: .barrier) {
block(&self.values)
}
}

// e.g. you might use `reader` and `writer` like the following:

subscript(_ index: Int) -> T {
get { reader { values[index] } }
set { writer { $0[index] = newValue } }
}

func append(_ value: T) {
writer { $0.append(value) }
}

func remove(at index: Int) {
writer { $0.remove(at: index)}
}
}

Obviously, the use of reader-writer in this Operation subclass is even simpler, but the above illustrates the pattern.

You also asked:


  1. What is the purpose of the three class methods in the NSObject section that return ["state"]? I don't see them being used anywhere. I found, in NSObject, class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String>, but that doesn't seem to help me understand why these methods are declared.

These are just methods that ensure that changes to the state property trigger KVO notifications for properties isReady, isExecuting and isFinished. The KVO notifications of these three keys is critical for the correct functioning of asynchronous operations. Anyway, this syntax is outlined in the Key-Value Observing Programming Guide: Registering Dependent Keys.

The keyPathsForValuesAffectingValue method you found is related. You can either register dependent keys using that method, or have the individual methods as shown in your original code snippet.


BTW, here is a revised version of the AsynchronousOperation class you provided, namely:

  1. You must not call super.start(). As the start documentation says (emphasis added):

    If you are implementing a concurrent operation, you must override this method and use it to initiate your operation. Your custom implementation must not call super at any time.

  2. Add @objc required in Swift 4.

  3. Renamed execute to use main, which is the convention for Operation subclasses.

  4. It is inappropriate to declare isReady as a final property. Any subclass should have the right to further refine its isReady logic (though we admittedly rarely do so).

  5. Use #keyPath to make code a little more safe/robust.

  6. You don't need to do manual KVO notifications when using dynamic property. The manual calling of willChangeValue and didChangeValue is not needed in this example.

  7. Change finish so that it only moves to .finished state if not already finished.

Thus:

public 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 _state: OperationState = .ready

/// The state of the operation

@objc private dynamic var state: OperationState {
get { return stateQueue.sync { _state } }
set { stateQueue.async(flags: .barrier) { self._state = 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 }

// 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)
}

// Start

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 !isFinished { state = .finished }
}
}

object of NSOperation doesn't removed from NSOperationQueue after executing

I have done the exact same thing, albiet in Swift.

There are a couple of aspects that I have implemented differently and are listed below:

  • I have noticed that we do not have to override cancel() method in the
    Asynchronous operation. The default behavior of NSOperation's cancel
    method is to set the self.cancelled boolean to true.
  • self.executing should be set to true only inside the overriden start() method of operation, but not in the init ( Not sure if this wil cause any issues). Also before setting the self.executing = true in the start() method, I ensure that the boolean self.cancelled is false. If self.cancelled = true, we should set self.finished = true.
  • Also , I have created a "didSet" property observer for the isExecuting and isFinished properties where I call the willChangeValueForKey and didChangeValueForKey. I am not 100% sure how to replicate the didSet behavior in Obj C. ( This says to overrride setter)

Please refer to the Ray Wenderlich video tutorials about "Concurrency" where Sam DAvies explains the creation of NSoperation subclass for Asynchronous operation. Please note, that it is only for subscribers and it is explained in Swift. I believe if you fix points 1 and 2, you should see your issues being fixed.

Exactly when NSOperation is removed from NSOperationQueue on cancelling request?

First, an operation is removed from the Queue only when its isFinished property becomes true.

Secondly, if you dealloc B, then the Queue will be dealloced and the operations as well. But you should make sure in your code that you are not referencing these operation objects or the Queue at a later point of time.



Related Topics



Leave a reply



Submit