mapbox / mapbox-maps-android

Interactive, thoroughly customizable maps in native Android powered by vector tiles and OpenGL.
https://www.mapbox.com/mobile-maps-sdk
Other
466 stars 131 forks source link

Crash in StandardGestureListener.onFling #1019

Closed florianPOLARSTEPS closed 1 year ago

florianPOLARSTEPS commented 2 years ago

Environment

Observed behavior and steps to reproduce

I am not able to reproduce this issue but it seems to be an issue with the inherited nullability of MotionEvent fields from the Android framework. (android.view.GestureDetector.GestureListener)

It may be wise to assume that all MotionEvents in the GestureDetector.OnGestureListener callbacks can be null.

Fatal Exception: java.lang.NullPointerException: Parameter specified as non-null is null: method com.mapbox.maps.plugin.gestures.GesturesPluginImpl$StandardGestureListener.onFling, parameter e1
       at com.mapbox.maps.plugin.gestures.GesturesPluginImpl$StandardGestureListener.onFling(GesturesPluginImpl.kt:2)
       at com.mapbox.android.gestures.StandardGestureDetector$1.onFling(StandardGestureDetector.java:56)
       at android.view.GestureDetector.onTouchEvent(GestureDetector.java:672)
       at androidx.core.view.GestureDetectorCompat$GestureDetectorCompatImplJellybeanMr2.onTouchEvent(GestureDetectorCompat.java:480)
       at androidx.core.view.GestureDetectorCompat.onTouchEvent(GestureDetectorCompat.java:543)
       at com.mapbox.android.gestures.StandardGestureDetector.analyzeEvent(StandardGestureDetector.java:147)
       at com.mapbox.maps.plugin.gestures.GesturesPluginImpl.<init>(GesturesPluginImpl.kt:61)
       at com.mapbox.android.gestures.BaseGesture.analyze(BaseGesture.java:61)
       at com.mapbox.android.gestures.BaseGesture.onTouchEvent(BaseGesture.java:39)
       at com.mapbox.android.gestures.AndroidGesturesManager.onTouchEvent(AndroidGesturesManager.java:193)
       at com.mapbox.maps.plugin.gestures.GesturesPluginImpl.onTouchEvent(GesturesPluginImpl.kt:268)
       at com.mapbox.maps.plugin.MapPluginRegistry.onTouch(MapPluginRegistry.kt:128)
       at com.mapbox.maps.MapController.onTouchEvent(MapController.kt:167)
       at com.mapbox.maps.MapView.onTouchEvent(MapView.kt:289)
       at android.view.View.dispatchTouchEvent(View.java:12543)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3153)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2829)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3159)
       at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2844)
       at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:601)
       at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1871)
       at android.app.Activity.dispatchTouchEvent(Activity.java:3384)
       at androidx.appcompat.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:69)
       at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:563)
       at android.view.View.dispatchPointerEvent(View.java:12791)
       at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:5675)
       at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:5470)
       at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4963)
       at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:5016)
       at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4982)
       at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:5119)
       at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4990)
       at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:5176)
       at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4963)
       at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:5016)
       at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4982)
       at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4990)
       at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4963)
       at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:7741)
       at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:7681)
       at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:7642)
       at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:7852)
       at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:197)
       at android.view.InputEventReceiver.nativeConsumeBatchedInputEvents(InputEventReceiver.java)
       at android.view.InputEventReceiver.consumeBatchedInputEvents(InputEventReceiver.java:186)
       at android.view.ViewRootImpl.doConsumeBatchedInput(ViewRootImpl.java:7815)
       at android.view.ViewRootImpl$ConsumeBatchedInputRunnable.run(ViewRootImpl.java:7879)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:911)
       at android.view.Choreographer.doCallbacks(Choreographer.java:723)
       at android.view.Choreographer.doFrame(Choreographer.java:652)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:897)
       at android.os.Handler.handleCallback(Handler.java:789)
       at android.os.Handler.dispatchMessage(Handler.java:98)
       at android.os.Looper.loop(Looper.java:164)
       at android.app.ActivityThread.main(ActivityThread.java:6944)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)

Expected behavior

Does not crash when the user interacts with the map

Notes / preliminary analysis

Additional links and references

kiryldz commented 2 years ago

@florianPOLARSTEPS could you please provide full code snippet that results in this crash?

florianPOLARSTEPS commented 2 years ago

@kiryldz Unfortunately I am not able to pinpoint the exact location of the crash since it seems to happen on the animation thread somewhere after a flyTo animation was started. (of which I have plenty). There is no entry of mine in the stack-trace.

The cause of the issue seems logical though... to me. In the mapbox-sdk (StandardGestureListener.onFling) fields are presumed NonNull which coming from the Android SDK are not annotated as such. In which case a MotionEvent can be null, I honestly do not know... Probably some sort of edge-case, but nevertheless I think it should be handled as Nullable.

florianPOLARSTEPS commented 2 years ago

What I forgot to mention is that it is happening rather rarely for my app, so it has not a big impact at the moment. Which again means it is some sort of edgecase/ race-condition.

kiryldz commented 2 years ago

Can you at least describe your use-case more precisely? If I understood correctly you call flyTo and sometimes it does crash? I assume you interrupt some camera flies by gestures - is it correct?

florianPOLARSTEPS commented 2 years ago

@kiryldz Like mentioned initially, I am unable to reproduce this issue... I am seeing it happening to users in my reporting (crashlytics) only. But to answer your question... Yes, the user can interrupt a flyTo animation with a gesture.

florianPOLARSTEPS commented 2 years ago

I am not sure why this fix is taking so long. In my view it's a clear bug in the mapbox sdk. If you imply a value to be non-null in kotlin and it can be null in the java world it will crash. Simple as that.

The cause of the issue seems logical though... to me. In the mapbox-sdk (StandardGestureListener.onFling) fields are presumed NonNull which coming from the Android SDK are not annotated as such. In which case a MotionEvent can be null, I honestly do not know... Probably some sort of edge-case, but nevertheless I think it should be handled as Nullable.

alexbranti commented 2 years ago

I'm also observing this crash on Mapbox 10.7.0

yunikkk commented 2 years ago

@alexbranti @florianPOLARSTEPS the issue is fixed in the downstream lib here and will be supposedly released in 10.9

yunikkk commented 1 year ago

@alexbranti @florianPOLARSTEPS 10.9.0 has the updated gestures dependency, this issues should be resolved.