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

Dispose thumbPainter in Switch to avoid crash #81089

Merged
merged 2 commits into from
Apr 26, 2021

Conversation

goderbauer
Copy link
Member

Fixes b/186203085

The thumpPainter is invoking its onChanged callback (_handleDecorationChanged) when the given ImageProvider finally completes to trigger a repaint. If the Switch is already disposed at that moment, the notifyListeners call in _handleDecorationChanged crashes. To avoid all that, the thumbPainter must be disposed when the Switch is disposed or when it is replaced with another painter.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 23, 2021
@google-cla google-cla bot added the cla: yes label Apr 23, 2021
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM, have a nice weekend

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac framework_tests_misc has failed. Please fix the issues identified (or deflake) before re-applying this label.

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM


@override
void dispose() {
_cachedThumbPainter?.dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we null _cachedThubPainter and other internal fields so that holding a reference to a disposed _MaterialSwitch doesn't prevent the image from being GC'd?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, will make that change.

@goderbauer goderbauer force-pushed the switchCrasher branch 2 times, most recently from 8531cda to 21cc219 Compare April 26, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants