Fix race condition causing crash when interacting with an animating scrollable #164392

victorsanni · 2025-02-28T19:07:51Z

Fix #163297
Fix #14452

Partial patch: #37267

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…crollable

Piinks

Google testing failures are unrelated. 👍

Piinks · 2025-03-03T16:40:22Z

packages/flutter/lib/src/widgets/scrollable.dart

@@ -861,7 +861,10 @@ class ScrollableState extends State<Scrollable>

  void _handleDragDown(DragDownDetails details) {
    assert(_drag == null);
-    assert(_hold == null);
+    // _hold might be non-null if the scroll position is currently animating.
+    if (_hold != null) {


This should be null. Do we have a scenario where it crashes here?

@victorsanni , I am also curious what the sequence of event that causes this to be none null. My suspicion is that the previous event was cancelled (thus going ballisitic) and starting a new one, but would be good to have confirmation.

packages/flutter/lib/src/widgets/scrollable.dart

victorsanni · 2025-03-04T03:14:21Z

packages/flutter/test/widgets/scrollable_animations_test.dart

@@ -103,6 +104,80 @@ void main() {
      reason: 'Expected an animation.',
    );
  });
+
+  testWidgets('HoldActivity can interrupt ScrollPosition.animateTo', (WidgetTester tester) async {


I can't seem to make the tests fail on master/ reproduce the crash with the test framework.

Fixed thanks to help from @Renzo-Olivares! Just needed to wrap the scroll view in a NotificationListener and call position.animateTo when the scroll view is interacted with.

Piinks

chunhtai

LGTM

bobekos · 2025-03-05T18:22:39Z

@victorsanni
Great to see that this is fixed. Any chance we can cherry pick this into the next HotFix version?

…nimating scrollable (flutter/flutter#164392)

…crollable (flutter#164392) Fix flutter#163297 Fix flutter#14452 Partial patch: flutter#37267 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

…nimating scrollable (flutter/flutter#164392)

flutter/flutter@2578d97...321fbc0 2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712) 2025-03-06 mdebbar@google.com [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491) 2025-03-06 matt.kosarek@canonical.com [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296) 2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701) 2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677) 2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669) 2025-03-06 jonahwilliams@google.com [Impeller] use device private on non-iOS devices. (flutter/flutter#164601) 2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661) 2025-03-05 codefu@google.com Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657) 2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649) 2025-03-05 50643541+Mairramer@users.noreply.github.com Adds animateToItem to the CarouselController (flutter/flutter#162694) 2025-03-05 30870216+gaaclarke@users.noreply.github.com Cleanup content context (flutter/flutter#164229) 2025-03-05 34465683+rkishan516@users.noreply.github.com Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700) 2025-03-05 jonahwilliams@google.com [Impeller] fix macOS managed memory. (flutter/flutter#164635) 2025-03-05 jacksongardner@google.com [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588) 2025-03-05 victorsanniay@gmail.com Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392) 2025-03-05 58529443+srujzs@users.noreply.github.com Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582) 2025-03-05 engine-flutter-autoroll@skia.org Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630) 2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622) 2025-03-05 harri.kirik@lab.mobi Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308) 2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599) 2025-03-05 34871572+gmackall@users.noreply.github.com Implement `clipPath` Mutator for hcpp (flutter/flutter#164525) 2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600) 2025-03-05 jonahwilliams@google.com [macos] prefer integrated GPU. (flutter/flutter#164569) 2025-03-05 muhatashim@google.com Enforce minSdk constraint for Android Flutter (flutter/flutter#164251) 2025-03-05 dkwingsmt@users.noreply.github.com Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111) 2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590) 2025-03-05 psturm@esri.com when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456) 2025-03-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…crollable (flutter#164392) Fix flutter#163297 Fix flutter#14452 Partial patch: flutter#37267 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

…n animating scrollable (#164708) …Fix race condition causing crash when interacting with an animating scrollable (#164392) Fix #163297 Fix #14452 Partial patch: #37267 https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md  *Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.* *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

Fix race condition causing crash when interacting with an animating s…

f3883b1

…crollable

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Feb 28, 2025

victorsanni added 3 commits February 28, 2025 11:18

Additional test

a335580

Merge branch 'master' into nav-bar-crash

be57cb5

Linux analyze

1a425ce

victorsanni requested review from chunhtai and Piinks February 28, 2025 19:19

victorsanni added 2 commits February 28, 2025 13:19

Dispose controllers in tests

eb5d3e5

Merge branch 'master' into nav-bar-crash

ff5c39d

Piinks reviewed Mar 3, 2025

View reviewed changes

Apply review suggestions

d9205cd

github-actions bot added the a: animation Animation APIs label Mar 4, 2025

victorsanni commented Mar 4, 2025

View reviewed changes

victorsanni added 3 commits March 3, 2025 19:14

Merge branch 'master' into nav-bar-crash

0dda61b

Add test

3951cde

Merge branch 'master' into nav-bar-crash

5ce3a48

victorsanni requested a review from Piinks March 4, 2025 23:53

Piinks approved these changes Mar 5, 2025

View reviewed changes

chunhtai approved these changes Mar 5, 2025

View reviewed changes

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025

auto-submit bot added this pull request to the merge queue Mar 5, 2025

Merged via the queue into flutter:master with commit 57cddce Mar 5, 2025
75 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025

victorsanni deleted the nav-bar-crash branch March 5, 2025 18:15

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Manual roll Flutter from 04d39343a691 to 57cddce92438 (45 revisions) flutter/packages#8800

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

d085db1

…nimating scrollable (flutter/flutter#164392)

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 57cddce92438 (45 revisions) flutter/packages#8801

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

1e5d7ec

…nimating scrollable (flutter/flutter#164392)

engine-flutter-autoroll mentioned this pull request Mar 5, 2025

Roll Flutter from 04d39343a691 to 1e9676fc9527 (46 revisions) flutter/packages#8802

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

1cd064a

…nimating scrollable (flutter/flutter#164392)

bobekos mentioned this pull request Mar 6, 2025

[CP][Stable] Fix race condition causing crash when interacting with an animating scrollable #164708

Merged

18 tasks

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to 099e6d39fe38 (27 revisions) flutter/packages#8806

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

4e537e8

…nimating scrollable (flutter/flutter#164392)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

077e50f

…nimating scrollable (flutter/flutter#164392)

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to d778ed25a485 (28 revisions) flutter/packages#8807

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

57cddce92 Fix race condition causing crash when interacting with an a…

4db9e93

…nimating scrollable (flutter/flutter#164392)

engine-flutter-autoroll mentioned this pull request Mar 6, 2025

Roll Flutter from 2578d97f32c8 to 321fbc0e7e81 (29 revisions) flutter/packages#8808

Merged

Piinks mentioned this pull request Mar 7, 2025

Flutter Notable Commits - 2025 #161460

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix race condition causing crash when interacting with an animating scrollable #164392

Fix race condition causing crash when interacting with an animating scrollable #164392

victorsanni commented Feb 28, 2025 •

edited

Loading

Piinks left a comment

Piinks Mar 3, 2025

chunhtai Mar 3, 2025 •

edited

Loading

victorsanni Mar 4, 2025

victorsanni Mar 4, 2025

Piinks left a comment

chunhtai left a comment

bobekos commented Mar 5, 2025

Fix race condition causing crash when interacting with an animating scrollable #164392

Fix race condition causing crash when interacting with an animating scrollable #164392

Conversation

victorsanni commented Feb 28, 2025 • edited Loading

Pre-launch Checklist

Piinks left a comment

Choose a reason for hiding this comment

Piinks Mar 3, 2025

Choose a reason for hiding this comment

chunhtai Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

victorsanni Mar 4, 2025

Choose a reason for hiding this comment

victorsanni Mar 4, 2025

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

chunhtai left a comment

Choose a reason for hiding this comment

bobekos commented Mar 5, 2025

victorsanni commented Feb 28, 2025 •

edited

Loading

chunhtai Mar 3, 2025 •

edited

Loading