Lint: PartialResults merging works incorrectly [239337003]

Fixed

Bug

Status Update

No update yet.

Description

li...@gmail.com

created issue #1

Jul 18, 2022 04:46PM

DESCRIBE THE ISSUE IN DETAIL:

STEPS TO REPRODUCE:

Download project from the attach;
Run ./gradlew :app:lintDebug and observe 2 issues;
Run ./gradlew :lint-checks:test and observe tests failing.

It looks like PartialResult#map doesn't follow the contract in unit tests. It claims to return LintMap for the project in context, but under the certain condition it seems to be violated.

The conditions require multimodule setup. A library module must put smth into its LintMap. Then, when the app module accesses its own LintMap for the first time it get a map prefilled with the data from the library module, instead of the fresh one.

To demonstrate the issue I created the following setup:

An :app module and a :library module;
:app module contains app_color color resource, while :library module contains :library_color;
I created a simple detector, which works only in partial analysis mode and does a simple thing: detects the color resource declaration within the module, puts into the partial results and then reports all colors found per module in checkPartialResults method;
There's a unit test, which mimics the same setup.

If you run lint checks using the ./gradlew :app:lintDebug command, you'll observe Lint correctly reporting app_color found in :app and library_color found in :library. However, if you run the unit tests, you'll observe that Lint also reports a library_color found in the :app.

It looks like while scanning the :app module, the first access to partial results, causes LintCliClient to create new instance of results and fill it with results of the dependent module (see LintCliClient#getPartialResults method). For some reason this is happening only in tests.

The replacement of the getPartialResult.map() method call with getPartialResult.mapFor(context.project) method call successfully addresses the issue.

There's still a chance, that my test is wrong. In this case, would be great to know in which way.

Studio Build: Chipmunk | 2021.2.1 Version of Gradle Plugin: 7.2.1, 7.4.0-alpha08 Version of Gradle: 7.4.2 OS: Mac OS Monteray 12.4

LintBug2.zip

803 KB

Download

Comments

li...@gmail.com <li...@gmail.com> #2Jul 18, 2022 04:58PM

It looks isAudioPathA2DPStatic() is not in the android internal code base which means Samsung specific changes.

ch...@google.com <ch...@google.com> Jul 20, 2022 01:09PM

Assigned to an...@google.com.

li...@gmail.com <li...@gmail.com> #3Jul 20, 2022 03:45PM

Banseok, do you know where to reroute this issue?

je...@google.com <je...@google.com> Jul 26, 2022 01:20AM

Reassigned to sp...@google.com.

sp...@google.com <sp...@google.com> #4Jul 26, 2022 05:00PM

Reassigned to ra...@google.com.

Any news on this.

Now have seen this crash for 16 unique users. Not much in the grand scheme of things, but still would be useful to have a workaround in the library, catching that IllegalArgumentException somewhere in that stack trace and ignoring it (maybe dumping the stack trace in the log cat).

ra...@google.com <ra...@google.com> Aug 15, 2022 06:32PM

Reassigned to pa...@google.com.

pa...@google.com <pa...@google.com> Aug 30, 2022 11:11AM

Reassigned to pa...@google.com.

pa...@google.com <pa...@google.com> #5Aug 30, 2022 07:38PM

The crash seems to be slightly increasing with Android 12. Could this error being handled on MediaRouter library?

li...@gmail.com <li...@gmail.com> #6Sep 9, 2022 12:14PM

Still happening more and more. Here's an Android 11 stack trace. Please do something about it, even if it is a Samsung specific bug, as it cannot be worked-around at the app level:

java.lang.IllegalArgumentException: Bad stream type -1
at android.os.Parcel.createExceptionOrNull(Parcel.java:2441)
at android.os.Parcel.createException(Parcel.java:2421)
at android.os.Parcel.readException(Parcel.java:2404)
at android.os.Parcel.readException(Parcel.java:2346)
at android.media.IAudioService$Stub$Proxy.getDevicesForStream(IAudioService.java:6999)
at android.media.MediaRouter.isAudioPathA2DPStatic(MediaRouter.java:3310)
at android.media.MediaRouter.selectDefaultRouteStatic(MediaRouter.java:1172)
at android.media.MediaRouter.removeRouteStatic(MediaRouter.java:1288)
at android.media.MediaRouter.removeUserRoute(MediaRouter.java:1244)
at androidx.mediarouter.media.MediaRouterJellybean.removeUserRoute(MediaRouterJellybean.java:110)
at androidx.mediarouter.media.SystemMediaRouteProvider$JellybeanImpl.onSyncRouteRemoved(SystemMediaRouteProvider.java:493)
at androidx.mediarouter.media.MediaRouter$GlobalMediaRouter$CallbackHandler.syncWithSystemProvider(MediaRouter.java:3654)
at androidx.mediarouter.media.MediaRouter$GlobalMediaRouter$CallbackHandler.handleMessage(MediaRouter.java:3622)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loopOnce(Looper.java:226)
at android.os.Looper.loop(Looper.java:313)
at android.app.ActivityThread.main(ActivityThread.java:8633)
at java.lang.reflect.Method.invoke(Method.java)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:567)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1135)
Caused by: android.os.RemoteException: Remote stack trace:
at com.android.server.audio.AudioService.ensureValidStreamType(AudioService.java:6407)
at com.android.server.audio.AudioService.getDevicesForStream(AudioService.java:6740)
at android.media.IAudioService$Stub.onTransact(IAudioService.java:3157)
at android.os.Binder.execTransactInternal(Binder.java:1215)
at android.os.Binder.execTransact(Binder.java:1179)
android.os.RemoteException: Remote stack trace:
at com.android.server.audio.AudioService.ensureValidStreamType(AudioService.java:6407)
at com.android.server.audio.AudioService.getDevicesForStream(AudioService.java:6740)
at android.media.IAudioService$Stub.onTransact(IAudioService.java:3157)
at android.os.Binder.execTransactInternal(Binder.java:1215)
at android.os.Binder.execTransact(Binder.java:1179)

pa...@google.com <pa...@google.com> #7Sep 23, 2022 11:46AM

Stack trace above is from a Samsung Android 12 device (not 11 as stated).

pa...@google.com <pa...@google.com> #8Oct 25, 2022 09:34AM

Marked as fixed.

Have same problem. For now 67 users affected, mostly android 12, 100% samsung

li...@gmail.com <li...@gmail.com> #9Nov 1, 2022 04:59PM

Seeing the same issue on my app - 100 users in the last 7 days. 100% Samsung phones running Android 12, always when the app is backgrounded, and always when they're casting video to Chromecast.
It'd be great if it could be caught, but ultimately this looks like a Samsung bug.

sa...@google.com <sa...@google.com> #10Nov 2, 2022 05:32PM

I can confirm that we see the exact same behavior in our app when casting. Around 120 users run into this crash each week, which is hurting our crashrate significantly. The key points are all the same:
- 100% Samsung, of which most are of the type Galaxy A52s 5G and Galaxy A51
- 100% Android 12
- 98% App in background

Unfortunately we don't seem to be the ones initiating some sort of action so we can't catch the exception in our application code.

sa...@google.com <sa...@google.com> #11Nov 3, 2022 01:42PM

This is happening more and more... Apparently, there does not seem to be an incentive from Google to workaround it in this library probably because "not our problem" (it is a Samsung bug). The only thing we can do is to decompile mediarouter and catch the IllegalArguementException ourselves...

li...@gmail.com <li...@gmail.com> #12Jan 13, 2023 03:51PM

Any chance to fix this in somehow? 200+ users in my app has this error. 100% Samsung 99% - android 12 1% - android 11

Message last modified on Jan 13, 2023 03:56PM

pa...@google.com <pa...@google.com> Jan 13, 2023 03:54PM

Status: Assigned (reopened)

pa...@google.com <pa...@google.com> #13Jan 18, 2023 05:45PM

Marked as fixed.

I finally patched androidx.mediarouter:mediarouter:1.3.0 with Recaf to catch that pesky IllegalArgumentException:

in androidx.mediarouter.media.MediaRouterJellybean, I changed:

 public static void removeUserRoute(Object routerObj, Object routeObj) {
       ((MediaRouter)routerObj).removeUserRoute((MediaRouter.UserRouteInfo)routeObj);
    }

to:

 public static void removeUserRoute(Object routerObj, Object routeObj) {
        try {
            ((MediaRouter)routerObj).removeUserRoute((MediaRouter.UserRouteInfo)routeObj);
        }
        catch (IllegalArgumentException ex) {
            Log.w(TAG, "android.media.MediaRouter.removeUserRoute() failed with IllegalArgumentException (Samsung Android 12 bug work-around)", ex);
        }
    }

To use it:

download https://bubblesoftapps.com/mediarouter-samsung-fix/mediarouter-samsung-fix-1.3.0.zip
unzip in your .m2/repository folder so you end up having /path/to/.m2/repository/androidx/mediarouter/mediarouter-samsung-fix/
update your build.gradle to use it: replace implementation 'androidx.mediarouter:mediarouter:1.3.0' with implementation 'androidx.mediarouter:mediarouter-samsung-fix:1.3.0'
if you have an AndroidX dependency pulling 'androidx.mediarouter:mediarouter', you will have to exclude it so it does not get in the way (duplicate classes). For example, the cast sdk needs it:

implementation('com.google.android.gms:play-services-cast:21.0.1') {
   // replaced by mediarouter-samsung-fix
   exclude group: 'androidx.mediarouter'
}

li...@gmail.com <li...@gmail.com> #14May 9, 2023 12:04PM

Comment has been deleted.

Message last modified on May 9, 2023 12:06PM

li...@gmail.com <li...@gmail.com> #15May 9, 2023 12:06PM

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

commit c3805953951058b509f629980ffad858c7a531ee
Author: Oliver Woodman <olly@google.com>
Date: Fri May 20 13:35:47 2022

Workaround for crash removing user routes.

Test: N/A
Bug: 202931542
Change-Id: Ia25cd716cf2cd48ec298aff04e13751cbcb27d45

M mediarouter/mediarouter/src/main/java/androidx/mediarouter/media/MediaRouterJellybean.java

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