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:
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 theDispatchWorkItem
it won't finish the operation, I therefore need to make sure thatcancel
will explicitly finish the operation itself.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 canceldataTask
(which will call its completion handler even if you cancel the request) and call thesuper
implementation.The third scenario is where you have some operation that is periodically checking
isCancelled
state. In that case, you don't have to implementcancel
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 updateisCancelled
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:
Trying to Understand Asynchronous Operation Subclass
You said:
- What is the purpose of the
stateQueue
property? I see it being used by get and set of thestate
computed property, but I can't find any documentation that explains thesync:flags:execute
andsync: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:
- 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, inNSObject
,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:
You must not call
super.start()
. As thestart
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.Add
@objc
required in Swift 4.Renamed
execute
to usemain
, which is the convention forOperation
subclasses.It is inappropriate to declare
isReady
as afinal
property. Any subclass should have the right to further refine itsisReady
logic (though we admittedly rarely do so).Use
#keyPath
to make code a little more safe/robust.You don't need to do manual KVO notifications when using
dynamic
property. The manual calling ofwillChangeValue
anddidChangeValue
is not needed in this example.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
How to Change Pin Color in Mapkit Under the Same Annotationview (Swift3)
How to Reuse Struct on Swift? or Is There Any Other Way
Nsurlsession/Nsurlconnection Http Load Failed (Kcfstreamerrordomainssl, -9802) on a Subdomain
How to Get Camera Calibration Data on iOS? Aka Avcameracalibrationdata
Array Element Checking in Swift
Swiftui Present View Modally via Tabview
Swift Conform to Protocol Subclass
Get "No Keychain Available" Error When Try to Access Keychain from App Extension
Alamofire Multipart Upload Post Error in Swift
How to Make Async/Await in Swift
Uitextview - Adjust Size Based on the Content in Swiftui
Nsclassfromstring Returning Nil for Nested Class
Turn Swift Object into a JSON String
Showing Hidden View Really Slow
How Should Secure Transport Tls Be Used with Bsd Sockets in Swift