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

DiffableDataSource: use weak self in _applyDiffSnapshot completion handler #126

Merged

Conversation

lachenmayer
Copy link
Collaborator

@lachenmayer lachenmayer commented Aug 27, 2024

Have you read the Contributing Guidelines? 🖤

Closes #125

Describe your changes

As discussed in the issue, this fixes a crash with Fatal error: Attempted to read an unowned reference but the object was already deallocated when the collection view disappears from the view hierarchy while the animation is still in progress.

Repro: https://github.com/lachenmayer/ReactiveCollectionsKitCrashRepro
Expected behavior: collection view appears & disappears a couple of times, and then the app crashes.

Repro with applied fix: https://github.com/lachenmayer/ReactiveCollectionsKitCrashRepro/tree/apply-fix
Expected behavior: collection view continuous to appear & disappear without a crash.

The collection view may have disappeared from the view hierarchy
before the `DiffableDataSource._applyDiffSnapshot` completion
handler is called. Weakly retain self to ensure that
`DiffableDataSource` does not outlive the collection view.

Fixes jessesquires#125
Copy link
Owner

@jessesquires jessesquires left a comment

Choose a reason for hiding this comment

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

Thanks so much @lachenmayer! 💯 🙌🏼 This is perfect.

Would you mind adding a changelog entry? We can add a new version, 0.1.6. (Just follow the existing format 😊)

@lachenmayer
Copy link
Collaborator Author

Done :)

@jessesquires
Copy link
Owner

Thanks @lachenmayer! 💯

@jessesquires jessesquires merged commit 17edec9 into jessesquires:main Sep 2, 2024
4 checks passed
@jessesquires
Copy link
Owner

@jessesquires
Copy link
Owner

jessesquires commented Sep 2, 2024

@lachenmayer per the contributing guidelines, after having a PR get merged, you'll be added as a contributor. 😄

Invite sent. This gives you push access, so you no longer have to use a fork.

No pressure to contribute further, but if you'd like to, you are very welcome to!

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

Successfully merging this pull request may close these issues.

[Bug]: Fatal error: Attempted to read an unowned reference but the object was already deallocated
2 participants