3

So, I'm having a bit of a time trying to get DispatchGroup to keep a for loop from iterating before a long asynchronous operation has completed. Most examples I've found are fairly straight forward and clear, but I can't seem to get my simple test case to work as I would expect.

let group = DispatchGroup()

    for i in 1...3 {
        group.enter()
        print("INDEX \(i)")
        asynchronousOperation(index: i, completion: {
            print("HELLO \(i)")
            self.group.leave()

        })
        print("OUTSIDE \(i)")
    }


func asynchronousOperation(index: Int, completion: @escaping () -> ()) {
    DispatchQueue.main.asyncAfter(deadline: DispatchTime.now()+5) {
        print("DONE \(index)")
        completion()

    }
}

This ends up printing

START
INDEX 1
OUTSIDE 1
INDEX 2
OUTSIDE 2
INDEX 3
OUTSIDE 3
DONE 1
HELLO 1
DONE 2
HELLO 2
DONE 3
HELLO 3

I would've expected it to print something more like

START
INDEX 1
OUTSIDE 1
HELLO 1
INDEX 2
OUTSIDE 2
HELLO 2
INDEX 3
OUTSIDE 3
HELLO 3

Insofar as the next "INDEX" following an OUTSIDE wouldn't be printed until group.leave() had been called inside the asynchronousOperation()

Probably something simple I'm misunderstanding — any ideas?

bleeckerj
  • 554
  • 7
  • 18
  • 1
    You are not making any use of the dispatch group. With using `group.wait` or `group.notify`, the group is useless. And a group isn't really what you need here anyway. – rmaddy Dec 10 '17 at 04:40
  • For the sake of future readers, no discussion on this topic is complete without acknowledging that this is an anti-pattern, to be avoided except for very specific situations, because if used incorrectly (and it almost always is), it can cause all sorts of problems, both subtle and serious. It’s an intoxicating pattern, because it seems like it simplifies one’s networking code, replacing complicated asynchronous patterns with enticingly simple and synchronous ones. But it’s almost always the wrong solution. – Rob Jul 29 '19 at 16:58
  • @Rob Can you describe what the preferred pattern would be in this context? – bleeckerj Sep 03 '19 at 14:12
  • Kumar and Andrea have shown you how to alter your code to achieve your “expected” output, but they involve blocking the current thread, which is generally a bad idea (and a horrible idea if the current thread is the main thread). Preferable patterns include (a) letting it run asynchronously (like your first output) and organize the results as desired; or (b) if you absolutely must have one asynchronous task not start until the prior one is done, make custom asynchronous `Operation` subclasses with dependencies between them. – Rob Sep 03 '19 at 16:14
  • In short, this question is a bit abstract and generated answers that achieved your expected output, but are generally wrong in practice. If the question is “why did I get the output I did” (and if you’re still unclear on that matter), I’m happy to post an answer to that. If your question was “I know why I got the output I did, but wonder what I need to do to achieve the desired output”, then I’d push back and ask for real-world example of what `asynchronousOperation` is doing and why you want the current thread to wait for it. The correct solution will depend upon what the broader problem is. – Rob Sep 03 '19 at 16:28

2 Answers2

5

execute the below code to get the proper output:

for i in 1...3 {
    let semaphore = DispatchSemaphore(value: 0) // create a semaphore with a value 0. signal() will make the value 1.
    print("INDEX \(i)")
    asynchronousOperation(index: i, completion: {
        print("HELLO \(i)")
        semaphore.signal() // once you make the signal(), then only next loop will get executed.
    })
    print("OUTSIDE \(i)")
    semaphore.wait() // asking the semaphore to wait, till it gets the signal.
}

   func asynchronousOperation(index: Int, completion: @escaping () -> ()) {
    DispatchQueue.global().asyncAfter(deadline: DispatchTime.now()+5) {
        print("DONE \(index)")
        completion()
    }
}

Output :

INDEX 1 OUTSIDE 1 DONE 1 HELLO 1

INDEX 2 OUTSIDE 2 DONE 2 HELLO 2

INDEX 3 OUTSIDE 3 DONE 3 HELLO 3

Kumar Reddy
  • 792
  • 6
  • 15
  • Thanks. That solves the question. I'll need to study more on how semaphores work in GDC. Thank you. – bleeckerj Dec 10 '17 at 15:43
  • 1
    Minor change, but you don’t need to create a new `DispatchSemaphore` on each iteration of the loop. I’d pull the `let semaphore = ...` line outside of the loop and declare it before the `for`. – Rob Jul 29 '19 at 15:18
3

In order to achieve that, If I understood correctly, you may want to use a DispatchSemaphore. Doing so:

let semaphore = DispatchSemaphore(value: 1)

for i in 1...3 {
   self.semaphore.wait()
   print("INDEX \(i)")
   asynchronousOperation(index: i, completion: {
     print("HELLO \(i)")
     self.semaphore.signal()
   })
   print("OUTSIDE \(i)")
}


func asynchronousOperation(index: Int, completion: @escaping () -> ()) {
  DispatchQueue.global().asyncAfter(deadline: DispatchTime.now()+5) {
    print("DONE \(index)")
    completion()
  }
}
mugx
  • 9,869
  • 3
  • 43
  • 55
  • You want the call to `wait` to be after the call to `asynchronousOperation` (just before or after the `print("OUTSIDE")` line. – rmaddy Dec 10 '17 at 04:43
  • @AndreaMugnaini This is close, but didn't quite work with initializing semaphore with `DispatchSemaphore(value: 1)`. @KumarReddy's answer with `DispatchSemaphore(value: 0)` produces the expected flow. Now I know where I need to study to understand why that makes a difference. Thank you. – bleeckerj Dec 10 '17 at 15:43
  • @rmaddy I have tried to use this code for my loop but it gives `XPS connection error`. And I also tried to use DispatchGroup and with that I got log like this when authenticate with apple id only https://paste.ubuntu.com/p/cs2XYj6gdD/ – Bhavin Bhadani Mar 22 '19 at 04:34
  • @rmaddy Please check why this issue for dispatch group? https://stackoverflow.com/questions/55294986/dispatchgroup-issue-when-authentcate-apple-id – Bhavin Bhadani Mar 22 '19 at 07:46
  • @rmaddy - Perhaps not. This is the “run x at a time” pattern (declare semaphore with non-zero value, waiting before async call, signaling in the completion handler), which is just a more generalized solution of the OPs question. E.g. let’s say you had 100 requests and wanted only four at a time, Andrea's pattern will accomplish that. And “only run one at a time” scenario is just a specialized case of the “run x at a time” pattern. – Rob Jul 29 '19 at 15:24
  • @Rob Did you mean to reply to bleeckerj? – rmaddy Jul 29 '19 at 16:13
  • 1
    @rmaddy - No, I was directing that to you (and my apologies for reopening an very old thread; lol). Your comment said to move the `wait` call, but Andrea has it in the correct place. Your comment about moving it would make sense if the dispatch group was initialized with `0` instead of `1` (i.e., the traditional “let’s do one at a time” pattern). But if you use a positive value for the `DispatchGroup` (the less common, but still valid, “run x at a time” pattern), then Andrea’s code is correct, and it would be incorrect to move the `wait` call as you advised. – Rob Jul 29 '19 at 16:41
  • 1
    FWIW, the use of the positive value for dispatch semaphore, shown here, is generally used for the “run _x_ tasks at a time” (used to control the degree of concurrency). For example, if you had 100 tasks to run, but you didn’t want to run more than 4 concurrently at any given time, you’d use `DispatchSemaphore(value: 4)` like [shown here](https://gist.github.com/robertmryan/fc3d924d663e2018740979f26c45de37). While the OPs question can be considered a special case of this pattern, where _x_ is `1`, the [accepted answer](https://stackoverflow.com/a/47736275/1271826) is the canonical solution. – Rob Jul 29 '19 at 17:16