-
Notifications
You must be signed in to change notification settings - Fork 361
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
Add cancelAllOperations to PINOperationQueue #122
Add cancelAllOperations to PINOperationQueue #122
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
- (void)cancelAllOperations | ||
{ | ||
[self lock]; | ||
for (PINOperation *operation in [_referenceToOperations objectEnumerator]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another advantage of the _referenceToOperations weakTable :) We wouldn't have been able to enumerate over _queuedOperations without making a copy. Do you know if we need a copy here? Is it safe to enumerate over it even as objects are potentially being nil'd out by locked_cancelOperation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it's not safe to modify a mutable collection while enumerating through it via an NSEnumerator. So we probably have to make a copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the right call to me.
@@ -128,18 +128,33 @@ - (void)dealloc | |||
return reference; | |||
} | |||
|
|||
- (void)cancelAllOperations | |||
{ | |||
[self lock]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you indent the code within the lock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@garrettmoon Addressed comments |
Add support to cancel all operations. Furthermore created a new method for cancelling an operation that the lock needs to be hold. This used by
cancelOperation
as well ascancelAllOperations
now.