Swift closure async order of execution

Question

In my model have function to fetch data which expects completion handler as parameter:

func fetchMostRecent(completion: (sortedSections: [TableItem]) -> ()) {

        self.addressBook.loadContacts({
            (contacts: [APContact]?, error: NSError?) in
            // 1
            if let unwrappedContacts = contacts {
                for contact in unwrappedContacts {

                    // handle constacts
                    ...                        
                    self.mostRecent.append(...)
                }
            }
            // 2
            completion(sortedSections: self.mostRecent)
        })
}

It's calling another function which does asynchronous loading of contacts, to which I'm forwarding my completion

The call of fetchMostRecent with completion looks like this:

model.fetchMostRecent({(sortedSections: [TableItem]) in
    dispatch_async(dispatch_get_main_queue()) {
        // update some UI
        self.state = State.Loaded(sortedSections)
        self.tableView.reloadData()
    }
})

This sometimes it works, but very often the order of execution is not the way as I would expect. Problem is, that sometimes completion() under // 2 is executed before scope of if under // 1 was finished.

Why is that? How can I ensure that execution of // 2 is started after // 1?

Answer 1

A couple of observations:

1. It will always execute what's at 1 before 2. The only way you'd get the behavior you describe is if you're doing something else inside that for loop that is, itself, asynchronous. And if that were the case, you'd use a dispatch group to solve that (or refactor the code to handle the asynchronous pattern). But without seeing what's in that for loop, it's hard to comment further. The code in the question, alone, should not manifest the problem you describe. It's got to be something else.

2. Unrelated, you should note that it's a little dangerous to be updating model objects inside your asynchronously executing for loop (assuming it is running on a background thread). It's much safer to update a local variable, and then pass that back via the completion handler, and let the caller take care of dispatching both the model update and the UI updates to the main queue.

3. In comments, you mention that in the for loop you're doing something asynchronous, and something that must be completed before the completionHandler is called. So you'd use a dispatch group to do ensure this happens only after all the asynchronous tasks are done.

4. Note, since you're doing something asynchronous inside the for loop, not only do you need to use a dispatch group to trigger the completion of these asynchronous tasks, but you probably also need to create your own synchronization queue (you shouldn't be mutating an array from multiple threads). So, you might create a queue for this.

Pulling this all together, you end up with something like:

func fetchMostRecent(completionHandler: ([TableItem]?) -> ()) {
    addressBook.loadContacts { contacts, error in
        var sections = [TableItem]()
        let group = dispatch_group_create()
        let syncQueue = dispatch_queue_create("com.domain.app.sections", nil)

        if let unwrappedContacts = contacts {
            for contact in unwrappedContacts {
                dispatch_group_enter(group)
                self.someAsynchronousMethod {
                    // handle contacts
                    dispatch_async(syncQueue) {
                        let something = ...
                        sections.append(something)
                        dispatch_group_leave(group)
                    }
                }
            }
            dispatch_group_notify(group, dispatch_get_main_queue()) {
                self.mostRecent = sections
                completionHandler(sections)
            }
        } else {
            completionHandler(nil)
        }
    }
}

And

model.fetchMostRecent { sortedSections in
    guard let sortedSections = sortedSections else {
        // handle failure however appropriate for your app
        return
    }

    // update some UI
    self.state = State.Loaded(sortedSections)
    self.tableView.reloadData()
}

Or, in Swift 3:

func fetchMostRecent(completionHandler: @escaping ([TableItem]?) -> ()) {
    addressBook.loadContacts { contacts, error in
        var sections = [TableItem]()
        let group = DispatchGroup()
        let syncQueue = DispatchQueue(label: "com.domain.app.sections")

        if let unwrappedContacts = contacts {
            for contact in unwrappedContacts {
                group.enter()
                self.someAsynchronousMethod {
                    // handle contacts
                    syncQueue.async {
                        let something = ...
                        sections.append(something)
                        group.leave()
                    }
                }
            }
            group.notify(queue: .main) {
                self.mostRecent = sections
                completionHandler(sections)
            }
        } else {
            completionHandler(nil)
        }
    }
}