search

mapbox / mapbox-navigation-android

Mapbox Navigation SDK for Android
https://docs.mapbox.com/android/navigation/overview/
Other
622 stars 319 forks source link

Crash: Attempt to invoke virtual method 'void com.mapbox.navigation.ui.MultiOnClickListener.clearListeners()' on a null object reference #3901

Open NathanaelA opened 3 years ago

NathanaelA commented 3 years ago

Android API: Mapbox Navigation SDK version: 9.6.0 and Navigator 1.1.0 & 1.3.0

Steps to trigger behavior

  1. Basically I'm using the sample from here https://docs.mapbox.com/android/navigation/guides/install/ (however it is converted into a different language).
  2. Pause/Switch Application

Expected behavior

No Crash

Actual behavior

Crash

Appears to be like #1971, however the stack trace appears to be slightly different.

This occurs when I click the Switch application button (i.e. Square Nav Button); the app throws this error. I am logging all the events; and I see: The Pause & Stop events in sequence, which I then chain to the MapBox Navigator onPause, onStop calls exactly like the demo. However about 1/2 a second after; I get the below stack trace crash.... I've also tried not sending the onStop, onPause, etc... No change...

Based on the callstack below; I suspect the issue is something has already called: https://github.com/mapbox/mapbox-navigation-android/blob/master/libnavigation-ui/src/main/java/com/mapbox/navigation/ui/RecenterButton.java#L142-L146

And https://github.com/mapbox/mapbox-navigation-android/blob/master/libnavigation-ui/src/main/java/com/mapbox/navigation/ui/RecenterButton.java#L144 This line I believe is actually invalid, as nothing in the code will re-create the MultiOnClickListener again in this element. as it is only created during creation of it, so if this element is re-attached it will misbehave and crash later: https://github.com/mapbox/mapbox-navigation-android/blob/master/libnavigation-ui/src/main/java/com/mapbox/navigation/ui/RecenterButton.java#L29

According to https://stackoverflow.com/a/65227045 & https://proandroiddev.com/make-your-custom-view-lifecycle-aware-its-a-piece-of-cake-90b7c0498686 it appears onDetachFromWindow is NOT the place to cleanup/null things like this, as the view can be re-attached without being re-created if used in things like tabs/pagers. So nulling the listener will lead to a crash if it is re-attached. In addition it might be good if you checked to verify that the MultiOnClickListener isn't null in the clearListeners function on line 143 as something obviously called this function a second time...

System.err: Attempt to invoke virtual method 'void com.mapbox.navigation.ui.MultiOnClickListener.clearListeners()' on a null object reference
System.err: 
System.err: StackTrace:
System.err: java.lang.NullPointerException: Attempt to invoke virtual method 'void com.mapbox.navigation.ui.MultiOnClickListener.clearListeners()' on a null object reference
System.err:     at com.mapbox.navigation.ui.RecenterButton.clearListeners(RecenterButton.java:143)
System.err:     at com.mapbox.navigation.ui.RecenterButton.onDetachedFromWindow(RecenterButton.java:135)
System.err:     at android.view.View.dispatchDetachedFromWindow(View.java:19607)
System.err:     at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3814)
System.err:     at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3806)
System.err:     at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3806)
System.err:     at android.view.ViewGroup.removeViewInternal(ViewGroup.java:5431)
System.err:     at android.view.ViewGroup.removeViewInternal(ViewGroup.java:5402)
System.err:     at android.view.ViewGroup.removeView(ViewGroup.java:5333)
System.err:     at android.view.ViewOverlay$OverlayViewGroup.remove(ViewOverlay.java:216)
System.err:     at android.view.ViewGroupOverlay.remove(ViewGroupOverlay.java:84)
System.err:     at androidx.transition.ViewGroupOverlayApi18.remove(ViewGroupOverlayApi18.java:53)
System.err:     at androidx.transition.Visibility$1.onTransitionEnd(Visibility.java:457)
System.err:     at androidx.transition.Transition.end(Transition.java:1965)
System.err:     at androidx.transition.Transition$3.onAnimationEnd(Transition.java:1914)
System.err:     at android.animation.Animator$AnimatorListener.onAnimationEnd(Animator.java:554)
System.err:     at android.animation.AnimatorSet.endAnimation(AnimatorSet.java:1301)
System.err:     at android.animation.AnimatorSet.doAnimationFrame(AnimatorSet.java:1086)
System.err:     at android.animation.AnimationHandler.doAnimationFrame(AnimationHandler.java:146)
System.err:     at android.animation.AnimationHandler.access$100(AnimationHandler.java:37)
System.err:     at android.animation.AnimationHandler$1.doFrame(AnimationHandler.java:54)
System.err:     at android.view.Choreographer$CallbackRecord.run(Choreographer.java:964)
System.err:     at android.view.Choreographer.doCallbacks(Choreographer.java:790)
System.err:     at android.view.Choreographer.doFrame(Choreographer.java:721)
System.err:     at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:951)
System.err:     at android.os.Handler.handleCallback(Handler.java:883)
System.err:     at android.os.Handler.dispatchMessage(Handler.java:100)
System.err:     at android.os.Looper.loop(Looper.java:214)
System.err:     at android.app.ActivityThread.main(ActivityThread.java:7356)
System.err:     at java.lang.reflect.Method.invoke(Native Method)
System.err:     at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
System.err:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
NathanaelA commented 3 years ago

I have modified the code for both the Sound & Recenter button to be:

  1. changed the multiOnClickListener to be a final in the definition

  2. Modified this

    private void clearListeners() {
setOnClickListener(null);
}

public void onDestroy() {
this.clearListeners();
multiOnClickListener.clearListeners();
}
  3. Called onDestroy for both buttons from the NativigationView shutdown function...

Would you like a PR for this, this seems to fix my issues with at least this problem?

This allows the Object to be detached and re-attached to the view w/o it crashing. Because we don't clear the listeners, but just setOnClickeListener = null basically it allows everything to continue working when it is re-attached. Once the button is being destroyed, then we actually clear the listeners in the multiOnClickListener as they will no longer be valid...