flex_
flex_

Reputation: 733

Capture of variable with non-sendable type NSImage in a '@Sendable closure'

I'm new to swift concurrency, I noticed that my code freezes UI when trying to initialize NSImage, so I decided to do it on the global thread like this:

func chooseImage() {
    Task { @MainActor in
        
        let openPanel = NSOpenPanel()
        openPanel.prompt = "Select File"
        openPanel.allowsMultipleSelection = false
        openPanel.canChooseDirectories = false
        openPanel.canCreateDirectories = false
        openPanel.canChooseFiles = true
        openPanel.allowedContentTypes = [.image]
        openPanel.begin { (result) -> Void in
            if result.rawValue == NSApplication.ModalResponse.OK.rawValue {
                guard let url = openPanel.url else {return}
                self.fileURL = url
                loading = true
                DispatchQueue.global(qos:.userInitiated).async
                {
                    guard let chosenImage = NSImage(contentsOf: url) else { return }
                    DispatchQueue.main.async()
                    {
                        autoreleasepool {
                            self.nsImage = chosenImage
                        }
                        self.recongizeText()
                        loading = false
                    }
                }
   }

Does this mean that NSImage is not thread safe? does this warning really mean that there might be some kind of race condition, and if so what would be the best way to fix this?

Upvotes: 2

Views: 2627

Answers (2)

Rob Napier
Rob Napier

Reputation: 299565

@vadian gives the best "how?" answer. My answer is to the "why?"

NSImage is explicitly documented not to be thread-safe, except when used in specific ways, which makes it non-Sendable. It would be very nice if this documentation were in the NSImage page itself, but it is explained in the Thread Safety Summary of the Threading Programming Guide, under NSImage Restrictions (emphasis added):

One thread can create an NSImage object, draw to the image buffer, and pass it off to the main thread for drawing. The underlying image cache is shared among all threads.

NSImage was designed at a time when threading was pretty rare and almost painfully explicit, unlike today's world of concurrency everywhere. UIImage, for example, is thread safe. But various operations on NSImage that modify the cache can cause safety problems. As with mutable Foundation objects, the general rule is "any thread is ok, but only one at a time."

Technically what you're doing is fine. You construct the NSImage on a background thread, and then pass it to the main thread for use. The compiler today just cannot prove that you never use the image on two threads. When SE-0414 is available, almost certainly after WWDC, this code may become legal.

In the meantime (and if SE-0414 doesn't fix this case), I'd probably use @vadian's answer. The other approach, and the one I usually use in my own code, is to work entirely in CGImage, which is thread-safe/Sendable, and convert to NSImage/UIImage only when needed. I often do this for portability reasons between iOS and Mac as much as thread-safety. If that's not a concern for you, and you don't have more complex needs than this, then @vadian's answer is best (though I highly recommend adding some error checking instead of try?).

Upvotes: 2

vadian
vadian

Reputation: 285210

As mentioned in the comments mixing GCD and Swift Concurrency is bad practice.

This is a translation to async/await

@MainActor
func chooseImage() async  {
    
    let openPanel = NSOpenPanel()
    openPanel.prompt = "Select File"
    openPanel.canChooseDirectories = false
    openPanel.allowedContentTypes = [.image]
    let result = await openPanel.begin()
    guard result == .OK, let url = openPanel.url else { return }
    self.fileURL = url
    loading = true
    let task = Task.detached {
        try? Data(contentsOf: url)
    }
    guard let data = await task.value, let image = NSImage(data: data) else { return }
    self.nsImage = image
    self.recongizeText() 
    loading = false
}

Upvotes: 2

Related Questions