Swift Multiple async web service response handler

Question

Approach 1: -

let fetchedImageArray = [Images]()
let requestQueue = OperationQueue()

func fetchProductImage() {
    let completionBlock = BlockOperation { [weak self] in
        updateProductByFetchMediumImageResult()
    }
    fetchProductImageOperations.append(completionBlock)

    var fetchProductImageOperations = [Operation]()
    for index in 0...products.count - 1 {
        let operation = FetchProductImageOperation(products[index], completion { image in
            fetchedImageArray.append(image)
        })

        fetchProductImageOperations.append(operation)
        completionBlock.addDependency(operation)
    }
    requestQueue.addOperations(fetchProductItemImageOperations, waitUntilFinished: false)
}

Approach 2: -

let isolationQueue = DispatchQueue(label: "com.humana.isolation", attributes: .concurrent)
var threadSafeProductsImage: [Images] {
    get {
        return isolationQueue.sync {
            fetchedImageArray
        }
    }
    set {
        isolationQueue.async(flags: .barrier) {
            self.fetchedImageArray = newValue
        }
    }
}

Approach 3:-

func fetchProductImage() {
    let completionBlock = BlockOperation { [weak self] in
        updateProductByFetchMediumImageResult()
    }
    fetchProductImageOperations.append(completionBlock)
    let semaphore = DispatchSemaphore(value: 1)

    var fetchProductImageOperations = [Operation]()
    for index in 0...products.count - 1 {
        let operation = FetchProductImageOperation(products[index], completion { image in
            semaphore.wait()
            self.threadSafeProductsImage.append(image)
            semaphore.signal()
        })

        fetchProductImageOperations.append(operation)
        completionBlock.addDependency(operation)
    }
    requestQueue.addOperations(fetchProductItemImageOperations, waitUntilFinished: false)
}

In my application I have list of product. And I have to fetch the image of each product and reassign to products. To achieve this I am using the above code.

I have tried three approach and and in each approach I have issue. Please help me to correct my code-

Approach 1 -

Have created an array to add web-service response. I do call the fetch web service as multiple operations and add responses into array. In this approach I do get crash when I append the response into array, as array is not thread safe. (Accessing thread safe array from multiple async call.)

Approach 2 -

In this approach I have converted array into a thread safe array with step 1 approach to add the image response into array. At this point of time I avoid the crash but there is issue as I do see lot of image response doesn’t gets append into image array after web service completion.

Approach 3 -

In this approach I have added semaphore wait and signal to sync the append response process. But in this approach I am facing deadlock condition and my background thread stuck and don’t process anything.

What am I missing or is there any better approach to perform the same approach.

The only requirement that I have to fetch the image of each product from list of products in multiple thread operations and then add those response into an array to use further.

Answer 1

A few observations:

1. Yes, if FetchProductImageOperation calls its completion handler on some random thread, you will need to synchronize the results. We often define our completion handlers to run on the main queue (or a serial queue, at least) to avoid the need for additional synchronization.

2. But the threadSafeProductsImage computed property is brittle and will not work the way you are using it. Consider this line:

   self.threadSafeProductsImage.append(image)
   

   That is retrieving a copy of the array, appending a result to this copy, but then discard that array copy. That is not going to work.

   The property accessors are simply the wrong level to perform synchronization. You want to synchronize the append, not the property accessors.

3. FWIW, while the reader-writer pattern (the concurrent queue with asynchronous updates with barrier) is intuitively appealing, in practice it introduces more problems than it solves. It is much better to stick with a simple serial queue. (Or nowadays, an actor.)

4. Semaphores are an anti-pattern and they can easily lead to deadlocks. That having been said, there is not enough here to reproduce a deadlock, so we cannot comment further about this particular case.

---

Unrelated to your immediate problems, a few other suggestions:

5. I would be careful about using properties to store the results as they come in. I might be inclined to store the results in local variables and only update model objects in the final completion operation.

6. Note that the results may not finish in the order in which they were started. Either resort the results when you are done, or use order-independent patterns (e.g., store the results in a dictionary rather than an array).

7. Seems that the fetchProductImageOperations is unnecessary. Just add operations to the queues directly.

8. Rather than 0...products.count - 1, I might suggest 0..<products.count, or, even better, products.indices.

E.g., while I would define FetchProductImageOperation to call its custom completion handler on the main queue, relieving the calling routine from having to do its own synchronization, here I will assume that you do want to synchronize within this method:

let requestQueue: OperationQueue = {
    let queue = OperationQueue()
    queue.maxConcurrentOperationCount = 6                                       // limit operation count to something reasonable
    return queue
}()

func fetchProductImage(for products: [Product]) {                               // take products as parameter so we're working with safe copy
    let synchronizationQueue = DispatchQueue(label: #function)
    var results: [Int: Images] = [:]                                            // unsorted results

    let completionOperation = BlockOperation { [weak self] in
        var images: [Images] = []
        synchronizationQueue.sync {
            images = products.indices.compactMap { results[$0] }                // if you want sorted array, let's `map` the `results`
        }
        self?.updateProductByFetchMediumImageResult(for: images)                // change this to take results as a parameter, not relying on property
    }

    for index in products.indices {                                             // rather than `0 ... products.count - 1`
        let operation = FetchProductImageOperation(products[index]) { image in
            synchronizationQueue.async {
                results[index] = image
            }
        }

        completionOperation.addDependency(operation)
        requestQueue.addOperation(operation)
    }

    OperationQueue.main.addOperation(completionOperation)                       // add completion operation to main queue
}