Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions mobile/ios/Runner/Images/ImageRequest.swift
Original file line number Diff line number Diff line change
@@ -1,12 +1,58 @@
import Foundation

class ImageRequest {
private struct State {
var isCancelled = false
var callback: ((Result<[String: Int64]?, any Error>) -> Void)?
}

private let state: Mutex<State>

var isCancelled: Bool {
state.withLock { $0.isCancelled }
}
Comment on lines +11 to +13
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mertalev back to your review questions in the other PR:

yes this does need a lock.
The swift docs say that the swift memory model has a conflict if all three conditions are met:

The accesses aren't both reads, and aren't both atomic. They access the same location in memory. Their durations overlap.

_isCancelled is written in cancel() (potentially from the Dart/main thread) and read in isCancelled (from the OperationQueue worker thread).
That's a concurrent write + read on the same memory address.

Swift also clarifies that plain variable access is not atomic:

An access is atomic if it’s a call to an atomic operation on Atomic or AtomicLazyReference, or if it uses only C atomic operations; otherwise it’s nonatomic. For a list of C atomic functions, see the stdatomic(3) man page.

So, a plain Bool write/read is nonatomic, without the lock we have two nonatomic, overlapping, one read, a written...means undefined behaviour.
This was actually a "bug" that was in the pre existing code.


init(callback: @escaping (Result<[String: Int64]?, any Error>) -> Void) {
Copy link
Copy Markdown
Collaborator Author

@LeLunZ LeLunZ Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mertalev Regarding this being nullable, thats how the pigeon communication is typed. null values means cancellation.
Thats also the same on main. See here

self.state = Mutex(State(callback: callback))
}

func cancel() {
let cb = state.withLock { state in
state.isCancelled = true
defer { state.callback = nil }
return state.callback
}
guard let cb else { return }
onCancel()
cb(ImageProcessing.cancelledResult)
}

/// Delivers the result to the callback. Returns true if the callback was called, false if it was already consumed.
@discardableResult
func finish(with result: Result<[String: Int64]?, any Error>) -> Bool {
let cb = state.withLock { state in
defer { state.callback = nil }
return state.callback
}
guard let cb else { return false }
cb(result)
return true
}

func onCancel() {}
}

struct RequestRegistry<T: AnyObject & Sendable>: ~Copyable, Sendable {
private let requests = Mutex<[Int64: T]>([:])

func add(requestId: Int64, request: T) {
requests.withLock { $0[requestId] = request }
}

func get(requestId: Int64) -> T? {
requests.withLock { $0[requestId] }
}

@discardableResult
func remove(requestId: Int64) -> T? {
requests.withLock { $0.removeValue(forKey: requestId) }
Expand Down
76 changes: 25 additions & 51 deletions mobile/ios/Runner/Images/LocalImagesImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,10 @@ import Flutter
import MobileCoreServices
import Photos

class LocalImageRequest {
class LocalImageRequest: ImageRequest {
weak var operation: Operation?
var isCancelled = false
let callback: (Result<[String: Int64]?, any Error>) -> Void

init(callback: @escaping (Result<[String: Int64]?, any Error>) -> Void) {
self.callback = callback
}

func cancel() {
isCancelled = true
override func onCancel() {
operation?.cancel()
}
}
Expand Down Expand Up @@ -67,22 +60,20 @@ class LocalImageApiImpl: LocalImageApi {
}

func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, preferEncoded: Bool, completion: @escaping (Result<[String: Int64]?, any Error>) -> Void) {
let request = LocalImageRequest(callback: completion)
let request = LocalImageRequest { result in
Self.registry.remove(requestId: requestId)
completion(result)
}
let operation = BlockOperation {
if request.isCancelled {
return completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

guard let asset = Self.requestAsset(assetId: assetId)
else {
Self.registry.remove(requestId: requestId)
completion(.failure(PigeonError(code: "", message: "Could not get asset data for \(assetId)", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Could not get asset data for \(assetId)", details: nil)))
return
}

if request.isCancelled {
return completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

if preferEncoded {
let dataOptions = PHImageRequestOptions()
Expand All @@ -99,29 +90,22 @@ class LocalImageApiImpl: LocalImageApi {
}
)

if request.isCancelled {
return completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

guard let data = imageData else {
Self.registry.remove(requestId: requestId)
return completion(.failure(PigeonError(code: "", message: "Could not get image data for \(assetId)", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Could not get image data for \(assetId)", details: nil)))
return
}

let length = data.count
let pointer = malloc(length)!
data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length)

if request.isCancelled {
free(pointer)
return completion(ImageProcessing.cancelledResult)
}

request.callback(.success([
if !request.finish(with: .success([
"pointer": Int64(Int(bitPattern: pointer)),
"length": Int64(length),
]))
Self.registry.remove(requestId: requestId)
])) {
free(pointer)
}
return
}

Expand All @@ -136,38 +120,28 @@ class LocalImageApiImpl: LocalImageApi {
}
)

if request.isCancelled {
return completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

guard let image = image,
let cgImage = image.cgImage else {
Self.registry.remove(requestId: requestId)
return completion(.failure(PigeonError(code: "", message: "Could not get pixel data for \(assetId)", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Could not get pixel data for \(assetId)", details: nil)))
return
}

if request.isCancelled {
return completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

do {
let buffer = try vImage_Buffer(cgImage: cgImage, format: Self.rgbaFormat)

if request.isCancelled {
buffer.free()
return completion(ImageProcessing.cancelledResult)
}

request.callback(.success([
if !request.finish(with: .success([
"pointer": Int64(Int(bitPattern: buffer.data)),
"width": Int64(buffer.width),
"height": Int64(buffer.height),
"rowBytes": Int64(buffer.rowBytes),
]))
Self.registry.remove(requestId: requestId)
])) {
buffer.free()
}
} catch {
Self.registry.remove(requestId: requestId)
return completion(.failure(PigeonError(code: "", message: "Failed to convert image for \(assetId): \(error)", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Failed to convert image for \(assetId): \(error)", details: nil)))
}
}

Expand Down
92 changes: 40 additions & 52 deletions mobile/ios/Runner/Images/RemoteImagesImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ import Flutter
import MobileCoreServices
import Photos

class RemoteImageRequest {
class RemoteImageRequest: ImageRequest {
weak var task: URLSessionDataTask?
let id: Int64
var isCancelled = false
let completion: (Result<[String: Int64]?, any Error>) -> Void

init(id: Int64, task: URLSessionDataTask, completion: @escaping (Result<[String: Int64]?, any Error>) -> Void) {
self.id = id
self.task = task
self.completion = completion
super.init(callback: completion)
}

func cancel() {
isCancelled = true
override func onCancel() {
task?.cancel()
}
}
Expand Down Expand Up @@ -45,82 +42,73 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi {
Self.handleCompletion(requestId: requestId, encoded: preferEncoded, data: data, response: response, error: error)
}

let request = RemoteImageRequest(id: requestId, task: task, completion: completion)
let request = RemoteImageRequest(id: requestId, task: task) { result in
Self.registry.remove(requestId: requestId)
completion(result)
}

Self.registry.add(requestId: requestId, request: request)

task.resume()
}

private static func handleCompletion(requestId: Int64, encoded: Bool, data: Data?, response: URLResponse?, error: Error?) {
guard let request = registry.remove(requestId: requestId) else {
guard let request = registry.get(requestId: requestId) else {
return
}

if request.isCancelled { return }

if let error = error {
if request.isCancelled || (error as NSError).code == NSURLErrorCancelled {
return request.completion(ImageProcessing.cancelledResult)
if (error as NSError).code == NSURLErrorCancelled {
request.finish(with: ImageProcessing.cancelledResult)
} else {
request.finish(with: .failure(error))
}
return request.completion(.failure(error))
}

if request.isCancelled {
return request.completion(ImageProcessing.cancelledResult)
return
}

guard let data = data else {
return request.completion(.failure(PigeonError(code: "", message: "No data received", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "No data received", details: nil)))
return
}

ImageProcessing.queue.addOperation {
if request.isCancelled {
return request.completion(ImageProcessing.cancelledResult)
if encoded {
let length = data.count
let pointer = malloc(length)!
data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length)
if !request.finish(with: .success([
"pointer": Int64(Int(bitPattern: pointer)),
"length": Int64(length),
])) {
free(pointer)
}
return
}

// Return raw encoded bytes when requested (for animated images)
if encoded {
let length = data.count
let pointer = malloc(length)!
data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length)

if request.isCancelled {
free(pointer)
return request.completion(ImageProcessing.cancelledResult)
}

return request.completion(
.success([
"pointer": Int64(Int(bitPattern: pointer)),
"length": Int64(length),
]))
}
ImageProcessing.queue.addOperation {
if request.isCancelled { return }

guard let imageSource = CGImageSourceCreateWithData(data as CFData, nil),
let cgImage = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, decodeOptions) else {
return request.completion(.failure(PigeonError(code: "", message: "Failed to decode image for request", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Failed to decode image for request", details: nil)))
return
}

if request.isCancelled {
return request.completion(ImageProcessing.cancelledResult)
}
if request.isCancelled { return }

do {
let buffer = try vImage_Buffer(cgImage: cgImage, format: rgbaFormat)

if request.isCancelled {
if !request.finish(with: .success([
"pointer": Int64(Int(bitPattern: buffer.data)),
"width": Int64(buffer.width),
"height": Int64(buffer.height),
"rowBytes": Int64(buffer.rowBytes),
])) {
buffer.free()
return request.completion(ImageProcessing.cancelledResult)
}

request.completion(
.success([
"pointer": Int64(Int(bitPattern: buffer.data)),
"width": Int64(buffer.width),
"height": Int64(buffer.height),
"rowBytes": Int64(buffer.rowBytes),
]))
} catch {
return request.completion(.failure(PigeonError(code: "", message: "Failed to convert image for request: \(error)", details: nil)))
request.finish(with: .failure(PigeonError(code: "", message: "Failed to convert image for request: \(error)", details: nil)))
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions mobile/ios/Runner/Images/UnfairLock.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import Foundation

// Can be replaced with OSAllocatedUnfairLock when the deployment target is iOS 16+
final class UnfairLock {
private let _lock: UnsafeMutablePointer<os_unfair_lock>

init() {
_lock = .allocate(capacity: 1)
_lock.initialize(to: os_unfair_lock())
}

deinit {
_lock.deinitialize(count: 1)
_lock.deallocate()
}

@discardableResult
func withLock<T>(_ body: () throws -> T) rethrows -> T {
os_unfair_lock_lock(_lock)
defer { os_unfair_lock_unlock(_lock) }
return try body()
}
}
Loading