I just tried to reproduce this with the code given in #14452 and I am seeing a slightly different chain of events to get to the crash: It doesn't seem to be the user gesture that's interrupted by the automated animation that's causing the crash. Instead, it seem to be the other way around: The animation is interrupting an "in-progress" user gesture and that causes the crash. (Maybe that's what was meant by the PR description and I misunderstood?) Here is the sequence of events I am seeing:

• Fling the scrollable so that it continues to move after the finger is lifted (generates Hold, Drag, Ballistic scroll activities in that order)
• while the scrollable is still moving, put the finger down (triggers a hold activity, and will trigger another drag activity)
• However, before the drag activity starts, the animation is triggered, interrupting the user gesture (this creates a Driven scroll activity)
• When now the previously triggered drag activity starts, things crash

Is this another sequence that leads to a crash? Or is that what you meant?

Also, I am not convinced that this actually fixes the root cause of the crash. It removes a notification, that is used in user-defined code to trigger the animation. The animation could also be triggered via other means from user code (e.g. a timer that just happens to fire in the right moment) and that would still crash...

Also, I am not convinced that this actually fixes the root cause of the crash. It removes a notification, that is used in user-defined code to trigger the animation. The animation could also be triggered via other means from user code (e.g. a timer that just happens to fire in the right moment) and that would still crash...

No, the timer will not cause the crash, it has to happen at the exact moment at this line.

I just tried to reproduce this with the code given in #14452 and I am seeing a slightly different chain of events to get to the crash: It doesn't seem to be the user gesture that's interrupted by the automated animation that's causing the crash. Instead, it seem to be the other way around: The animation is interrupting an "in-progress" user gesture and that causes the crash. (Maybe that's what was meant by the PR description and I misunderstood?) Here is the sequence of events I am seeing:

• Fling the scrollable so that it continues to move after the finger is lifted (generates Hold, Drag, Ballistic scroll activities in that order)
• while the scrollable is still moving, put the finger down (triggers a hold activity, and will trigger another drag activity)
• However, before the drag activity starts, the animation is triggered, interrupting the user gesture (this creates a Driven scroll activity)
• When now the previously triggered drag activity starts, things crash

Is this another sequence that leads to a crash? Or is that what you meant?

Yes that is what i observed as well.

I just tried to reproduce this with the code given in #14452 and I am seeing a slightly different chain of events to get to the crash: It doesn't seem to be the user gesture that's interrupted by the automated animation that's causing the crash. Instead, it seem to be the other way around: The animation is interrupting an "in-progress" user gesture and that causes the crash. (Maybe that's what was meant by the PR description and I misunderstood?) Here is the sequence of events I am seeing:

• Fling the scrollable so that it continues to move after the finger is lifted (generates Hold, Drag, Ballistic scroll activities in that order)
• while the scrollable is still moving, put the finger down (triggers a hold activity, and will trigger another drag activity)
• However, before the drag activity starts, the animation is triggered, interrupting the user gesture (this creates a Driven scroll activity)
• When now the previously triggered drag activity starts, things crash

Is this another sequence that leads to a crash? Or is that what you meant?

Yes. I also agree. Very well explained. :)

I'm going to see about addressing the notification error, since I think we all agree that a HoldActivity does not constitute that the ScrollDirection is idle.

As for other cases where an animation may interrupt a user gesture, I will see about creating a reproduction of the timer case you recommended @goderbauer and see if it causes a crash.

I created a gist that triggers the animation after a time delay instead of after a notification.

Available here: https://gist.github.com/Piinks/955c449d2764e4591b574859c7a0395f

If you hold (don't drag) after triggering the future that delays the animation, the HoldActivity is properly disposed of before the DrivenActivity begins. In this case, it does not crash.
Is this similar to the concern you mentioned earlier @goderbauer?

@chunhtai Ah, I see. Looking at the line you pointed out:

The underlying problem is that _disposeHold is executed (as a result of the dispatched scroll notification) before hold returns and therefore before _hold is assigned. Because of this, _hold is never cleared properly.

We also dispatch a scrollendnotification when the hold starts:

What if you trigger the animation from that one?

Codecov Report

Merging #37267 into master will increase coverage by 0.82%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #37267      +/-   ##
==========================================
+ Coverage   55.22%   56.04%   +0.82%     
==========================================
  Files         193      193              
  Lines       18172    18141      -31     
==========================================
+ Hits        10036    10168     +132     
+ Misses       8136     7973     -163

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cf1a35...5f8e4de. Read the comment docs.

What if you trigger the animation from that one?

Yes, I think that is another corner case. it seems like we should somehow know how to reset _hold when it gets interrupted before 'hold' returns.

What if you trigger the animation from that one?

Yes, I think that is another corner case. it seems like we should somehow know how to reset _hold when it gets interrupted before 'hold' returns.

Thank you for the feedback. I will look into this corner case as well and see if adding a reset _hold type method would be a better approach to fixing this crash. I will close this for now while I investigate further.

That being said, I am still thinking this pr is worth fixing as hold activity should not be consider as idle according to the documentation

I've added a reset-like path for when an animation is fired before the hold can be disposed of properly. A similar functionality existed in the class for keeping track of _currentDrag status, so I tried to mirror this through _currentHold. I will add more tests if we agree this is a good direction to move in to solve this problem. 😉

After offline chat with @goderbauer, the problem here lies in the potential gap that is created when a scroll notification is used to initiate an activity. It leaves open a small window where the activity can interrupt a currently executing one since scroll notifications are dispatched at different points in an activity's execution.
This leads us to believe that the use of scroll notifiers for triggering these events in not a sound use of the API. I'm looking currently for a good alternative to recommend and document, and will close this since I do not believe we can try to stifle each possible scroll notification creating an undesirable state.