Attempt to invoke virtual method 'void com.mapbox.services.android.navigation.ui.v5.MultiOnClickListener.clearListeners()' on a null object reference]

JFDionne commented 5 years ago

Android API: 21 Mapbox Navigation SDK version: 0.39.0

Steps to trigger behavior

I'm trying to use the NavigationView on a embedded layout and trying to "move" the layout to another contentview, but it crash has soon I remove the View from the parent with:

Attempt to invoke virtual method 'void com.mapbox.services.android.navigation.ui.v5.MultiOnClickListener.clearListeners()' on a null object reference]

I have a simple class with:

public class clsNavigationView extends CoordinatorLayout implements OnNavigationReadyCallback, MapboxMap.OnMapLongClickListener,
        NavigationListener, ProgressChangeListener, InstructionListListener, SpeechAnnouncementListener,
        BannerInstructionsListener, OnTrackingModeChangedListener
...
        inflate(getContext(), R.layout.mapbox_activity_embedded_navigation, this);
...

The layout have

<com.mapbox.services.android.navigation.ui.v5.NavigationView
android:id="@+id/navigationView"
android:layout_width="match_parent"
android:layout_height="match_parent"
app:navigationDarkTheme="@style/NavigationViewDark"
app:navigationLightTheme="@style/NavigationViewLight"/>

I put the clsNavigationView on a static FrameLayout, I add it to my mainView on another FrameLayout.

When I change view, I remove the view with:

((FrameLayout) objFrameMap.getParent()).removeView(objFrameMap);

Do my new setContentview on my main Activity.

Then I add back my Frame into my new FrameLayout on the new Content

((FrameLayout) findViewById(R.id.layer_mapview)).addView(objFrameMap);

Actual behavior

It crash at the removeView.

** I'm not using fragment.

Fix?

I tested with the source, I just add a check if multiOnClickListener before doing the clear on 3 classes (RecenterButton, FeedbackButton and SoundButton), it's seem to be fine after this change.

  private void clearListeners() {
    multiOnClickListener.clearListeners();
    multiOnClickListener = null;
    setOnClickListener(null);
  }

to

  private void clearListeners() {
    if (multiOnClickListener != null) multiOnClickListener.clearListeners();
    multiOnClickListener = null;
    setOnClickListener(null);
  }

JFDionne commented 5 years ago

I don't think I'm fixing it correctly.

The problem seem to be inside the clearListeners, you do a clearListeners and put multiOnClickListener to null;

But on the setupOnClickListeners you don't recreate it.

So I add the create object on it:

  private void setupOnClickListeners() {
    if (multiOnClickListener == null)
      multiOnClickListener = new MultiOnClickListener();
    soundFab.setOnClickListener(multiOnClickListener);
  }

Seem to work fine, each time it detach is clear and when Attach it's create, but we loose the onClickListener.

JFDionne commented 5 years ago

I think the "best way", for now, is to comment clearListeners() on the onDetachedFromWindow. But I don't think it's the best option, I'll wait for a reply of someone, for now I'm the only one who speaks here :)

gajdzik commented 5 years ago

Has anyone investigated it?

andrlee commented 5 years ago

@JFDionne @gajdzik looking into it - stay tuned.

Guardiola31337 commented 5 years ago

Hey @JFDionne 👋 thanks for reaching out and for the detailed report.

The problem seem to be inside the clearListeners, you do a clearListeners and put multiOnClickListener to null;

As this is correct, I'm wondering why onDetachedFromWindow (clearListeners()) is called twice in your use case - first making it null and second causing the crash

Attempt to invoke virtual method 'void com.mapbox.services.android.navigation.ui.v5.MultiOnClickListener.clearListeners()' on a null object reference]

Supposedly the views should be clean up and recreated afterwards as part of the lifecycle, i.e. MultiOnClickListener shouldn't be null (unless something else is making an unexpected call to onDetachedFromWindow). Wondering what lifecycle methods are called in your scenario and in what order. Could you debug this from your side and report back here when you have a chance?

Also it'd be really helpful if you can provide a sample code / project reproducing the issue. That'll give us better insight into your implementation and what could be causing your problem. Are you able to reproduce that behavior in any of the examples of the test app? That'd be also really helpful to troubleshoot the issue described.

andrlee commented 5 years ago

hey @JFDionne any updates?

JFDionne commented 5 years ago

Hi, sorry I'm outside right now, I'll be able to check it in a few days, but it was doing the same thing with the test app.

Guardiola31337 commented 5 years ago

Hey @JFDionne thanks for the update.

Wondering what lifecycle methods are called in your scenario and in what order.

Would you be able to debug this from your side and report back here when you have a chance?

Also you mentioned that it was doing the same thing with the test app, could you specify what example you're using and what changes you made so we can reproduce locally? That'd be 💯

Thanks again!

JFDionne commented 5 years ago

@Guardiola31337 Hi, sorry for the delay, I just create a little solution with the problem, I reduce my stuff at the maximum for making a test project.

https://github.com/JFDionne/Test_Mapbox

If you try to click on the bottom layout the map go there, but if you try to go upper again, it crash with:

   Process: com.example.test_mapbox, PID: 31929
    java.lang.NullPointerException: Attempt to invoke virtual method 'void com.mapbox.services.android.navigation.ui.v5.MultiOnClickListener.clearListeners()' on a null object reference
        at com.mapbox.services.android.navigation.ui.v5.RecenterButton.clearListeners(RecenterButton.java:112)
        at com.mapbox.services.android.navigation.ui.v5.RecenterButton.onDetachedFromWindow(RecenterButton.java:104)
        at android.view.View.dispatchDetachedFromWindow(View.java:16747)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3429)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3421)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3421)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3421)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3421)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3421)
        at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4959)
        at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4933)
        at android.view.ViewGroup.removeView(ViewGroup.java:4864)
        at com.example.test_mapbox.MainActivity$1.onClick(MainActivity.java:69)
        at android.view.View.performClick(View.java:6257)
        at android.view.View$PerformClick.run(View.java:23705)
        at android.os.Handler.handleCallback(Handler.java:751)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:154)
        at android.app.ActivityThread.main(ActivityThread.java:6780)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1496)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1386)

JunDai commented 4 years ago

@JFDionne - very appreciate your test app. I could reproduce it and I will follow up with the team. Will update here if any progress.

JunDai commented 4 years ago

@Guardiola31337 - Upload the screen recording to show the use case: as @JFDionne mentioned before, he was trying to move a navigation view between two containers. Since the navigationView is same, so it's just been added/removed from the container view. That's why you mentioned the onDetachedFromWindow executed twice and the second time caused the NPE crash.

ezgif com-video-to-gif

sachinshettigar commented 4 years ago

any update on this? Is this fixed?

JunDai commented 4 years ago

@JFDionne / @sachinshettigar : thanks for checking out our UI SDK.

The current implementation doesn't support this use case. We will keep this issue in mind and will consider it in future. But now, we don't have a plan.

Guardiola31337 commented 4 years ago

@asinghal22 is this something we want to support in 1.0?

cc @abhishek1508 @JunDai

JunDai commented 4 years ago

When I use @JFDionne 's example code to test our V1.0 latest code today, I got another crash from NN. It seems this use case isn't supported by NN in Navigation SDK V1.0?

2020-07-27 14:31:02.341 10565-10973/com.mapbox.navigation.examples E/AndroidRuntime: FATAL EXCEPTION: pool-1-thread-1
    Process: com.mapbox.navigation.examples, PID: 10565
    java.lang.Error: Any tile edge can only be used once as part of the geometry
        at com.mapbox.navigator.Navigator.getRouteBufferGeoJson(Native Method)
        at com.mapbox.navigation.navigator.internal.MapboxNativeNavigatorImpl.setRoute(MapboxNativeNavigatorImpl.kt:140)
        at com.mapbox.navigation.navigator.internal.MapboxNativeNavigator$DefaultImpls.setRoute$default(MapboxNativeNavigator.kt:84)
        at com.mapbox.navigation.core.trip.session.MapboxTripSession$route$1.invokeSuspend(MapboxTripSession.kt:75)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:919)

JunDai commented 4 years ago

@asinghal22 - we might defer this issue to after v1.0. but feel free to reprioritize it.

zugaldia commented 4 years ago

When I use @JFDionne 's example code to test our V1.0 latest code today, I got another crash from NN. It seems this use case isn't supported by NN in Navigation SDK V1.0?

@JunDai I've reopened https://github.com/mapbox/mapbox-navigation-android/issues/2337 to track that Exception.

abhishek1508 commented 2 years ago

Thanks for using the Mapbox Navigation SDK for Android and being a valued customer.

Mapbox will be soon deprecating any support for v0 and v1 versions of the SDK. To facilitate this transition we’re launching a new drop-in UI component into v2, equivalent to the existing NavigationView v1 in its design goals, however with a more modern and customizable API.

We plan to launch this new drop-in UI component as a Developer Preview feature in April, as part of the v2.5 series. Since you are using NavigationView with v1, we’d love to hear your feedback so that we can incorporate it ahead of a GA release.

If you’re interested in having early access to the upcoming drop-in UI for v2 and its documentation, drop a comment on this ticket or send an email to abhishek.kejriwal@mapbox.com

/cc @zugaldia @AhmerKhan1

mapbox / mapbox-navigation-android