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

Unexpected null value in BaseTapGestureRecognizer._checkDown #82107

Closed
jhass opened this issue May 8, 2021 · 17 comments
Closed

Unexpected null value in BaseTapGestureRecognizer._checkDown #82107

jhass opened this issue May 8, 2021 · 17 comments
Labels
a: error message Error messages from the Flutter framework c: crash Stack traces logged to the console f: gestures flutter/packages/flutter/gestures repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.3 Found to occur in 2.3 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on r: invalid Issue is closed as not valid

Comments

@jhass
Copy link

jhass commented May 8, 2021

I'm seeing the following crash in a Flutter 2.0.6 based app:

Null check operator used on a null value

Stack trace:

#0 BaseTapGestureRecognizer._checkDown (package:flutter/src/gestures/tap.dart:287)
#1 BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:265)
#2 MultipleTapGestureRecognizer.rejectGesture (package:flutter_html/src/utils.dart:54)
#3 GestureArenaManager._resolve (package:flutter/src/gestures/arena.dart:214)
#4 GestureArenaEntry.resolve (package:flutter/src/gestures/arena.dart:53)
#5 OneSequenceGestureRecognizer.resolve (package:flutter/src/gestures/recognizer.dart:260)
#6 BaseTapGestureRecognizer.resolve (package:flutter/src/gestures/tap.dart:253)
#7 PrimaryPointerGestureRecognizer.handleEvent (package:flutter/src/gestures/recognizer.dart:472)
#8 PointerRouter._dispatch (package:flutter/src/gestures/pointer_router.dart:93)
#9 PointerRouter._dispatchEventToRoutes.<anonymous closure> (package:flutter/src/gestures/pointer_router.dart:138)
#10 _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:397)
#11 PointerRouter._dispatchEventToRoutes (package:flutter/src/gestures/pointer_router.dart:136)
#12 PointerRouter.route (package:flutter/src/gestures/pointer_router.dart:122)
#13 GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:381)
#14 GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:361)
#15 RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:278)
#16 GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:316)
#17 GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:280)
#18 GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:238)
#19 GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:221)
#20 _rootRunUnary (dart:async/zone.dart:1370)
#21 _CustomZone.runUnary (dart:async/zone.dart:1265)
#22 _CustomZone.runUnaryGuarded (dart:async/zone.dart:1170)
#23 _invoke1 (dart:ui/hooks.dart:180)
#24 PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:276)
#25 _dispatchPointerDataPacket (dart:ui/hooks.dart:96)

I couldn't find a matching existing report, but it seems quite close to #43310 / #50574.

Unfortunately I have no good way to reproduce this but I'm seeing it while scrolling down a ListView of user generated content containing among lots of other things tapable images. So maybe it's related to a large tapable area being used to scroll down something it's contained within.

@darshankawar darshankawar added the in triage Presently being triaged by the triage team label May 10, 2021
@darshankawar
Copy link
Member

darshankawar commented May 10, 2021

@jhass
It'd be great if you could try to find a pattern and a reproducible code sample that shows this crash around below scenario you mentioned, that'll help us to properly address this issue.

but I'm seeing it while scrolling down a ListView of user generated content containing among lots of other things tapable images

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 10, 2021
@jhass
Copy link
Author

jhass commented May 10, 2021

Hi @darshankawar

as I outlined my app is essentially a ListView of user generated content, in particular markdown transformed to HTML and rendered using the flutter_html. Given this app is also a hobby project I cannot foresee being able to invest the substantial time it will take to find such a minimal reproducing example. Please don't make this a requirement to you fixing crashes in your product.

Do you imagine the fix is substantially different to turning the null assertion into a null check?

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 10, 2021
@nt4f04uNd
Copy link
Member

It looks like you are describing a bug in a third party package, flutter_html is not supported by Flutter team

@jhass
Copy link
Author

jhass commented May 10, 2021

Did you take a look at the stack trace? Where does it as much as touch third party code?

@darshankawar
Copy link
Member

It does look to me similar to #43310.

@jhass Can you try by switching to latest master and see if you get same error ?

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 11, 2021
@jhass
Copy link
Author

jhass commented May 11, 2021

It seems to be quite the race condition, I couldn't find reliable reproduction steps even on the known bad version :/

Do you see any negative consequences from turning that null assertion into a null check?

The other solution I see, if that instance variable is modified concurrently, is passing the value down the call chain as a parameter from the place it was last null checked.

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label May 11, 2021
@nt4f04uNd
Copy link
Member

Did you take a look at the stack trace? Where does it as much as touch third party code?

Sorry, I'm not quite sure I understood what you are asking.

This line of the stack trace you attached indicates that exception is thrown from flutter_html library
#2 MultipleTapGestureRecognizer.rejectGesture (package:flutter_html/src/utils.dart:54)

Do you see any negative consequences from turning that null assertion into a null check?

As far as I can tell, this assertion is there to ensure that gesture recognizer is properly used. Converting it into null check will hide potential errors like this and developers will be left unaware of potential unexpected behaviors

@jhass
Copy link
Author

jhass commented May 11, 2021

Let's dive into the code, MultipleTapGestureRecognizer is very simple:

https://github.com/Sub6Resources/flutter_html/blob/6cc6c37e0850691434f596f473f28c10df17cfc2/lib/src/utils.dart#L51-L56

  • It does not access any internal state
  • It does not call any private API
  • It does not inject any external state
  • It does not retain any state of its own

I'm sorry if there's a better term for the ! operator than null assertion, but let's be clear this assertion is in a private method of BaseTapGestureRecognizer accessing private internal state that nothing external can really influence:

void _checkDown() {
if (_sentTapDown) {
return;
}
handleTapDown(down: _down!);
_sentTapDown = true;
}

It's using ! before checking for null because it, and I believe wrongly so, assumes in this case the private nullable field _down cannot and not be null. It's not an explicit assert to guard against API misuse.

@nt4f04uNd
Copy link
Member

cc @dkwingsmt

@nt4f04uNd
Copy link
Member

nt4f04uNd commented May 11, 2021

I reproduced it

Code sample
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Home(),
    );
  }
}

class Home extends StatelessWidget {
  Home({Key key}) : super(key: key);

  final list = List.generate(1000, (index) => index);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: ListView.builder(
        itemCount: list.length,
        itemBuilder: (BuildContext context, int index) {
          return RawGestureDetector(
            gestures: {
              MultipleTapGestureRecognizer: GestureRecognizerFactoryWithHandlers<MultipleTapGestureRecognizer>(
                () => MultipleTapGestureRecognizer(),
                (MultipleTapGestureRecognizer instance) => instance
                ..onTap = () {
                  print('tap');
                },
              )
            },
            child: const Image(
              image: NetworkImage('https://flutter.github.io/assets-for-api-docs/assets/widgets/owl.jpg'),
            ),
          );
        },
      ),
    );
  }
}

class MultipleTapGestureRecognizer extends TapGestureRecognizer {
  @override
  void rejectGesture(int pointer) {
    acceptGesture(pointer);
  }
}
flutter doctor -v
[√] Flutter (Channel unknown, 2.3.0-2.0.pre.94, on Microsoft Windows [Version 10.0.19041.928], locale ru-RU)
    • Flutter version 2.3.0-2.0.pre.94 at C:\dev\src\flutter
    • Upstream repository unknown
    • Framework revision 3009bb3ffa (7 hours ago), 2021-05-11 14:11:03 +0300
    • Engine revision 1825befbc7
    • Dart version 2.14.0 (build 2.14.0-74.0.dev)

[√] Android toolchain - develop for Android devices (Android SDK version 30.0.2)     
    • Android SDK at C:\Users\danya\AppData\Local\Android\sdk
    • Platform android-30, build-tools 30.0.2
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)    
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2019 16.7.7)        
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community 
    • Visual Studio Community 2019 version 16.7.30621.155
    • Windows 10 SDK version 10.0.19041.0

[√] Android Studio (version 4.1.0)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)    

[√] IntelliJ IDEA Community Edition (version 2020.3)
    • IntelliJ at C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.3.3
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart

[√] VS Code (version 1.56.0)
    • VS Code at C:\Users\danya\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.22.0

[√] Connected device (4 available)
    • sdk gphone x86 (mobile) • emulator-5554 • android-x86    • Android 11 (API 30) (emulator)
    • Windows (desktop)       • windows       • windows-x64    • Microsoft Windows [Version 10.0.19041.928]
    • Chrome (web)            • chrome        • web-javascript • Google Chrome 90.0.4430.93
    • Edge (web)              • edge          • web-javascript • Microsoft Edge 90.0.818.46

• No issues found!

When assertions disabled, it throws the error from the OP
When assertions enabled, it throws this

══╡ EXCEPTION CAUGHT BY GESTURE LIBRARY ╞═══════════════════════════════════════════════════════════
The following assertion was thrown while dispatching a pointer event:
'package:flutter/src/gestures/tap.dart': Failed assertion: line 201 pos 14: '_down == null && _up ==
null': is not true.

Either the assertion indicates an error in the framework itself, or we should provide substantially
more information in this error message to help you determine and fix the underlying cause.
In either case, please report this assertion by filing a bug on GitHub:
  https://github.com/flutter/flutter/issues/new?template=2_bug.md

When the exception was thrown, this was the stack:
#2      BaseTapGestureRecognizer.addAllowedPointer (package:flutter/src/gestures/tap.dart:201:14)
#3      GestureRecognizer.addPointer (package:flutter/src/gestures/recognizer.dart:114:7)
#4      RawGestureDetectorState._handlePointerDown (package:flutter/src/widgets/gesture_detector.dart:1464:18)
#5      RenderPointerListener.handleEvent (package:flutter/src/rendering/proxy_box.dart:2848:29)
#6      GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:420:22)
#7      RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:287:11)
#8      GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:374:7)
#9      GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:338:5)
#10     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:296:7)
#11     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:279:7)
#15     _invoke1 (dart:ui/hooks.dart:182:10)
#16     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:282:7)
#17     _dispatchPointerDataPacket (dart:ui/hooks.dart:96:31)
(elided 5 frames from class _AssertionError and dart:async)

Event:
  PointerDownEvent#b1bba(position: Offset(145.8, 216.7))
Target:
  RenderPointerListener#1bb16 relayoutBoundary=up6
════════════════════════════════════════════════════════════════════════════════════════════════════

@jhass
Copy link
Author

jhass commented May 11, 2021

That's great, thank you!

@darshankawar
Copy link
Member

I see the same behavior as below:

console error log
Null check operator used on a null value

Stack trace:

#0 BaseTapGestureRecognizer._checkDown (package:flutter/src/gestures/tap.dart:287)
#1 BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:265)
#2 MultipleTapGestureRecognizer.rejectGesture (package:flutter_html/src/utils.dart:54)
#3 GestureArenaManager._resolve (package:flutter/src/gestures/arena.dart:214)
#4 GestureArenaEntry.resolve (package:flutter/src/gestures/arena.dart:53)
#5 OneSequenceGestureRecognizer.resolve (package:flutter/src/gestures/recognizer.dart:260)
#6 BaseTapGestureRecognizer.resolve (package:flutter/src/gestures/tap.dart:253)
#7 PrimaryPointerGestureRecognizer.handleEvent (package:flutter/src/gestures/recognizer.dart:472)
#8 PointerRouter._dispatch (package:flutter/src/gestures/pointer_router.dart:93)
#9 PointerRouter._dispatchEventToRoutes.<anonymous closure> (package:flutter/src/gestures/pointer_router.dart:138)
#10 _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:397)
#11 PointerRouter._dispatchEventToRoutes (package:flutter/src/gestures/pointer_router.dart:136)
#12 PointerRouter.route (package:flutter/src/gestures/pointer_router.dart:122)
#13 GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:381)
#14 GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:361)
#15 RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:278)
#16 GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:316)
#17 GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:280)
#18 GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:238)
#19 GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:221)
#20 _rootRunUnary (dart:async/zone.dart:1370)
#21 _CustomZone.runUnary (dart:async/zone.dart:1265)
#22 _CustomZone.runUnaryGuarded (dart:async/zone.dart:1170)
#23 _invoke1 (dart:ui/hooks.dart:180)
#24 PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:276)
#25 _dispatchPointerDataPacket (dart:ui/hooks.dart:96)



flutter doctor -v

[✓] Flutter (Channel master, 2.3.0-2.0.pre.139, on Mac OS X 10.15.4 19E2269
    darwin-x64, locale en-GB)
    • Flutter version 2.3.0-2.0.pre.139 at
      /Users/dhs/documents/fluttersdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision e621ebfb34 (3 hours ago), 2021-05-11 23:29:03 -0400
    • Engine revision 8e332453b8
    • Dart version 2.14.0 (build
      2.14.0-edge.b99466d472e369990eb22de332cb5b2967b3ea00)

[!] Xcode - develop for iOS and macOS
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 12.3, Build version 12C33
    ! CocoaPods 1.9.3 out of date (1.10.0 is recommended).
        CocoaPods is used to retrieve the iOS and macOS platform side's plugin
        code that responds to your plugin usage on the Dart side.
        Without CocoaPods, plugins will not work on iOS or macOS.
        For more info, see https://flutter.dev/platform-plugins
      To upgrade see
      https://guides.cocoapods.org/using/getting-started.html#installation for
      instructions.

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.55.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (3 available)
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729 • ios
      • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                               •
      web-javascript • Google Chrome 90.0.4430.212

! Doctor found issues in 1 category.

@darshankawar darshankawar added a: error message Error messages from the Flutter framework f: gestures flutter/packages/flutter/gestures repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.3 Found to occur in 2.3 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on c: crash Stack traces logged to the console and removed in triage Presently being triaged by the triage team labels May 12, 2021
@xu-baolin
Copy link
Member

Same issue #71331
There is no reason to override rejectGesture with acceptGesture, and we can't do this. This will destroy the architectural design of the flutter.
CC @dkwingsmt

@jhass
Copy link
Author

jhass commented May 13, 2021

Looks like the initial approach is from https://gist.github.com/Nash0x7E2/08acca529096d93f3df0f60f9c034056

This was introduced to flutter_html to fix another issue: Sub6Resources/flutter_html#631

Your "fix" in flutter_carousel_slider was just removing the workaround, restoring the original issue this wanted to fix. Nobody ever followed up with a proper way to do this: https://github.com/serenader2014/flutter_carousel_slider/blob/master/lib/carousel_slider.dart#L338

What is the proper way to do this? I hope just living with the broken behavior this wants to workaround is not your recommended solution.

@goderbauer
Copy link
Member

As @xu-baolin points out, this code is just not legal:

class MultipleTapGestureRecognizer extends TapGestureRecognizer {
  @override
  void rejectGesture(int pointer) {
    acceptGesture(pointer);
  }
}

Since the MultipleTapGestureRecognizer (which is causing this) is not part of the framework, I am going to close this issue here.

@dkwingsmt
Copy link
Contributor

TapGestureRecognizer is not meant to be used as a base class. If it works in this way, it's fine, and if the usage is added it into flutter/tests, we'll probably even keep compatibility for it. But if it is extended in an unintended way and caused crashes, that's probably because the extension is incorrect. Providing these methods does not mean using them in any order is legal.

Please file this bug to the package owner. (Or not, if you don't mean to file at all.)

We perform assertions against internal states all the time, especially for complex logic as gesture recognizers, and we encourage so. We have done too few assertions, not too many. Assertions ensure that premises are kept throughout the lifecycle of the object. In the situation of this ticket, _checkDown means "trigger the tap down callback if non null". It is called whenever the recognizer is confirmed to own this tap, which means this tap must have happened, which of course means there must have been a down event.

@github-actions
Copy link

github-actions bot commented Aug 1, 2021

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: error message Error messages from the Flutter framework c: crash Stack traces logged to the console f: gestures flutter/packages/flutter/gestures repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.3 Found to occur in 2.3 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on r: invalid Issue is closed as not valid
Projects
None yet
Development

No branches or pull requests

6 participants