Async in a serial queue bug

Question

init(type: String, color: String, brand: String, price: String, image: UIImage) {
    self.type = type
    self.color = color
    self.brand = brand
    self.price = price
    self.imageURL = setImage(image: image)
}

func setImage(image: UIImage?) -> String {
    if image != nil {
        let unwrappedImage: UIImage = image!
        let image = unwrappedImage.pngData()!
        let storage = Storage.storage()
        let storageRef = storage.reference()
        let data = image
        var iURL: String = "images/\(String(describing: UUID())).jpg"
        let dataRef = storageRef.child(iURL)
        _ = dataRef.putData(data, metadata: nil) { (metadata, error) in
            guard let metadata = metadata else {
                print("error uploading image")
                return
            }
            _ = metadata.size
            dataRef.downloadURL { (url, error) in
                if error != nil {
                    print("error uploading image")
                } else {
                    print(url!.absoluteString)
                    iURL = url!.absoluteString
                }
            }
        }
        return iURL
    }
    return ""
}

I am trying to take an image upload it to Firebase storage and save the absolute URL to a field in the struct. After the struct is created, I upload it to a Firebase database. My issue is that at the moment the struct is created and uploaded with the old value of iURL, before the async call goes through with the absolute url.

I tried to use a dispatch queue to fix this:

let serialQueue = DispatchQueue(label: "imageupload.serial.queue")
        serialQueue.sync {
            _ = dataRef.putData(data, metadata: nil) { (metadata, error) in
                guard let metadata = metadata else {
                    print("error uploading image")
                    return
                }
                _ = metadata.size
                dataRef.downloadURL { (url, error) in
                    if error != nil {
                        print("error uploading image")
                    } else {
                        print(url!.absoluteString) //(1)
                        iURL = url!.absoluteString
                    }
                }
            }
        }
        serialQueue.sync {
            
            self.imageURL = iURL //(2)
            return
        }

But it made no difference (2) still happened before (1)

The call `putData()` will indeed be synchrone, BUT its `completionHandler` still remain asynchrone. And same for `downloadURL`. It's usually a bad idea to try to transform async into sync. So why do you want to that? — Larme, Apr 09 '21 at 17:43
@Larme how would you recommend making the handlers synchronous? Or do you have any recommendations on a better pattern? — Dante, Apr 09 '21 at 17:48
Give context on why you want to make it synchrone. Tell us what's the code calling that, etc. An asynchrone is usually handler this way: do the async, when it's done, notify me, I'll react and do something in response (like changing an image because the web call is done), etc. It's just a different way of thinking instead of line after line. — Larme, Apr 09 '21 at 17:49
also `let serialQueue = DispatchQueue(label: "imageupload.serial.queue")` should be defined on class level, not inside some function — timbre timbre, Apr 09 '21 at 17:52
On a point of stye, `if image != nil { let unwrappedImage: UIImage = image! let image = unwrappedImage.pngData()!` is an anti-pattern. You want `if let image = image?.pngData() {` No need for force unwrapping — Paulw11, Apr 09 '21 at 21:06
I don't think a DispatchQueue or DispatchGroup is really needed. To make it really easy, I would suggest replacing this piece of code `iURL = url!.absoluteString` with the code that creates the struct and writes it to Firestore. The url is valid at that point so that's where to do it. For another option see my answer [here](https://stackoverflow.com/questions/61238857/firebase-getdocument-querysnapshot-is-not-working/61276604#61276604) that you could use. — Jay, Apr 10 '21 at 12:49

score 0 · Answer 1 · answered Apr 09 '21 at 18:02

The approach I've been using for something like that is to use a dispatchGroup:

    let dataGroup = DispatchGroup()
             dataGroup.enter()
 _ = dataRef.putData(data, metadata: nil) { (metadata, error) in
                guard let metadata = metadata else {
                    print("error uploading image")
                    dataGroup.leave()
                    return
                }
                _ = metadata.size
                dataRef.downloadURL { (url, error) in
                    if error != nil {
                        print("error uploading image")
                        dataGroup.leave()
                    } else {
                        print(url!.absoluteString) //(1)
                        iURL = url!.absoluteString
                        dataGroup.leave()
                    }
                }
            }

            dataGroup.notify(queue: .main) {
              self.imageURL = iURL
              print(self.imageURL) //(2)
           }

If you have multiple .enter() calls for different work, it will wait for all of them to .leave() before the notify block is activated.

This looks promising! But now I'm getting a "Escaping closure captures mutating 'self' parameter" — Dante, Apr 09 '21 at 18:16
@Dante make your closure `@escaping` and your function `mutating`, and look up what those do. — vrwim, Apr 09 '21 at 19:27
@virwim i understand mutating but wouldn’t I want non-escaping — Dante, Apr 09 '21 at 19:34
You are going to have an `@escaping` closure. That is unavoidable. It doesn't matter. — Paulw11, Apr 09 '21 at 21:04

score 0 · Answer 2 · answered Apr 09 '21 at 18:52

0

You can use DispatchGroup, add let group = DispatchGroup() before you enter async block you can add group.enter(), in the completion block you must add group.leave(), and repeat it for all your async blocks, then add group.notify(queue: yourQueue){ //upload to Firestore}

answered Apr 09 '21 at 18:52

Ahmed Allam

104
8

Wouldn’t this give me an escape closure catches mutating ‘self’ parameter error like in Kendall’s answer? – Dante Apr 09 '21 at 18:55
DispatchGroup is used for waiting two or more async blocks running at the same time and you need to be notified after all this async tasks done. in your case, Why you didn't use self.imageUrl = url!.absoluteString in dataRef.downloadURL directly from the beginning? – Ahmed Allam Apr 11 '21 at 14:55

Async in a serial queue bug

2 Answers2