Signalling notify only after all sync requests have completed

Question

Getting a notification when all async requests have completed.

I call registerTrack multiple times in the loop below. I want to only trigger the syncGroup.notify when all requests have been completed. From looking at this code, its going to notify after each successful completion of registerTrack. How do I make it signal notify only after all registerTrack operations have been completed?

My code is as follows:

    var fail = false
    let syncGroup = DispatchGroup() //used so we can determine if all the register requests have finished

    for track in unsyncedTracks {
        syncGroup.enter()

        if fail {
            syncGroup.leave()
            break //might save an aync request from getting executed on failure
        }

        registerTrack(track: track, withCompletion: { (success: Bool) -> () in

            if success {
                self.debug.log(tag: "RecordViewController", content: "Registered track: \(track.name)")
                syncGroup.leave()
            }
            else {
                fail = true
                syncGroup.leave()
            }
        })
    }

    //all requests complete
    syncGroup.notify(queue: .main) {
        if fail {
            complete(false)
        }
        else {
            self.debug.log(tag: "RecordViewController", content: "Finished registering all unsynced tracks")
            complete(true)
        }
    }

EDIT:

Based on the recommendation to use GCD concurrent queues, I changed my code to:

    var failed = false
    //work item to loop through and call registerTrack on each track
    let registerTracksWorkItem = DispatchWorkItem {
        for track in unsyncedTracks {
            if failed { //exit early on failure, potentially save a network request
                break
            }

            self.registerTrack(track: track, withCompletion: { (success: Bool) -> () in

                if success {
                    self.debug.log(tag: "RecordViewController", content: "Registered track: \(track.name)")
                }
                else {
                    failed = true
                }
            })
        }
    }

    //handler for when all registerTrack calls are complete
    registerTracksWorkItem.notify(queue: DispatchQueue.main) {
        if failed {
            self.debug.log(tag: "RecordViewController", content: "At least one registerTrack call failed")
            complete(false)
        }
        else {
            self.debug.log(tag: "RecordViewController", content: "Finished registering all unsynced tracks")
            complete(true)
        }
    }

    //execute the work item
    let queue = DispatchQueue.global()
    queue.async(execute: registerTracksWorkItem)

This code does not wait for registerTrack to finish, rather it executes them all and calls the notify event. :(

EDIT 3:

So this works. It uses a counter to check to see if its at the final unsyncedTracks object.

    var fail = false
    let syncGroup = DispatchGroup() //used so we can determine if all the register requests have finished

    //used to make sure notify is only triggered when all registerTracks are complete
    let unsyncedTracksCount = unsyncedTracks.count
    var completeCounter = 0

    syncGroup.enter()
    for track in unsyncedTracks {

        registerTrack(track: track, withCompletion: { (success: Bool) -> () in

            if success {
                self.debug.log(tag: "RecordViewController", content: "Registered track: \(track.name)")
            }
            else {
                fail = true
            }

            completeCounter = completeCounter + 1
            if completeCounter == unsyncedTracksCount {
                syncGroup.leave()
            }
        })
    }

    //all requests complete
    syncGroup.notify(queue: .main) {
        if fail {
            complete(false)
        }
        else {
            self.debug.log(tag: "RecordViewController", content: "Finished registering all unsynced tracks")
            complete(true)
        }
    }

Can I do this (it also appears to work)? I added the if fail, leave() if.

    var fail = false
    let syncGroup = DispatchGroup() //used so we can determine if all the register requests have finished

    //used to make sure notify is only triggered when all registerTracks are complete
    let unsyncedTracksCount = unsyncedTracks.count
    var completeCounter = 0

    syncGroup.enter()
    for track in unsyncedTracks {
        if fail { //might save an aync request from getting executed on failure
            syncGroup.leave()
            break
        }

        registerTrack(track: track, withCompletion: { (success: Bool) -> () in

            if success {
                self.debug.log(tag: "RecordViewController", content: "Registered track: \(track.name)")
            }
            else {
                fail = true
            }

            completeCounter = completeCounter + 1
            if completeCounter == unsyncedTracksCount {
                syncGroup.leave()
            }
        })
    }

    //all requests complete
    syncGroup.notify(queue: .main) {
        if fail {
            complete(false)
        }
        else {
            self.debug.log(tag: "RecordViewController", content: "Finished registering all unsynced tracks")
            complete(true)
        }
    }

Putting multiple asynchonrous calls into a loop isn't good design. GCD's concurrent queues may be a good fit for your problem. That way you can run all the registerTracks tasks and know when they are all finished. https://developer.apple.com/library/content/documentation/General/Conceptual/ConcurrencyProgrammingGuide/OperationQueues/OperationQueues.html — Gruntcakes, Nov 17 '16 at 21:46
I Added a link above to the Apple documentation, theres code snippets in there. — Gruntcakes, Nov 17 '16 at 21:48
@Essenceofchicken The samples are all in C++. Is there a Swift version somewhere, specifically similar to what I'm trying to do? — toast, Nov 17 '16 at 21:57
https://www.raywenderlich.com/79149/grand-central-dispatch-tutorial-swift-part-1 — Gruntcakes, Nov 17 '16 at 22:00
@Essenceofchicken Do I wrap my `for track in unsyncedTracks` loop into a `async` closure? If I understand this right, it'll execute the closure asynchronously, which then executes all the `registerTrack` calls asynchoronously as well? If so, then how do I do the `notify` bit? I'm a bit confused on how to layout the code in my example — toast, Nov 17 '16 at 22:08
@Essenceofchicken See my updated post. I think I've implemented your suggestion, but its not waiting until completion now. — toast, Nov 17 '16 at 22:57
@toast, first of all, are you sure your `registerTrack()` function called the completion handler just once? Check it with logging or debugger. Second, in your first sample of code you call `break` between `.enter()` and `.leave()`, it's a straight road to more issues, you shouldn't break you loop between `.enter()` and `.leave()` without proper managing of the states of queue. — dive, Nov 17 '16 at 23:01
@dive #1 yes, `registerTrack` only calls the completion handler once, with a boolean value. #2 I updated the break, so now it does a `leave()` before the break. — toast, Nov 17 '16 at 23:11

score 0 · Answer 1 · answered Nov 18 '16 at 00:02

0

So, the problem is related to your fail variable and for loop structure. You assign fail as false before the loop execution. When you code enters the loop and register .enter() then it checks for if fail condition and call .leave() if fail is true. Basically, when one of the call to registerTrack() sets the fail to true your code is in undetermined state, because registerTrack() is async and you don't know which of .enter() calls set the flag to true. At the moment when .enter() count is equal to .leave() count (two fails in a row for example), CGD decides that your queue is finished and called .notify() "prematurely".

I suggest you to remove the flag fail (the code will work as expected, but will execute registerTrack() for all unsyncedTracks) and rethink your solution and logic. For example, DispatchWorkItem.cancel() can be a solution for your problem.

answered Nov 18 '16 at 00:02

dive

2,130
1
17
26

I've updated my answer, and the code is working as expected now. The reason I had that `if fail` there was to exit prematurely, in the event of a failure, so it saves the phone from making another network call. While `syncGroup` may become unsycned (undertermined), does it matter, as its acting as a cancel all? Thanks. – toast Nov 18 '16 at 00:33
Theoretically, it's possible to cancel the dispatched queue with `DispatchWorkItem.cancel()` or with `dispatch_cancel` handler, but you will be forced to handle the results and `dispatch_cancel` just cancel future execution of the block, but already dispatched blocks will be executed anyway. I recommend you to rewrite your code using `OperationQueue` ([Documentation](https://developer.apple.com/reference/foundation/operationqueue)), because it does exactly what you want and can be cancelled easily with `cancelAllOperations()`. – dive Nov 18 '16 at 22:35
In the past, I have tried using NSOperations, but it doesn't work too well when you have a bunch of async requests. For example, say you make one a dependency on another, if it contains an async request, it will continuing executing, not waiting for the response. To be honest, I like the idea of these queues in swift, but none are really helpful to me. I would love to see an example of a few async requests together, one not being called until the other is complete. I wonder if I'm just implementing them poorly. – toast Nov 19 '16 at 13:39
See this question where I attempted to use NSOperations for a similar problem. I ended up ditching the NSOperation stuff because it just didn't seem to suit this type of application. http://stackoverflow.com/questions/39899105/waiting-for-request-to-finish-before-executing-the-next-operation – toast Nov 19 '16 at 13:43

Signalling notify only after all sync requests have completed

1 Answers1