Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancelling group operation #69

Open
jcarroll-mediafly opened this issue Jul 20, 2016 · 1 comment
Open

Cancelling group operation #69

jcarroll-mediafly opened this issue Jul 20, 2016 · 1 comment

Comments

@jcarroll-mediafly
Copy link

jcarroll-mediafly commented Jul 20, 2016

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant