Crash when Toggleable widget disappears while pointer is down #76411

Activity

1. goderbauer

   added

   c: crashStack traces logged to the consoleStack traces logged to the console

   frameworkflutter/packages/flutter repository. See also f: labels.flutter/packages/flutter repository. See also f: labels.

   f: gesturesflutter/packages/flutter/gestures repository.flutter/packages/flutter/gestures repository.

   customer: money (g3)

   P2

   on Feb 19, 2021
2. goderbauer commented on Feb 19, 2021

   

   goderbauer

   on Feb 19, 2021

   MemberAuthor

   More actions

   Test case to reproduce:

     testWidgets('Do not crash when widget disappears while pointer is down', (WidgetTester tester) async {
       Widget buildCheckbox(bool show) {
         return MaterialApp(
           home: Directionality(
             textDirection: TextDirection.ltr,
             child: StatefulBuilder(
               builder: (BuildContext context, StateSetter setState) {
                 return MediaQuery(
                   data: MediaQueryData.fromWindow(window),
                   child: Material(
                     child: show ? Checkbox(value: true, onChanged: (_) { }) : Container(),
                   ),
                 );
               },
             ),
           ),
         );
       }
   
       await tester.pumpWidget(buildCheckbox(true));
       final Offset center = tester.getCenter(find.byType(Checkbox));
       // Put a pointer down on the screen.
       final TestGesture gesture = await tester.startGesture(center);
       await tester.pump();
       // While the pointer is down, the widget disappears.
       await tester.pumpWidget(buildCheckbox(false));
       expect(find.byType(Checkbox), findsNothing);
       // Release pointer after widget disappeared.
       gesture.up();
     });

   This crash appears to affect all widgets that are backed by the RenderToggleable: Checkbox, Switch, and Radio.
3. goderbauer commented on Feb 19, 2021

   

   goderbauer

   on Feb 19, 2021

   MemberAuthor

   More actions

   The culprit is that the RenderToggleable owns a TapGestureRecognizer, which is never disposed:

   flutter/packages/flutter/lib/src/material/toggleable.dart

   Lines 63 to 67 in 75470a0

   	_tap = TapGestureRecognizer()
   	..onTapDown = _handleTapDown
   	..onTap = _handleTap
   	..onTapUp = _handleTapUp
   	..onTapCancel = _handleTapCancel;

   We now observe the following sequence of events:

   1. PointerDown is routed to the tap recognizer owned by the RenderToggleable.
   2. Tap recognizer registers a route with the PointerRouter to receive all events for the pointer that caused the down event.
   3. The RenderToggleable is detached from the tree (the tap recognizer doesn't know about this, its registered route stays in place).
   4. The PointerUp event is dispatched to the tap recognizer via the registered route. The tap handler set by the RenderToggleable is called.
   5. Tap handler calls sendSemanticsEvent (which assumes the render object is attached) and crashes.

   The tap handler also calls other user provided callbacks, which may also crash (depending on what they do) if they are called in the detached state...

   If we would be properly disposing the tap recognizer, it would unregister its pointer route and the crash would go away. Problem: There's no good point in the life cycle of the RenderObject to call dispose. Disposing in detach is not great because during global key reparenting, the render object could be attached again. If we instantiate a new TapRecognizer in attach, we wouldn't be able to detect a gesture that started before reparenting and ends after.

   We probably have to move the ownership of the tap controller to e.g. a State object and dispose it in the State's dispose.
4. goderbauer commented on Feb 19, 2021

   

   goderbauer

   on Feb 19, 2021

   MemberAuthor

   More actions

   This problem has also been discussed in #49999 (comment). The context there was a little different (memory leaks).

   In #51606 the CupertinoSwitch was refactored to move the gesture recognizer out of the renderobject into a state object.
5. goderbauer commented on Feb 19, 2021

   

   goderbauer

   on Feb 19, 2021

   MemberAuthor

   More actions

   Related: #14204
6. pedromassangocode

   added

   passed first triage

   on Feb 23, 2021
7. Hixie

   assigned

   goderbauer

   on Feb 24, 2021
8. goderbauer commented on Mar 3, 2021

   

   goderbauer

   on Mar 3, 2021

   MemberAuthor

   More actions

   The fix was reverted due to a failure in devicelab. Fixing that looks straight-forward. Hoping that I can re-apply the fix this week.
9. goderbauer commented on Mar 4, 2021

   

   goderbauer

   on Mar 4, 2021

   MemberAuthor

   More actions

   The re-land is staged at #77263.
10. fluttergithubbot

    closed this as completedin #77263on Mar 4, 2021
11. github-actions commented on Aug 5, 2021

    

    github-actionsBot

    on Aug 5, 2021

    More actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
12. github-actions

    locked as resolved and limited conversation to collaborators on Aug 5, 2021
13. flutter-triage-bot

    added

    P0Critical issues such as a build break or regressionCritical issues such as a build break or regression

    and removed

    P2

    on Jun 28, 2023