Great work!

[malhal]

But could I suggest a small improvement: in the finish method, inside the perform block, if it called a new method finishOnDispatchQueue to set the state instead of setting it directly, then that method could be overridden by subclasses and allow them to call their own custom completion block and then calling super to change the state and end the operation. This way they can guarantee their completion block is called on the dispatch queue using your nifty code.

[malhal]

Oh and finish should probably take an NSError param, e.g. finishWithError and also pass it to finishOnDispatchQueueWithError I mentioned. This way the custom completion blocks can also supply an error param.

[dmcrodrigues]

Thanks @malhal ✨

I analysed your suggestion about creating a new method finishOnDispatchQueue but I'm not seeing a good motive to do that kind of exposure.

This way they can guarantee their completion block is called on the dispatch queue using your nifty code.

Why do you think is important guarantee the invocation of completion block on the dispatch queue used internally? I'm asking that because the only purpose of that queue is change the internal state of the operation in a safely way.

Another question is if we really need that explicit completion block or if we can rely on the completionBlock from NSOperation's which is always executed after the operation has completed even when it was cancelled.

Let me know your thoughts about this, I'm not against the change but currently I'm not seeing a strong reason to do that change.

[malhal]

Hi thanks for taking the time to think about this! Very nice of you.

I began writing an example to demonstrate the problem and I couldn't produce a concrete issue so I thought it would be better just to answer the questions, in reverse order if you don't mind.

NSOperation's completionBlock and the next NSOperation in the queue run simultaneously. This means if using dependent operations it isn't possible to initialise a property on the next operation on the completion of the first, because it has already started. So we can't use that. Anyway we would want our own block so we can pass the result and error.

Given this, we then have to resort to our completion block running on our own background thread we created in the async task. From there it's more tricky to explain why I think it would be better for a custom completion block to run on the dispatch queue instead of on that but I have some guesses. Maybe it would ensure safe manipulation of the operations running on the queue. Since say for example you get an error so in your completion block you make make the queue cancel all operations and if an operation overrides cancel, it is safer to be on the dispatch queue then. Also any calls to operation properties like isFinished result in a dispatch sync to the dispatch queue which seem unnecessary given the operation is about to finish. Maybe also to ensure the operation finishing is the next thing after the completion block ends without a delay to sync with the dispatch queue. Lastly I've been reverse engineering the CloudKit iOS API and they do it this way, then again they call the main from their dispatch queue so maybe that's their reason!

@interface TestOperation : CKQueryOperation
@end

@implementation TestOperation
-(void)main{ // 12
    dispatch_queue_t d = dispatch_get_current_queue();   
/*
<OS_dispatch_queue: com.apple.cloudkit.operation.callback[0x7fe5c1d0f290] = { xrefcnt = 0x2, refcnt = 0x2, suspend_cnt = 0x0, locked = 1, target = com.apple.root.default-qos.overcommit[0x10852b300], width = 0x0, running = 0x0, barrier = 1 }>
*/

    [super main];
}
@end

Methods of interest on CKOperation:

    NSObject<OS_dispatch_queue> *_callbackQueue;

- (void)_finishOnCallbackQueueWithError:(id)arg1;
- (void)finishWithError:(id)arg1;
- (void)_finishInternalOnCallbackQueueWithError:(id)arg1;
- (void)_handleCompletionCallback:(id)arg1;
- (void)_handleProgressCallback:(id)arg1;

So that's my thoughts sorry if I was vague on the last answer and I understand if you don't see a strong reason in it.