Crash when disconnecting from headunit when navigation is started [181143772]

Fixed

Bug

Status Update

No update yet.

Description

kr...@tomtom.com

created issue #1

Feb 25, 2021 11:23AM

Library version used: 1.0.0-beta01

Devices/Android versions reproduced on: Any

Android Auto version reproduced on: 6.1.6105

Crash when disconnecting from headunit when navigation is started. Simply start the app, start navigating, disconnect/close dhu. After Android Auto service gets destroyed - this is where we clear NavigationManager callback, this crash occurs.

    java.lang.NullPointerException
        at java.util.Objects.requireNonNull(Objects.java:220)
        at androidx.car.app.navigation.NavigationManager.lambda$onStopNavigation$3$NavigationManager(NavigationManager.java:285)
        at androidx.car.app.navigation.-$$Lambda$NavigationManager$VfCV3KNUFOmGR8cBjFUUBVJugqQ.run(Unknown Source:2)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.app.ActivityThread.main(ActivityThread.java:8456)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:596)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1130)

It seems that NavigationManager tries to call onStopNavigation in the callback even though the service is no longer created.

Comments

kr...@tomtom.com <kr...@tomtom.com> #2Mar 1, 2021 12:49PM

It seems that there was an executor added that cause call rebound when service is being unbound.

https://github.com/androidx/androidx/commit/e93987222bd963dce24fe07f0b338235b2831a17#diff-91442da2be2c68325de87b2370642ab5267190d366f45637261fac1ffa965dea

When service is being unbound at https://github.com/androidx/androidx/blob/7c3bc7636eaf58c7ea050f79c01bbc23ada80de6/car/app/app/src/main/java/androidx/car/app/CarAppService.java#L145

 runOnMain(() -> {
            // Destroy the session
            if (mCurrentSession != null) {
                CarContext carContext = mCurrentSession.getCarContext();

                // Stop any active navigation
                carContext.getCarService(NavigationManager.class).onStopNavigation();

                // Destroy all screens in the stack
                carContext.getCarService(ScreenManager.class).destroyAndClearScreenStack();

                // Remove binders to the host
                carContext.resetHosts();

                ((LifecycleRegistry) mCurrentSession.getLifecycle()).handleLifecycleEvent(
                        Event.ON_DESTROY);
            }
            mCurrentSession = null;
        });

Due to running via an additional executor in https://github.com/androidx/androidx/blob/7c3bc7636eaf58c7ea050f79c01bbc23ada80de6/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java#L274 the call arrives after service is already destroyed. Is there any reason to have this executor there? The call seems to be already happening on main thread and will crash due to first line assert if it doesn't.

li...@tomtom.com <li...@tomtom.com> #3Mar 8, 2021 11:02AM

We are seeing similar crash. See logs

Objects.java:220 java.util.Objects.requireNonNull
NavigationManager.java:285 androidx.car.app.navigation.NavigationManager.lambda$onStopNavigation$3$NavigationManager
Unknown:2 androidx.car.app.navigation.-$$Lambda$NavigationManager$zqLKz70Zaz8Uc9Ya7K69PxjfMYg.run
Handler.java:938 android.os.Handler.handleCallback
Handler.java:99 android.os.Handler.dispatchMessage
Looper.java:223 android.os.Looper.loop
ActivityThread.java:7660 android.app.ActivityThread.main
Method.java:-2 java.lang.reflect.Method.invoke
RuntimeInit.java:592 com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run
ZygoteInit.java:947 com.android.internal.os.ZygoteInit.main

li...@google.com <li...@google.com> Mar 9, 2021 05:00PM

Assigned to li...@google.com.

li...@google.com <li...@google.com> #4Mar 9, 2021 09:05PM

Thanks for filing this bug, we are looking into fixing it in the next release

li...@google.com <li...@google.com> Mar 10, 2021 12:37AM

Accepted by li...@google.com.

li...@google.com <li...@google.com> #5Mar 10, 2021 06:50PM

Marked as fixed.

This is now fixed and should be available in a following release of the AndroidX library

ap...@google.com <ap...@google.com> #6Mar 10, 2021 06:51PM

Project: platform/frameworks/support
Branch: androidx-main

commit 507107531e87d9543f74b998b23ee6bdd9fdbd4c
Author: Rafael Lima <therafael@google.com>
Date: Tue Mar 09 18:46:21 2021

Fixed a `NullPointerException` that was thrown when the app receives a `stopNavigation` call when it has already removed a callback

Relnote: Fixed a `NullPointerException` that was thrown when the app receives a `stopNavigation` call when it has already removed a callback
Bug: 181143772
Test: Unit Test
Change-Id: Ib8b89f43bd5891ef9f8b53944b5ef00979d8e607

M car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
M car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java

https://android-review.googlesource.com/1624421

kr...@tomtom.com <kr...@tomtom.com> #7Mar 11, 2021 08:55AM

As per comment in https://android-review.googlesource.com/c/platform/frameworks/support/+/1624421/4#message-5aefce71146b7665bc24aaa2bc5ca2d7dc0f6b83

The deferred nature of this callback may still cause issues to car library clients like us.

li...@google.com <li...@google.com> Mar 17, 2021 08:41PM

Status: Assigned (reopened)

li...@google.com <li...@google.com> #8Mar 17, 2021 09:44PM

Thanks for pointing this out, working on a fix now

ap...@google.com <ap...@google.com> #9Mar 17, 2021 11:12PM

Project: platform/frameworks/support
Branch: androidx-main

commit 19a4e804b650056de0ecb54a2470e98fcb076727
Author: Rafael Lima <therafael@google.com>
Date: Wed Mar 17 21:46:05 2021

Do not callback `NavigationManagerCallback#onStopNavigation` if the callback is cleared before the executor executes

Relnote: Do not callback `NavigationManagerCallback#onStopNavigation` if the callback is cleared before the executor executes
Bug: 181143772
Test: Unit tests
Change-Id: I7fc5ebe1f371b0530f084f76236ab058f71f2a5c

M car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
M car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java

https://android-review.googlesource.com/1643642

li...@google.com <li...@google.com> Mar 22, 2021 05:09PM

Marked as fixed.

kr...@tomtom.com <kr...@tomtom.com> #10Mar 25, 2021 09:18AM

Thanks a lot for fixing it! Is there any ETA for new library release?

sh...@google.com <sh...@google.com> #11Mar 25, 2021 05:58PM

This should be part of the now-released rc01 https://developer.android.com/jetpack/androidx/releases/car-app#1.0.0-rc01