PSOperations icon indicating copy to clipboard operation
PSOperations copied to clipboard

Cancelling group operation

Open jcarroll-mediafly opened this issue 8 years ago • 1 comments

Was wondering what people's thoughts are on the GroupOperation cancellation. I'm in a situation where the queue (or GroupOperation directly) gets cancelled. The cancel method on GroupOperation will then cancel all the internal queue operations and finally call super.cancel().

The issue is that the queue delegate may (race condition) finish all the internal operations before super is called. Which means the operationQueue(operationQueue: OperationQueue, operationDidFinish operation: NSOperation, withErrors errors: [NSError]) could trigger (on a separate thread) finish(aggregatedErrors). Then all the observers / didFinishCalls / etc... will get called before the cancelled flag is set.

I'm thinking it only really affects GroupOperation more so than Operation given the way finishingOperation works; but in technically its possible to cancel any operation on a given thread and it finish on another a different on another leading to race conditions and mixed results when notified upon completion and investigating cancelled state.

My proposal:

Change

override public func cancel() {
        internalQueue.cancelAllOperations()
        internalQueue.suspended = false
        super.cancel()
    }

To

override public func cancel() {
        super.cancelWithErrors(aggregatedErrors)
        internalQueue.cancelAllOperations()
        internalQueue.suspended = false        
    }

That way when the internal queue finishes cancelling all the individual operations the overall GroupOperation will be properly marked cancelled

jcarroll-mediafly avatar Jul 20 '16 04:07 jcarroll-mediafly

Or add a lock around the original lines in cancel() and the finish(aggregatedErrors) in the queue callback.

jcarroll-mediafly avatar Jul 20 '16 04:07 jcarroll-mediafly