DisplayManagerCompat leaks all used Contexts [279625765]

Fixed

Bug

Status Update

No update yet.

Description

ub...@gmail.com

created issue #1

Apr 25, 2023 06:58PM

Component used: androidx-core

Version used: 1.10.0

Devices/Android versions reproduced on: All

DisplayManagerCompat leaks the Context argument used for DisplayManagerCompat.getInstance(Context). This is easily seen from code inspection:

getInstance(Context) populates DisplayManagerCompat.sInstances, which is of type WeakHashMap<Context, DisplayManagerCompat>.

WeakHashMap has weak keys, not weak values. DisplayManagerCompat.mContext is a hard reference to the same (key) Context object, and therefore causes the entire DisplayManagerCompat instance to leak, including the Context. This circular behavior caveat is spelled out in the javadoc for WeakHashMap.

This issue was one of the root causes for https://issuetracker.google.com/issues/37137738

Comments

al...@google.com <al...@google.com> #2Apr 25, 2023 07:09PM

Accepted by al...@google.com.

Trying to reproduce this on my 4.2.2 (v17) Nexus 4. Added this drawable:

<transition xmlns:android="

http://schemas.android.com/apk/res/android" >
<item android:drawable="@drawable/test_drawable_blue"/>
<item android:drawable="@drawable/test_drawable_green"/>
</transition>

where blue/green drawables look like this:

<shape
xmlns:android="

http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<size
android:width="@dimen/drawable_large_size"
android:height="@dimen/drawable_small_size" />
<solid
android:color="@color/test_blue" />
</shape>

Then added this test:

@Test
public void testMutateTransitionDrawable() {
Drawable drawable = ResourcesCompat.getDrawable(mResources,
R.drawable.test_transition_drawable, null);
assertTrue(drawable instanceof TransitionDrawable);

Drawable mutated = drawable.mutate();
assertTrue(drawable instanceof TransitionDrawable);
assertTrue(mutated instanceof TransitionDrawable);
}

It passes on the device. Going to also try on other earlier devices a bit later in the day once they are charged.

al...@google.com <al...@google.com> #3Apr 26, 2023 04:36PM

Also not reproducible on Galaxy Nexus running 4.0.1 (v14), 4.0.4 (v15) and 4.2.2 (v17)

al...@google.com <al...@google.com> #4Apr 26, 2023 04:39PM

https://android-review.googlesource.com/c/platform/frameworks/support/+/960032 with the test. Closing as not reproducible. If this is still happening for you, we'd need to see either a test (same as in this CL) or a small, standalone sample project that reproduces this issue on v17 (or other) devices.

ow...@google.com <ow...@google.com> #5Apr 26, 2023 04:42PM

Is there a feasible way to check if we have the same issue anywhere else?

al...@google.com <al...@google.com> #6Apr 26, 2023 05:10PM

We do have a static final WeakHashMap<LocationListenerKey, WeakReference<LocationListenerTransport>> sLocationListeners = new WeakHashMap<>(); so hopefully that works as intended.

There are also a couple usages that are wrong, but not in a way that will cause a leak -- just caches that aren't actually doing anything useful.

Ah, except this in sdkruntime-client:

private val sInstances = WeakHashMap<Context, SdkSandboxManagerCompat>()

Time to file some more bugs.

ap...@google.com <ap...@google.com> #7Apr 26, 2023 08:30PM

Marked as fixed.

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

commit 78cb5d98af3a7e4433e6623c90f56a311dc4fdd5
Author: Alan Viverette <alanv@google.com>
Date: Tue Apr 25 16:06:46 2023

Use WeakReference for caching DisplayManagerCompat instances

Relnote: Fixed Context leak in DisplayManagerCompat
Fixes: 279625765
Test: DisplayManagerCompatTest
Change-Id: I3409b324301609dba940ef5894ff349b0f229d13

A core/core/src/androidTest/java/androidx/core/hardware/display/DisplayManagerCompatTest.kt
M core/core/src/main/java/androidx/core/hardware/display/DisplayManagerCompat.java

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

na...@google.com <na...@google.com> #8May 10, 2023 07:08PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.core:core:1.11.0-alpha04