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.

There is a report about it in kotlinlang Slack: https://kotlinlang.slack.com/archives/C01D6HTPATV/p1706005056105739

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

Thanks for the report!

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

The release notes documentation has been edited to clarify this change in behavior for line height.

To support non-standard text sizes, we encourage users to follow the Material design system and use a different type scale rather than changing the font size directly. Alternatively, users can overwrite the line height like so: style = LocalTextStyle.current.copy(lineHeight = TextUnit.Unspecified), or create a custom Typography entirely.

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

deleted

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

In my case, I have multiple font sizes in the same Text (using SpanStyle in AnnotatedString). There are legitimate reasons for this. For example, when combining Chinese and English (phonetic) together (for language-learning purposes).

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