'NSInternalInconsistencyException' crash happening when query is changing for CollectionViewDataSource #234
Comments
|
This affects also us, please merge and release 2.0.2 as soon as possible, thank you! |
|
This has been fixed in v2.0.2. Now, when you don't want a data source to send updates to its view component anymore, just set it to nil. dataSource.tableView = nil |
|
Thanks for the quick fix! Now it is working correctly. |
|
Hey @morganchen12 , Thanks for the quick update, but I've been messing around with the code and I'm still getting crashes. Figuring that these are FUI objects I thought I'd ask you if you know what's up. To give you an update, I'm clearing out the old data like this And I'm getting this crash: When I reset the data source, then wait a second before starting the next query or When I reset the data source, then immediately start the next query Stack trace: @swftvsn hey, if you got yours to work, and you know what my issue is, please chime in. I appreciate your help! |
|
This looks like an app-specific timing/shared state issue. Can you post more of your code? |
|
@morganchen12 I've created a gist with the two files that are involved in the crash. The BrowseVC is where users toggle between online and offline users through queries, and the FirebaseDB file is where the queries are generated with singleton. Thanks for looking into it, Morgan! :) |
|
Hm, you may need to call If your collection views are massively different, I'd suggest keeping two separate views in a tab bar controller (or other similar controller), as this may help avoid stale state bugs. Let me know if you still run into crashes. |
|
Thank you for your input, @morganchen12. I was thinking that the reloading of data was still free, but I guess it's not after I null the data source's associated view. I'll work on the code some and report back. If it continues acting funky, I'll also take your suggestion on keeping two views. That was something I had in mind, but thought the memory and data would've been wasteful. Better than crashes by all measures though! |
|
Hi, I actually couldn't get it to work correctly, the problem is just more hidden. Isn't there always a chance for an error because if we take a look at this pattern: self.dataSource = FUITableViewDataSource(collection: sortedArray, view: table) { tableView, indexPath, snapshot in
let cell = tableView.dequeueReusableCell(withIdentifier: "cell", for: indexPath) as! MyCell
...
return cell
}
table.dataSource = self.dataSource
table.reloadData()We can see that the sortedArray will start observing in the constructor of the So what we actually would need to do, is to delay the What do you think @morganchen12 Using
where the numbers work out so that the "after update" count is always double the number inserted. |
|
Yeah, imo this is an issue in the design of our data source class. We should be separating initialization from the pushing of events, since it's surprising (and bad) that initializing an object should have side effects. I'm thinking something like /** initializes the data source, doesn't do anything */
public init(collection: FUICollection)
/** attaches the data source to the view and starts sending updates */
public func bind(toView: UICollectionView) -> Void
/** unbinds the data source from the view */
public func unbind() -> VoidUnfortunately, this would be a breaking API change. |
|
It sounds exactly right to remove the side effects. Do you have any estimate when this could be merged and released? We actually have an app waiting to go to approval, but this is the last major issue that prevents us from going forward. |
|
There's nothing blocking a new release, but I'd like to aggregate any breaking API changes we'd want now so I don't end up bumping the major version every 2 weeks. Any suggestions? |
|
Not really, the only thing we'd like to see added is #230, but I don't think this release should wait any new features as the issue may be affecting multitude of apps resulting in hard to trace (seemingly) random bugs. So I vote for quick releases now and in future too. |
|
I appreciate Firebase and the developers who are in charge of it. The bug I have created a ticket for, would be great to have the Firebase team help me and the community by solving it. But, @swftvsn, you do know that you're pretty much demanding iOS Firebase to be updated to solve a problem that you are dealing with on an app that no one yet uses, to break the code bases on the entire iOS Firebase community, right? Are you even a paying Firebase customer? 🤣 The bugs we're dealing with, not all that many developers are dealing with them (vocally and up in arms), so I think your reasoning for quick releases is bull$hit. |
|
@datureezy I too am thankfull for the great product and associated code that we can use to speed up the development of our apps. I'm not demanding anything, sorry if the text came out too harsh, just gave my wish how to proceed, of course from my point of view. I'm strong proponent for quick and numerous releases. It's way better than the old once a year thing that some companies seem to do. After all devs can and should limit / nail down the version in their pod / what not file and update when they want to. We're in control of that. But we can't update if the fixes are not released. We're not in control of that. And it's not like this is the main firebase release, but the ios accompanying components that we're talking about. And yes, we're happy to pay the price they ask for the service. Back to this issue though - @morganchen12 could we get some kind of time frame / educated guess for this release? Are we talking about weeks or months? If it's going to take long then I probably should fork your branch and build it from there? |
|
No worries, this release has been cut. I appreciate all your concerns! https://github.com/firebase/FirebaseUI-iOS/releases/tag/v3.0.0 |
|
Awesome! Thank you! |
|
Hey @morganchen12, |
|
Hi, The way our app is designed we need to change the data source during infinite left / right swipe scenario (Changing the month user is observing), so we can't disable the user controls. (Which is extremely bad anyway from UX standpoint.) Also disabling for a moment is bad because the time frame when the error will manifest itself will grow when you have more data or slower devices. I hope this helps someone in the future, I ended up doing this, and don't get the crashes anymore (so far..., fingers crossed) //Unbind the old datasource from the tableview if there is already one
self.dataSource?.unbind()
//Set empty datasource to the tableview
self.tableView.dataSource = EmptyDataSource()
//Instruct tableview to reload.
//This causes it to remove any rows there might be lingering around.
self.tableView.reloadData()
// Assign new datasource to self.dataSource here. Do not start to observe yet.
//This will replace the empty datasource we just set and
//start observing and updating the empty tableView
self.dataSource?.bind(to: self.tableView)
//Ready. Do not reload tableView past this point, datasource will push changes to it!One future improvement point would probably be for the data source to clean after itself to reset the table to empty state, but that's not very important as we can do it ourselves. |
|
@swftvsn, are you populating your cells in the closure after you initialize your dataSource? I'm running my code just like yours, step by step (but just for collection views), and my app still crashes with:
With your queries being even more varied than mine (offline & online), I'm not sure how yours is not crashing, and mine crashes just after 5-6 new queried calls. My Empty DataSource |
|
Hmm, that's really odd, or then some codepath in UICollectionDataViewSource wasn't updated vs. the tableview one. That said, I do as little as possible in the closure (I only extract some ints and strings from the FIRDataSnapshot and set those to my own data transfer object and set it to the cell), and nothing that would change the UI. The real work is done in func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) method that sets the label texts and images, starts the animations etc. |
|
Thanks for your input, @swftvsn. Perhaps me pulling out 3-4 values per row in that closure is causing the slow-up followed by a crash. I'll try extracting only a lookup key and do some willDisplay type method for collectionView cells and report back. 👍 |
|
Extremely good thread, thanks! |
|
I also getting the "NSInternalInconsistencyException", but I have 2 Views, each has a tableview and gets populated via Does this also apply for me? I can't figure out how to fix this Exception. |
|
@Warhost can you file a new issue with full stack trace and other info? |
|
@morganchen12 Solved. Updated my code to the lates sample practices, does the Job. |
|
@datureezy Did you ever solve your problem? I am facing the exact error, after changing data sources 4-5 times I will get the internal inconsistency crash. Before binding to a new ref I do the steps listed above (unbind, clear data source, etc). I'm also using a CollectionView and not a TableView. |
|
@adornoventura Same thing here. |
|
@Warhost It seems like no matter what I do it will always crash after loading a new data source 4-5 times. On the contrary, if I perform the button clicks that change the data source very slowly I can maybe get 10 data source changes before it fails. |
|
@adornoventura Could you post the code to what you are describing? |
|
@Warhost I have one CollectionView that has to switch between 4 different data sets based on two segmented controls (one "outside" control and one "inside" control). I have one method that swaps the data source and I pass it the string representation of the child node from where the data should be loaded. The method is called from various places, most notably the segmentValueChanged callback from one of the segment controls. In the if collectionViewDataSource != nil block I've tried a different combinations of things, none of which solves the problem (such as setting an empty data source as mentioned in previous posts). I didn't include my code from the closure as I tried running with nothing in the closure (just returning an empty cell) and it does nothing in regards to the problem. |
|
@adornoventura Okay this looks sort of similar. I am trying to follow the database example of the firebase quickstart repo, but also face these Exception issues. |
Step 2: Describe your environment
Step 3: Describe the problem:
For a little more background, I have received help from Morgan Chen regarding my problem on StackOverflow: here
On my collection view, I have a segmented control that allows users to toggle queries from online to offline users, and vice versa.
Even after clearing out the data source as recommended, the app crashes because of NSInternalInconsistencyExceptions.
I've been clearing out my data source like this:
I am still getting crashes.
Steps to reproduce:
Observed Results:
Expected Results:
Relevant Code:
Initially getUserData() is being called when the VC is first created, but for every query change, resetQuery() is being called which removes the old data source and runs getUserData() again.
The text was updated successfully, but these errors were encountered: