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

[ASDataController] Fix a crash in table view caused by executing an empty change set during layoutSubviews #416

Merged
merged 2 commits into from Jul 5, 2017

Conversation

nguyenhuy
Copy link
Member

  • Previously, when a change set is empty, ASDataController forwards the change set to its delegate right away, without dispatching to its editing queue and then back to main.
  • This behaviour can potentially cause bad internal states in UITableView which trigger a crash reported in Scrolling ASTableNode in editing mode causes NSRangeException #83.
  • Fix by still reusing the existing pending map, because the data source's state has not changed, but go through the editing queue and main queue tunnel.

Fixes #83.

…ing layoutSubviews

- Previously, when a change set is empty, `ASDataController` forwards the change set to its delegate right away, without dispatching to its editing queue and then back to main.
- This behaviour can potentially cause bad internal states in UITableView which trigger a crash reported in TextureGroup#83.
- Fix by still reusing the existing pending map, because the data source's state has not changed, but go through the editing queue and main queue tunnel.
Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Beautiful! I know @rewandy has been patiently waiting for a fix for this, and I love the unity of the code paths in the new approach.

@nguyenhuy nguyenhuy merged commit 4a1aea2 into TextureGroup:master Jul 5, 2017
@rewandy
Copy link
Contributor

rewandy commented Jul 5, 2017

We can confirm this addresses our issue! Thanks so much for tackling this - we can finally get with the times and update to Texture!

@nguyenhuy
Copy link
Member Author

You're welcome, @rewandy! We've just found out that we got a number of crash reports in the Pinterest app with the same stack trace. It would be super hard for us to debug it without your details GitHub issue and test project. So thank you!

bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…mpty change set during layoutSubviews (TextureGroup#416)

* Fix a crash in table view caused by executing an empty change set during layoutSubviews
- Previously, when a change set is empty, `ASDataController` forwards the change set to its delegate right away, without dispatching to its editing queue and then back to main.
- This behaviour can potentially cause bad internal states in UITableView which trigger a crash reported in TextureGroup#83.
- Fix by still reusing the existing pending map, because the data source's state has not changed, but go through the editing queue and main queue tunnel.

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

Successfully merging this pull request may close these issues.

None yet

3 participants