Status Update
No update yet.
Comments
al...@google.com
Bugfix candidate.
We should move the DisplayManagerCompat to be the key or iterate over a list of weak references to instances.
Long-term -- once we bump the minSdk above 17, we can deprecate the non-static methods on this class and convert it to a static shims compat class.
al...@google.com
Owen suggested WeakHashMap<Context, WeakReference<DisplayManagerCompat>>, which seems like it should work. I'm still looking into how we can write a reliable test to ensure the objects actually get GC'ed before committing to an approach.
al...@google.com
Not finding anything that suggests I can reliably test a change involving GC. I'm going to go with the simpler approach.
ow...@google.com
Is there a feasible way to check if we have the same issue anywhere else?
al...@google.com
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
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
na...@google.com
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.core:core:1.11.0-alpha04
Description
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)populatesDisplayManagerCompat.sInstances, which is of typeWeakHashMap<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