Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Realm Storage Auto Update Affects Multiple Sections Crash #21

Closed
anthonydito opened this issue Aug 17, 2017 · 10 comments
Closed

Realm Storage Auto Update Affects Multiple Sections Crash #21

anthonydito opened this issue Aug 17, 2017 · 10 comments

Comments

@anthonydito
Copy link

In Realm Storage. If a change affects multiple sections (for instance, if a realm object is moved from one section to another on a change), there will be a crash surfaced by UITableView.endUpdates that states the number of cells inserted or deleted is not expected.

@DenTelezhkin
Copy link
Owner

Can i ask you to reproduce this in a simple test? If it's reproduced, fix will be easier to implement.

@DenTelezhkin DenTelezhkin self-assigned this Aug 17, 2017
@anthonydito
Copy link
Author

@DenHeadless Will do

@anthonydito
Copy link
Author

@DenHeadless The example at https://github.com/anthonydito/CrashExample contains a simple example to demonstrate the issue. If you add a cell to section 1 and section 2 using the buttons and then hit the switch button it will crash.

@DenTelezhkin
Copy link
Owner

Thanks! I will try to get to it as soon as possible.

@DenTelezhkin
Copy link
Owner

@anthonydito Okay, so I did some research of this problem and here's what I found:

Crash happens, because even though event is a move event(insert and delete), since we use two instances of Realm, they report operations separately, starting with the delete. Then only delete animation is getting applied to UITableView, even though datasource on Realm side is actually already changed with both delete and insert. And then crash happens because of datasource inconsistency.

I'm not really sure how to proceed with the fix of this problem, as it's really is connected to Realm behaviour, as well as my decision to use single RealmResults instance for each separate section. When we subscribe to updates, we don't actually know if that is a batch operation or not. Changes are coming one by one, so deletion and insertion, which should be a single move operation, are not bundled together, and there's no signal from Realm that we should wait a little longer before animating those changes on UITableView. This can be verified by simply changing this line:
https://github.com/DenHeadless/DTModelStorage/blob/master/Source/Realm/RealmStorage.swift#L119
to this hacky code:

DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { self.finishUpdate() }

With this change your code perfectly works and beautifully animates, because both delete and insert update were able to deliver in time from two separate notification observers. However of course this is not a solution, since it's hacky and unstable by any means.

What do you think, maybe there are different Realm mechanisms that we can use to solve this issue properly?

@anthonydito
Copy link
Author

The workaround that I implemented is simply changing the value to a bogus intermediate value is not within either section. I'll definitely look into this problem further and fork the repo.

@stefandesu
Copy link

Hi,

I found this issue because I had the same problem with using Realm in a table view with multiple sections. The more elegant workaround that I'm now using is to manually keep track of the number of rows in each section, and to only update the number for the respective section when a notification is fired. For me it works perfectly, so maybe it would be an option for you as well.

@DenTelezhkin
Copy link
Owner

One thing I'm also considering is calculating diff at the moment we apply performBatchUpdates operation. 7.0.0 release contains ability to defer datasource updates as well as UI updates until update is ready to animate. We could then calculate diffs using some diffing library like https://github.com/jflinter/Dwifft for example, or have the abilty to hook up your custom differ, if needed. And then since the UI updates is calculated not by Realm, but by a diffing library, this issue could potentially be solved.

@bartleby
Copy link

bartleby commented Aug 28, 2018

If you select more than 30 items and then delete them, you'll get a crash =(
DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { self.finishUpdate() }
this code does not help =(

I using collectionView + realmStorage (but my collectionView look like tableView)

PS. I have only one section in collectionView

@DenTelezhkin DenTelezhkin removed their assignment May 15, 2019
@DenTelezhkin
Copy link
Owner

Hi all!

It seems that multi-section RealmStorage currently has an unfixable design flaw with no clear way to fix it on a framework level.

However, with introduction of Diffable Datasources in iOS 13, which I think should become a de-facto standard in updating UITableView, this problem has a solution. It seems that with diffable datasources we don't need RealmStorage for Realm nor CoreDataStorage for CoreData in current form, because sections and items are built from snapshot update rather than from Storage.

At this point, i think we can consider RealmStorage and CoreDataStorage to be soft-deprecated, since there's much better solution going forward.

I encourage you to try new beta releases with support for diffable datasources in iOS 13, and hope this solution works for you.

https://github.com/DenTelezhkin/DTTableViewManager/blob/master/Guides/7.0%20Migration%20Guide.md
https://github.com/DenTelezhkin/DTCollectionViewManager/blob/master/Guides/7.0%20Migration%20Guide.md
https://github.com/DenTelezhkin/DTModelStorage/blob/master/Guides/8.0%20Migration%20Guide.md

I'm going to close this issue for now, but in any case, looking forward to any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants