Completion handler called twice (with threading)

Question

I'm currently testing this code in the Xcode 10 playground (Swift 5):

func one() {
    let test = "bla"
    two(test, completion: { (returned) in
        print(returned)
        })
}

func two(_ test: String, completion: @escaping (_ returned: String?) -> Void) {
    DispatchQueue.global(qos:.background).async {
        if !test.isEmpty {
            //Some slow stuff
            DispatchQueue.main.async {
                return completion("hi!")

            }
        }

        //Also some slow stuff
        DispatchQueue.main.async {
            return completion(nil) //can't have this in "else"!
        }
    }
}

one()

The problem is that both "hi" and "nil" are printed.

If I get rid of the threading, it works fine but with it it seems like it gets to the second DispatchQueue.main.async before the first has the chance to return.

There's a lot more stuff going on in the "Some slow stuff" if in my actual code but I can't rely on that taking long enough to return before the second return is called too.

How do I accomplish this: Have the function run in a background thread but return only once on the main thread (like code without threading normally would)?

What is your goal here? Do you want to execute both sets of slow stuff? Do you want just a single completion call? — vacawama, Jul 23 '19 at 10:36
I want to call the `if` and the "slow stuff" within, then call the completion handler to get the result back to `one()`. If the `if` doesn't trigger, I only want to call the "Also some slow stuff" and return `nil`. — Neph, Jul 23 '19 at 11:13
Check out my answer below. I think it does what you want. I'm curious why the second slow stuff couldn't be in an `else`. — vacawama, Jul 23 '19 at 11:17
In this test (in the playground) it could be in an `else` but in my real app it's basically a long list of `if`s with only a single good outcome. Of course I could just return in every `else` but I also have to do some other stuff before I do (e.g. close sockets) and then the same code would be in there appr. 10 times. — Neph, Jul 23 '19 at 11:21

score 6 · Accepted Answer · answered Jul 23 '19 at 11:07

6

I believe your goal is to only call the completion handler once, and when you do you are done. In that case, call return in the .background thread after queueing the completion call on the main thread:

func two(_ test: String, completion: @escaping (_ returned: String?) -> Void) {
    DispatchQueue.global(qos:.background).async {
        if !test.isEmpty {
            //Some slow stuff

            // notify main thread we're done
            DispatchQueue.main.async {
                completion("hi!")
            }

            // we are done and don't want to do more work on the
            // background thread
            return
        }

        //Also some slow stuff
        DispatchQueue.main.async {
            completion(nil)
        }
    }
}

answered Jul 23 '19 at 11:07

vacawama

150,663
30
266
294

Damn, it's really this easy and the order everything is being called in is right too. Thanks! Just wondering: Xcode doesn't complatin about `return completion("hi")` but removing it doesn't seem to make much of a difference, so what does the `return` do exactly in combination with the completion handler? Is it more of a thing like `if somebool == true` being the same as `if somebool`? – Neph Jul 23 '19 at 11:29
1

`return completion("hi!")` will call the completion handler and return the result which is `()`. Since the closure is returning anyway, the `return` is useless. – vacawama Jul 23 '19 at 12:55
1

I just tested it with my actual code outside the playground and it's working perfectly there too, so thanks again! – Neph Jul 23 '19 at 14:10

score 0 · Answer 2 · answered Jul 23 '19 at 10:38

Why it's called twice it pretty obvious. (btw you dont return completion blocks) What you have written is this:

func two(_ test: String, completion: @escaping (_ returned: String?) -> Void) {
    DispatchQueue.global(qos:.background).async {
        if !test.isEmpty {
            //Some slow stuff
            DispatchQueue.main.async {
                completion("hi!")

            }
        }
        completion(nil) //can't have this in "else"!
    }
}

And since the mainthread always keeps going while you do other things on the background threat you will get 2 completions. What you want to do i guess is to remove the second one?

Alternative is that you create a DispatchGroup and enter each call inside it and then write a Dispatch wait till done to wait for all requests to finish

`What you want to do i guess is to remove the second one?` - No, I need the second `DispatchQueue.main.async` too, otherwise `completion(nil)` will return in the background thread. I'll have to look at `DispatchGroup`s but if there's a way to do it just with the `DispatchQueue`, I'd prefer that. — Neph, Jul 23 '19 at 11:17

Ildar.Z · Answer 3 · 2019-07-23T12:28:21.463

0

You can use defer statement to return completion block once after all the If statements. Here just the example with your code, but I hope it's clear.

func two(_ test: String, completion: @escaping (_ returned: String?) -> Void) {

        DispatchQueue.global(qos:.background).async {

            var resultString: String? 

            // Called only once after all code inside this async block.
            defer {
                DispatchQueue.main.async {
                    completion(resultString)
                }
            }

            if !test.isEmpty {

               //Some slow stuff
               resultString = "hi"

               return 
            }

            // Another stuff 
            resultString = nil
        }
    }

edited Jul 23 '19 at 12:28

answered Jul 23 '19 at 11:45

Ildar.Z

666
1
7
17

As stated in the question, I can't do "Also some slow stuff" in an `else`. So while `defer` is a good thing to use in other cases, it won't work here because if I remove the `else` around "Also some slow stuff", it'll just overwrite the result. – Neph Jul 23 '19 at 11:51
I understand it but overwriting of the result depend on your if statements, so you could write the statements that there wouldn't any overwrite of the result. – Ildar.Z Jul 23 '19 at 11:57
You can only be sure that it won't overwrite if you use an `else`, which I can't because of how my actual code is set up/works. Removing the `else` and just having `resultString = nil` there "naked" would always return `nil`. – Neph Jul 23 '19 at 12:00
Yeah but anyway you can mix it with "return", code will be more clear with just one return of completion block. Updated answer. – Ildar.Z Jul 23 '19 at 12:29

Completion handler called twice (with threading)

3 Answers3