0

I have an OperationQueue with multiple custom Operations which all append to the same array on completion (each operation downloads a file from user's iCloud and when it's done it appends the file to the array)

This, sometimes, causes the app to crash, because several operations try to edit the array at the same time.

How can I prevent this and only edit the array 1 operation at a time but running all operations simultaneously?

I must use OperationQueue because I need the operations to be cancelable.

func convertAssetsToMedias(assets: [PHAsset],
                           completion: @escaping (_ medias: [Media]) ->()) {
    
    operationQueue = OperationQueue()
    var medias: [Media] = []
    
    operationQueue?.progress.totalUnitCount = Int64(assets.count)

    for asset in assets {
        
        // For each asset we start a new operation
        let convertionOperation = ConvertPHAssetToMediaOperation(asset)
        convertionOperation.qualityOfService = .userInteractive
        
        convertionOperation.completionBlock = { [unowned convertionOperation] in
            
            let media = convertionOperation.media
        
            medias.append(media) // CRASH HERE (sometimes)
               
            self.operationQueue?.progress.completedUnitCount += 1
            if let progress = self.operationQueue?.progress.fractionCompleted {
                self.delegate?.onICloudProgressUpdate(progress: progress)
            }
            
            convertionOperation.completionBlock = nil
        }
    
        operationQueue?.addOperation(convertionOperation)
    }
    
    operationQueue?.addBarrierBlock {            
        completion(medias)
    }
    
}

Edit 1:

The Media file itself is nothing big, just a bunch of metadata and a url to an actual file at documents directory. There are usually about 24 medias max at 1 run. The memory is barely increasing during those operations. The crash never occured due to a lack of memory.

The operation ConvertPHAssetToMediaOperation is a subclass of AsyncOperation where isAsynchronous propery is set to true. That's how I construct the Media object in the end of each operation:

self.media = Media(type: mediaType, url: resultURL, creationDate: date)
self.finish()

Edit 2: The crash is always the same: enter image description here

enter image description here

SmartTree
  • 1,381
  • 3
  • 21
  • 40
  • You could use `let serialQueue = DispatchQueue(label: "medias.queue")`, and serialQueue.sync { medias.append(media) }` to ensure that the access to it can't by simulataneous. – Larme Apr 26 '22 at 11:15
  • @Larme That's what I thought and tried as well, but still got the crash happening. – SmartTree Apr 26 '22 at 11:19
  • Because you are not "interacting with the user" you shouldn't be using a qos of userInteractive, consider using userInitiated instead – Shadowrun Apr 26 '22 at 11:47
  • Larme is correct, that you should be synchronizing your access. Temporarily [turn on TSAN](https://developer.apple.com/documentation/xcode/diagnosing-memory-thread-and-crash-issues-early/) and have it warn you of non-thread-safe interaction with `media`. Synchronizing the `append`, alone, is likely insufficient, as you have to synchronize _all_ interaction, and TSAN will warn you of where you have data races. – Rob Apr 26 '22 at 14:53
  • 1
    It strikes me that there could be other issues, though. E.g. how is memory usage? What exactly is this `Media`? Just a bunch of meta data or is it the `Data` associated with the actual asset? If actual assets, then loading all of that into an `Array` is impractical. Also, when doing something with so many assets, I would generally set queue’s `maxConcurrentOperationCount` to mitigate how many assets are loaded into memory at the same time. There simply is not enough here to diagnose the problem, much less to suggest a precise solution… – Rob Apr 26 '22 at 15:01
  • Besides what people said above, the basic rule is: if your code may access array from multiple threads, you cannot use a simple array, as it's not thread-safe. You have to implement an AtomicArray (there's lots of examples for swift online, or you can use a new Apple's swift-atomics). – timbre timbre Apr 26 '22 at 15:29
  • 1
    Any synchronization mechanism will do. It does not have to be an atomic array. – Rob Apr 26 '22 at 17:42
  • 1
    @Rob thank you for the idea about TSAN. I've added an edit with more information about `Media` and operations. – SmartTree Apr 26 '22 at 19:59
  • 1
    Thanks for the clarifications. Sounds like it is not a running-out-of-memory issue. Thing is, if you added synchronization (GCD serial queue, reader-writer, locks, atomic array, or whatever) around _all_ interactions with this array (not just where you append) and it is still crashing then the problem must rest elsewhere. Still, a clean bill of health from TSAN will help eliminate unsynchronized data races from the list of contenders. – Rob Apr 26 '22 at 20:11
  • Also, please be aware that `OperationQueue` operations are only cancellable BEFORE they start. Once an operation starts executing, there's no generalized way to cancel it offered by any of the mechanisms. If you want cancel-ability for an already-running operation, you'll have to implement that mechanism yourself. – ipmcc Apr 26 '22 at 20:40
  • @Rob thank you for your thoughts. I wish to use TSAN, however, I just figured it's not possible to run it on a device. And I don't see how I can test iCloud assets downloading on a simulator, bummer. I've tried adding locking with NSLock so I will test it and see how it goes. And I don't edit the array in any way before reading it, besides appending. – SmartTree Apr 26 '22 at 23:37
  • Gotcha. “And I don't edit the array in any way before reading it, besides appending.” … That’s good, but when you synchronize your writes, you have to synchronize all other interaction, even if only reads. You can’t have any other simulataneous interaction with this shared resource while the write is underway. – Rob Apr 27 '22 at 00:11

0 Answers0