mikaoj / BSImagePicker

A multiple image picker for iOS
MIT License
1.35k stars 380 forks source link

Crash in AssetsViewController.photoLibraryDidChange #254

Closed Haccoon closed 4 years ago

Haccoon commented 4 years ago

Hi! Thanks for such a great library.

When I switched the app to the background, and launched the camera app to take a picture, when I came back to the foreground, it crashed inAssetsViewController.photoLibraryDidChange(_:) every time.

The log is shown below.

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of items in section 0. The number of items contained in an existing section after the update (67) must be equal to the number of items contained in that section before the update (67), plus or minus the number of items inserted or deleted from that section (1 inserted, 0 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).

Haccoon commented 4 years ago

why use performBatchUpdates ?

Is there a problem if I only handle it with reloadData?

    func photoLibraryDidChange(_ changeInstance: PHChange) {
        guard let changes = changeInstance.changeDetails(for: fetchResult) else { return }
        // Since we are gonna update UI, make sure we are on main
        DispatchQueue.main.async {
            self.fetchResult = changes.fetchResultAfterChanges
            self.collectionView.reloadData()

            // No matter if we have incremental changes or not, sync the selections
            self.syncSelections(self.store.assets)
        }
    }
mikaoj commented 4 years ago

It won't crash if you use reload data, sure. However it won't look good. The UI will blink/glitch every time photoLibraryDidChange is called as it reload all cells instead of just updating those that have changed.

Put a breakpoint in the performBatchUpdates block and figure out what is going on. Does it have incremental changes? Does it have deleted, inserted or moved items? And are we updating the collection view with those changes.

I'll look into too whenever I have time.

Haccoon commented 4 years ago

If the UI blinking is the only problem, is it okay to take temporary action by using reloaddata first? Crash is a serious problem.

I'm still analyzing it with performanceBatchUpdates, but it's too difficult. ;(

mikaoj commented 4 years ago

If you are fine with that.

  1. Fork this repo
  2. Make the fix
  3. Update your Podfile to use your fork until I've made a proper fix.
    pod 'BSImagePicker', :git => 'https://github.com/Haccoon/BSImagePicker.git'
mikaoj commented 4 years ago

So I just tried reproducing the crash, but it works for me. Could you give me any more details?

Haccoon commented 4 years ago

I attach a reenactment video. Photos taken with cameras should be added, but the app is crash.

Apr-23-2020 11-33-54

mikaoj commented 4 years ago

Yeapp, reproducible on iOS 12. Thanks :)

Haccoon commented 4 years ago

Oh, this is not a crash that occurs only with iOS 12. It also happens in 13.

mikaoj commented 4 years ago

Alright, worked for me on iOS 13 but crashes on iOS 12 :)

mikaoj commented 4 years ago

Try 3.1.4 - it should be fixed now. Thanks for helping out :)