Fix DraggableScrollableSheet leaks Ticker#102916
Fix DraggableScrollableSheet leaks Ticker#102916fluttergithubbot merged 1 commit intoflutter:masterfrom
Conversation
flutter-dashboard
bot
added
f: scrolling
| ballisticController.dispose(); | ||
| if (_ballisticControllers.contains(ballisticController)) { | ||
| _ballisticControllers.remove(ballisticController); | ||
| ballisticController.dispose(); |
There was a problem hiding this comment.
This should be outside of the if statement.
There was a problem hiding this comment.
@dnfield
Thanks for reviewing this PR.
In some edge cases (the one in #101061, which is also the one in the test I added with this PR), code in whenCompleteOrCancel might be called after _DraggableScrollableSheetScrollPosition.dispose. If I kept this line outside the if statement it would throw an error about the AnimationController being already disposed.
To avoid this error, I put this line in the if statement and in _DraggableScrollableSheetScrollPosition.dispose I add a call to _ballisticControllers.clear() so if an AnimationController ends just after the call to _DraggableScrollableSheetScrollPosition.dispose it is no more in _ballisticControllers and ballisticController.dispose won’t be called as it is in the if statement. If the AnimationController ends before, it is still in _ballisticControllers and it will be disposed by this line (and removed from _ballisticControllers so we won't try to dispose it again in _DraggableScrollableSheetScrollPosition.dispose)
There was a problem hiding this comment.
Ahh ok, thanks, that makes sense.
3 participants
Description
_DraggableScrollableSheetScrollPosition.goBallisticcreates new AnimationControllers but doesn’t always dispose them. It disposes AnimationControllers when the animation is stopped or canceled. If theDraggableScrollableSheetis disposed, while some AnimationControllers are running, some Tickers leak.The solution is similar to the one in #102504
Related Issue
Fixes #101061
Tests
Add one test in
draggable_scrollable_sheet_test.dartPre-launch Checklist
///).