18

I want to run a for loop in swift in order, DispatchGroup will fire them together, so I want to use DispatchQueue and DispatchSemaphore to achieve my goal. I failed to make my program work, how can I force them to wait and run one by one?

let dispatchGroup = DispatchGroup()
let dispatchQueue = DispatchQueue(label: "taskQueue")
let dispatchSemaphore = DispatchSemaphore(value: 1)

for c in self.categories {

    dispatchSemaphore.wait()

    dispatchQueue.async(group: dispatchGroup) {

        if let id = c.categoryId {

            dispatchGroup.enter()

            self.downloadProductsByCategory(categoryId: id) { success, data in

                if success, let products = data {

                    self.products.append(products)
                }

                dispatchSemaphore.signal()
                dispatchGroup.leave()
            }
        }
    }
}

dispatchGroup.notify(queue: dispatchQueue) {

    self.refreshOrderTable { _ in

        self.productCollectionView.reloadData()

        NVActivityIndicatorPresenter.sharedInstance.stopAnimating()
    }
}

Thanks to Palle, here is my final code:

let dispatchGroup = DispatchGroup()
let dispatchQueue = DispatchQueue(label: "taskQueue")
let dispatchSemaphore = DispatchSemaphore(value: 0)

dispatchQueue.async {

    for c in self.categories {

        if let id = c.categoryId {

            dispatchGroup.enter()

            self.downloadProductsByCategory(categoryId: id) { success, data in

                if success, let products = data {

                    self.products.append(products)
                }

                dispatchSemaphore.signal()
                dispatchGroup.leave()
            }

            dispatchSemaphore.wait()
        }
    }
}

dispatchGroup.notify(queue: dispatchQueue) {

    DispatchQueue.main.async {

        self.refreshOrderTable { _ in

            self.productCollectionView.reloadData()
        }
    }
}
Timeless
  • 7,338
  • 9
  • 60
  • 94
  • 2
    Unrelated but don't make the `weak self` - `strong self` dance. It's nonsensical because GCD does **not** cause retain cycles. – vadian Oct 21 '17 at 13:07
  • If you want to execute in order, why use concurrency anyway? Just create a loop in a background queue and execute your `downloadProductsByCategory` function directly in the loop. – Palle Oct 21 '17 at 13:15
  • @Palle `downloadProductsByCategory` itself is async method using alamofire – Timeless Oct 21 '17 at 13:16
  • Then just put `dispatchSemaphore.wait()` after the call to `downloadProductsByCategory` and `dispatchSemaphore.signal()` in the completion handler. – Palle Oct 21 '17 at 13:17
  • @Palle for some reason, when `dispatchSemaphore.wait()` called for the *second* time, the whole program will be blocked there forever. and `downloadProductsByCategory` never receive a callback – Timeless Oct 21 '17 at 13:24
  • You have to create your semaphore with an initial value of 0, otherwise there will always be two concurrent download tasks. Is your code the same as in my answer? – Palle Oct 21 '17 at 13:26
  • Why is there a `semaphore.wait()` call in the first line of the for loop? It should not be there. If `categoryId` is nil, this will cause more `wait` calls than `signal` calls and you will get a deadlock. The `wait` call has to be **after** the call to `downloadProductsByCategory`. – Palle Oct 21 '17 at 13:30
  • There is no need for `DispatchGroup` when executing tasks synchronously.. https://pastebin.com/6MgAaJSq – Brandon Dec 29 '17 at 01:13
  • @Brandon could you please answer the question and provide your solution? Thanks. – Timeless Oct 13 '18 at 00:54
  • @Timeless https://pastebin.com/4Tt90NJx – Brandon Oct 13 '18 at 16:12
  • If you want asynchronous but still ordered then: https://pastebin.com/rqrA5Fxm – Brandon Oct 13 '18 at 16:24
  • Why are you using a dispatch group and a semaphore at the same time? It makes absolutely no sense. – Peter Schorn Nov 07 '20 at 18:49

1 Answers1

19

You can put the whole loop in a block instead of putting just the download function in a block:

dispatchQueue.async {
    for c in self.categories {
        if let id = c.categoryId {
            self.downloadProductsByCategory(categoryId: id) { success, data in
                if success, let products = data {
                    self.products.append(products)
                }

                dispatchSemaphore.signal()
            }
            dispatchSemaphore.wait()
        }
    }
}

You can simplify your code by using compactMap to unwrap your product ids:

for id in self.categories.compactMap({$0.categoryId}) { ... }
Palle
  • 11,511
  • 2
  • 40
  • 61
  • thanks, it works. but one more thing, how can I get notified when I get all data? can I use `DispatchGroup` with the current code? – Timeless Oct 21 '17 at 13:39
  • Just put a `DispatchQueue.main.async { ... }` in the block when you're done with the loop. A dispatch group with a single `enter` and `leave` will of course also work. – Palle Oct 21 '17 at 13:53
  • There really is NO need for a dispatch group when the code itself is synchronous. It doesn't make sense. Just call your completion block right after your for-loop. – Brandon Dec 29 '17 at 00:59
  • THe `dispatchSemaphore` in this answer is not created anywhere. – Jonny Apr 22 '19 at 07:55
  • The semaphore is initialized with a value of 0: `let dispatchSemaphore = DispatchSemaphore(value: 0)` before. You can see the full final solution in the question. – Palle Apr 22 '19 at 16:50
  • @Palle could you have a lookt at this question? It is similiar to this one but a bit different and I am stuck: https://stackoverflow.com/questions/62954319/run-for-loop-with-asynchronous-calls-in-order?noredirect=1#comment111329717_62954319 – Chris Jul 17 '20 at 14:57