Fixed
Status Update
Comments
jo...@google.com <jo...@google.com>
li...@tomtom.com <li...@tomtom.com> #2
We now have made sure that in all implementation of getTemplate(), XXXTemplateBuilder.build() is returned; it will never explicitly returning a null. However, this crash is still happening to our alpha/beta users.
We don't have reproduce steps.
We don't have reproduce steps.
jo...@google.com <jo...@google.com> #3
getTemplate is annotated as @NonNull but unfortunately, this is not currently reflected in the reference documentation. We are working to fix that.
Rafa, can you please look at the log and see if there's anything else that we may be missing. We should not be crashing in this case, instead the error template should be displayed. If that's already happening, I don't believe there's anything to fix in the library beyond the documentation.
Rafa, can you please look at the log and see if there's anything else that we may be missing. We should not be crashing in this case, instead the error template should be displayed. If that's already happening, I don't believe there's anything to fix in the library beyond the documentation.
li...@tomtom.com <li...@tomtom.com> #4
Hi, thank you very much for your response, but, please investigate this issue a bit further.
It is entirely my speculation that the cause of this crash is that getTemplate() could be returning null sometimes.
I would like to repeat what I said in the previous comment, we have now made sure that getTemplate() will never return null from our code, it is always "return XXXTemplateBuilder.build();", however, this is crash still happening a lot to our beta users. I'll provide with more logs on Monday.
It is entirely my speculation that the cause of this crash is that getTemplate() could be returning null sometimes.
I would like to repeat what I said in the previous comment, we have now made sure that getTemplate() will never return null from our code, it is always "return XXXTemplateBuilder.build();", however, this is crash still happening a lot to our beta users. I'll provide with more logs on Monday.
li...@tomtom.com <li...@tomtom.com> #5
Hi,
I have attached new logs for your reference.
I have attached new logs for your reference.
jo...@google.com <jo...@google.com> #6
Thank you for the new log, we'll look at it soon and get back to you.
li...@google.com <li...@google.com> #7
I've been looking at this, the way for this null pointer to happen is if the value returned from getTemplate is null. getTemplate is abstract, so any implementation of it is done in the app being built. Is there perhaps some codepath that is still returning null in some specific situation? Perhaps are those beta testers using the latest build where you removed all null returns?
li...@tomtom.com <li...@tomtom.com> #8
Hi, yes, we are still seeing this crash happening to users using the new version where we have removed all the explicate return of "null" while using getTemplate().
All our use case of getTemplate() is now returning XXXBuilder.build().
We are not doing a null check on XXXBuilder.build(), assuming it will never be null. Could that be null sometimes? Maybe we will add a null check before returning XXXBuilder.build() and returning an placeholderTemplate with error message when that happens.
All our use case of getTemplate() is now returning XXXBuilder.build().
We are not doing a null check on XXXBuilder.build(), assuming it will never be null. Could that be null sometimes? Maybe we will add a null check before returning XXXBuilder.build() and returning an placeholderTemplate with error message when that happens.
li...@google.com <li...@google.com>
li...@tomtom.com <li...@tomtom.com> #9
Some more logs from yesterday if there is any help.
li...@google.com <li...@google.com>
li...@google.com <li...@google.com> #10
A couple of follow up questions:
- Can you repro this bug locally?
- Does this happen on a specific Screen of yours, or on some, many, all?
li...@tomtom.com <li...@tomtom.com> #11
Hi,
Can you repro this bug locally?
> We don't have reproduce steps locally for this. We will let you know if we can manage to reproduce it consistently locally.
Does this happen on a specific Screen of yours, or on some, many, all?
> We don't know which screen this happens. We do see this crash happen very frequently among our beta users.
We use builder.build() in all the getTemplate() methods, can builder.build() ever return null? Do we need to do null check there and return a special screen?
In our code, we have "getLifecycle().addObserver(this)" in all the screen constructors, like it is used in the sample apps, but, not doing removeObserver(this) anywhere. Could this be a problem? What is the best practice here? Is it necessary to do removeObserver? Perhaps in finalize()?
Are there any best practice on how the screen should be used that we perhaps missed?
Hope you have received the code Snippet I sent to N, if not, I can send to you directly.
Cheers!
Can you repro this bug locally?
> We don't have reproduce steps locally for this. We will let you know if we can manage to reproduce it consistently locally.
Does this happen on a specific Screen of yours, or on some, many, all?
> We don't know which screen this happens. We do see this crash happen very frequently among our beta users.
We use builder.build() in all the getTemplate() methods, can builder.build() ever return null? Do we need to do null check there and return a special screen?
In our code, we have "getLifecycle().addObserver(this)" in all the screen constructors, like it is used in the sample apps, but, not doing removeObserver(this) anywhere. Could this be a problem? What is the best practice here? Is it necessary to do removeObserver? Perhaps in finalize()?
Are there any best practice on how the screen should be used that we perhaps missed?
Hope you have received the code Snippet I sent to N, if not, I can send to you directly.
Cheers!
li...@tomtom.com <li...@tomtom.com> #12
Maybe a bit naive here, but, is it possibly to just do a null pointer check in your code?
Template template = XXX.getTemplate();
if (template !=null) {
template.getClass()
} else {
Log.E(TAG, "template should not be null")
}
Template template = XXX.getTemplate();
if (template !=null) {
template.getClass()
} else {
Log.E(TAG, "template should not be null")
}
li...@tomtom.com <li...@tomtom.com> #13
li...@google.com <li...@google.com> #14
As per our offline conversation, this seems to be a race condition in the shutdown of both processes. I'll look into mitigating this in a future release
li...@google.com <li...@google.com>
ap...@google.com <ap...@google.com> #15
Project: platform/frameworks/support
Branch: androidx-main
commit 076857f2f01e8e30f8186cad44ce22192438073a
Author: Rafael Lima <therafael@google.com>
Date: Thu Feb 25 23:28:48 2021
Improvements to shutdown scenarios
1. Do not send calls to app level callbacks if the app is not at least
in created state
2. Do not crash the app if the host is dead while trying to send an IPC
Follow up review will stop dispatching calls from template related
callbacks (i.e. onClick, etc)
Relnote: "Improvements to not dispatch calls to app if its lifecycle is
not at least in a created state"
Test: Unit tests
Bug:180530156
Bug:179800224
Bug:177921120
Bug:178516889
Change-Id: I8696503d1a9859411c4a52fa73d2852c8b3383d9
M car/app/app-testing/src/main/java/androidx/car/app/testing/TestAppManager.java
M car/app/app-testing/src/main/java/androidx/car/app/testing/TestCarContext.java
M car/app/app-testing/src/main/java/androidx/car/app/testing/navigation/TestNavigationManager.java
M car/app/app/src/main/java/androidx/car/app/AppManager.java
M car/app/app/src/main/java/androidx/car/app/CarAppService.java
M car/app/app/src/main/java/androidx/car/app/CarContext.java
M car/app/app/src/main/java/androidx/car/app/HostDispatcher.java
M car/app/app/src/main/java/androidx/car/app/ScreenManager.java
M car/app/app/src/main/java/androidx/car/app/model/OnCheckedChangeDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnClickDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnItemVisibilityChangedDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnSelectedDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/SearchCallbackDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
M car/app/app/src/main/java/androidx/car/app/utils/LogTags.java
M car/app/app/src/main/java/androidx/car/app/utils/RemoteUtils.java
M car/app/app/src/test/java/androidx/car/app/AppManagerTest.java
M car/app/app/src/test/java/androidx/car/app/CarAppServiceTest.java
M car/app/app/src/test/java/androidx/car/app/CarContextTest.java
M car/app/app/src/test/java/androidx/car/app/HostDispatcherTest.java
M car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java
https://android-review.googlesource.com/1607842
Branch: androidx-main
commit 076857f2f01e8e30f8186cad44ce22192438073a
Author: Rafael Lima <therafael@google.com>
Date: Thu Feb 25 23:28:48 2021
Improvements to shutdown scenarios
1. Do not send calls to app level callbacks if the app is not at least
in created state
2. Do not crash the app if the host is dead while trying to send an IPC
Follow up review will stop dispatching calls from template related
callbacks (i.e. onClick, etc)
Relnote: "Improvements to not dispatch calls to app if its lifecycle is
not at least in a created state"
Test: Unit tests
Bug:180530156
Bug:179800224
Bug:177921120
Bug:178516889
Change-Id: I8696503d1a9859411c4a52fa73d2852c8b3383d9
M car/app/app-testing/src/main/java/androidx/car/app/testing/TestAppManager.java
M car/app/app-testing/src/main/java/androidx/car/app/testing/TestCarContext.java
M car/app/app-testing/src/main/java/androidx/car/app/testing/navigation/TestNavigationManager.java
M car/app/app/src/main/java/androidx/car/app/AppManager.java
M car/app/app/src/main/java/androidx/car/app/CarAppService.java
M car/app/app/src/main/java/androidx/car/app/CarContext.java
M car/app/app/src/main/java/androidx/car/app/HostDispatcher.java
M car/app/app/src/main/java/androidx/car/app/ScreenManager.java
M car/app/app/src/main/java/androidx/car/app/model/OnCheckedChangeDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnClickDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnItemVisibilityChangedDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/OnSelectedDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/model/SearchCallbackDelegateImpl.java
M car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
M car/app/app/src/main/java/androidx/car/app/utils/LogTags.java
M car/app/app/src/main/java/androidx/car/app/utils/RemoteUtils.java
M car/app/app/src/test/java/androidx/car/app/AppManagerTest.java
M car/app/app/src/test/java/androidx/car/app/CarAppServiceTest.java
M car/app/app/src/test/java/androidx/car/app/CarContextTest.java
M car/app/app/src/test/java/androidx/car/app/HostDispatcherTest.java
M car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java
li...@google.com <li...@google.com> #16
We have fixed this issue now and it will be available in a following release
Description
1.0.0-beta.1
Devices/Android versions reproduced on:
- Samsung Galaxy. Pixel 3. A wide range of devices. with Android 9, Android 10 and Android 11.
Android Auto version reproduced on:
- 5.9.604633-release
If this is a bug in the library, we would appreciate if you could attach, if possible:
- Sample project to trigger the issue.
In the code, getTemplate() could return null sometimes. It is not clear in the documentation if we could return null for this function.
- A screen record or screenshots showing the issue (if UI related).
- A bug report. See attached log.txt.