NSOperationQueue Is Finished Before All Operations Are Completed

Question

I have x number different task that need to run. Each task is responsible for going out to the internet, downloading some data, process that data, then saving that data.

They all run at the same time on a background thread. Though I would be fine with them running one after the other.

When all 4 tasks are complete I would like to be notified that they have finished in some fashion.

However, NSOperationQueue is firing off the completed queue before any of the other actual operations have started.

My Log with the current setup:

fetchMyData: completionOperation YES

FILE: MySet1 CREATED

FILE: MySet2 CREATED

FILE: MySet3 CREATED

FILE: MySet4 CREATED

Is there a better way to chain the operations together and fully waiting on all of them to complete before firing the final operation?

Here is what I am doing:

- (void) timeToUpdate {
    [self fetchMyData:^(BOOL done) {
        NSLog(@"fetchMyData: completionOperation : %@", done ? @"YES":@"NO");
    }];


- (void) fetchMyData: (void(^)(BOOL done)) completion {
    
    NSOperationQueue *queue = [[NSOperationQueue alloc] init];
    
    NSOperation *completionOperation = [NSBlockOperation blockOperationWithBlock:^{
        NSLog(@"fetchMyData: completionOperation");
        completion(true);
    }];
    
    NSOperation *operation;
    

    operation = [NSBlockOperation blockOperationWithBlock:^{
        [self downloadDataSet:@"MySet1"];
    }];
    
    [completionOperation addDependency:operation];
    [queue addOperation:operation];
    

    operation = [NSBlockOperation blockOperationWithBlock:^{
        [self downloadDataSet:@"MySet2"];
    }];
    
    [completionOperation addDependency:operation];
    [queue addOperation:operation];
    

    operation = [NSBlockOperation blockOperationWithBlock:^{
        [self downloadDataSet:@"MySet3"];
    }];
    
    [completionOperation addDependency:operation];
    [queue addOperation:operation];
    

    operation = [NSBlockOperation blockOperationWithBlock:^{
        [self downloadDataSet:@"MySet4"];
    }];
    
    [completionOperation addDependency:operation];
    [queue addOperation:operation];
    
    [queue addOperation:completionOperation];
}

//The below is just abstract sudo code that works and does its job of downloading, processing and saving the data.

- (void) downloadDataSet:(SomeSet) {
    
    [NSURLSessionUploadTask  uploadTaskWithRequest:task 
                                    fromData: Someset 
                                   completionHandler: {
                    process(set);
    }]
}

- process:(data) {
    doSomethignWithData(data)
//then
    write(data)

}
- write(data) {
    //save to file system, sql, coredata whatever
    
    if(success){
        NSLog(@"FILE: data CREATED");
    }
}

Answer 1

The problem is that your operations are calling uploadTaskWithRequest:fromData: completionHandler:, an asynchronous operation which returns before its job has been completed. What you should do is to look into using an asynchronous NSOperation; basically you override the isAsynchronous method to return YES, and then you set the operation's finished property when uploadTaskWithRequest:fromData: completionHandler:'s completion handler is called. This way, any operations that depend on your download operations will wait until they are actually complete before firing.

Here's a handy set of async NSOperation subclasses that I use; it's Swift, not Objective-C, but it illustrates the concept and should provide a decent pseudocode for you to build your own implementation.

open class AsyncOperation: Operation {
    override open var isAsynchronous: Bool { return true }
    override open var isExecuting: Bool { return self.state == .started }
    override open var isFinished: Bool { return self.state == .done }

    private enum State {
        case initial
        case started
        case done
    }

    private var state: State = .initial {
        willSet {
            // due to a legacy issue, these have to be strings. Don't make them key paths.
            self.willChangeValue(forKey: "isExecuting")
            self.willChangeValue(forKey: "isFinished")
        }
        didSet {
            self.didChangeValue(forKey: "isFinished")
            self.didChangeValue(forKey: "isExecuting")
        }
    }

    public init(name: String? = nil) {
        super.init()

        if #available(macOS 10.10, *) {
            self.name = name
        }
    }

    final override public func start() {
        self.state = .started

        self.main {
            if case .done = self.state {
                fatalError("AsyncOperation completion block called twice")
            }

            self.state = .done
        }
    }

    final override public func main() {}

    open func main(completionHandler: @escaping () -> ()) {
        fatalError("Subclass must override main(completionHandler:)")
    }
}

open class AsyncBlockOperation: AsyncOperation {
    private let closure: (_ completionHandler: @escaping () -> ()) -> ()

    public init(name: String? = nil, closure: @escaping (_ completionHandler: @escaping () -> ()) -> ()) {
        self.closure = closure
        super.init(name: name)
    }

    override open func main(completionHandler: @escaping () -> ()) {
        self.closure(completionHandler)
    }
}

Alternatively, you could eschew the use of NSOperation and use a dispatch_group_t to get similar behavior; call dispatch_group_enter() initially, call dispatch_group_leave() in the download task's completion handler, and use dispatch_notify() to run your completion block.