onevcat / Kingfisher

A lightweight, pure-Swift library for downloading and caching images from the web.
MIT License
23.41k stars 2.66k forks source link

ImageProgressive.swift:121 Crash #2216

Open b-onc opened 8 months ago

b-onc commented 8 months ago

Check List

Thanks for considering to open an issue. Before you submit your issue, please confirm these boxes are checked.

Issue Description

We're using Kingfisher 7.10.2 via Carthage.

When loading multiple images using Swift concurrency, sometimes we see a crash originating from ImageProgressive.swift:121

What

We have this code:

Task {
  let images = await withTaskGroup(of: [UUID: UIImage].self) { group in
      var results = [UUID: UIImage]()

      for entry in imageTemplates { // `imageTemplates` is of type `[(uuid: UUID, url: URL)]`
          group.addTask {
              await withCheckedContinuation { continuation in
                  KingfisherManager.shared.retrieveImage(
                      with: .network(entry.url),
                      options: kingfisherHeadersOptions, // some headers so we can load images from our CDN
                      progressBlock: nil,
                      downloadTaskUpdated: nil
                  ) { [weak self] result in
                      guard let self else {
                          return
                      }
                      switch result {
                      case let .success(imageResult):
                          continuation.resume(returning: [entry.uuid: imageResult])
                      case let .failure(error):
                          // Handle error
                      }
                  }
              }
          }
      }

      for await dict in group {
          results.merge(dict) { $1 }
      }

      return results
  }

  // Do something with `images`
}

Our aim is to concurrently load multiple images and keep the ordering in a given array. The code works almost always. Sometimes we see the loading fail due to various reasons but that's not the topic of this issue.

We see a crash originating from this part of the code. It looks like this:

image

This looks like #1449 but that issue is quite old, Swift Concurrency wasn't available back then. Perhaps it's somewhat related?

Reproduce

Unfortunately we are not able to reproduce the issue.

bharat9806 commented 8 months ago

Task { let images = await withTaskGroup(of: [UUID: UIImage?].self) { group in var results = [UUID: UIImage?]()

    for entry in imageTemplates { // Assuming `imageTemplates` is of type `[(uuid: UUID, url: URL)]`
        group.addTask {
            do {
                return try await withCheckedThrowingContinuation { continuation in
                    KingfisherManager.shared.retrieveImage(
                        with: .network(entry.url),
                        options: kingfisherHeadersOptions, // Headers for CDN
                        progressBlock: nil,
                        downloadTaskUpdated: nil
                    ) { result in
                        switch result {
                        case .success(let imageResult):
                            continuation.resume(returning: [entry.uuid: imageResult.image])
                        case .failure(let error):
                            print("Error loading image: \(error)")
                            continuation.resume(returning: [entry.uuid: nil]) // Consider how you want to handle errors.
                        }
                    }
                }
            } catch {
                print("Encountered error: \(error)")
                return [entry.uuid: nil] // Or handle differently as needed.
            }
        }
    }

    for await dict in group {
        results.merge(dict) { $1 }
    }

    return results.compactMapValues { $0 } // Remove nil values if they are not needed.
}

// Do something with `images`

}

@b-onc try this i don't know if it will work

b-onc commented 8 months ago

We don't have a problem with error handling