From 8b5890b17d23780ee60c4fd0a4d252b2c63446f1 Mon Sep 17 00:00:00 2001 From: hoisie Date: Fri, 19 Nov 2021 16:30:31 -0800 Subject: [PATCH] Fix theme native object collection in Android S Previously, in Android P through R, Robolectric would clean up native themes from the registry when AssetManager.nativeThemeDestroy was called by the ThemeImpl finalizer. However, starting in Android S, AssetManager.nativeThemeDestroy does not exist any more. As a workaround, hook into the AssetManager.releaseTheme method to free the theme from the registry. Longer term we may want to have the theme destruction logic be part of AssetManager.nativeDestroy, where all themes can potentially be freed in one fell swoop. This should fix #6872. Thanks to @calvarez-ov for reporting the issue and helping debug :) PiperOrigin-RevId: 411169531 --- .../robolectric/shadows/ShadowThemeTest.java | 44 +++++++++++++++++++ .../shadows/ShadowArscAssetManager10.java | 10 +++++ 2 files changed, 54 insertions(+) diff --git a/robolectric/src/test/java/org/robolectric/shadows/ShadowThemeTest.java b/robolectric/src/test/java/org/robolectric/shadows/ShadowThemeTest.java index 1daf23bb8a7..32bace8ed3e 100644 --- a/robolectric/src/test/java/org/robolectric/shadows/ShadowThemeTest.java +++ b/robolectric/src/test/java/org/robolectric/shadows/ShadowThemeTest.java @@ -9,6 +9,7 @@ import android.content.res.TypedArray; import android.content.res.XmlResourceParser; import android.graphics.drawable.ColorDrawable; +import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.util.AttributeSet; import android.util.Xml; @@ -16,6 +17,11 @@ import android.widget.Button; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +import java.lang.ref.WeakReference; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -23,7 +29,10 @@ import org.robolectric.R; import org.robolectric.Robolectric; import org.robolectric.android.controller.ActivityController; +import org.robolectric.annotation.Config; +import org.robolectric.res.android.Registries; import org.robolectric.shadows.testing.TestActivity; +import org.robolectric.util.ReflectionHelpers; import org.xmlpull.v1.XmlPullParser; @RunWith(AndroidJUnit4.class) @@ -272,6 +281,41 @@ public void shouldApplyFromStyleAttribute() { assertThat(button.getLayoutParams().width).isEqualTo(42); // comes via style attr } + @Test + @Config(minSdk = VERSION_CODES.N) + public void shouldFreeNativeObjectInRegistry() { + final AtomicLong themeId = new AtomicLong(0); + Supplier themeSupplier = + () -> { + Theme theme = resources.newTheme(); + long nativeId = + ReflectionHelpers.getField(ReflectionHelpers.getField(theme, "mThemeImpl"), "mTheme"); + themeId.set(nativeId); + return theme; + }; + + WeakReference weakRef = new WeakReference<>(themeSupplier.get()); + awaitFinalized(weakRef); + assertThat(Registries.NATIVE_THEME9_REGISTRY.peekNativeObject(themeId.get())).isNull(); + } + + private static void awaitFinalized(WeakReference weakRef) { + final CountDownLatch latch = new CountDownLatch(1); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (System.nanoTime() < deadline) { + if (weakRef.get() == null) { + return; + } + try { + System.gc(); + latch.await(100, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + throw new AssertionError(e); + } + } + } + ; + //////////////////////////// private XmlResourceParser getFirstElementAttrSet(int resId) throws Exception { diff --git a/shadows/framework/src/main/java/org/robolectric/shadows/ShadowArscAssetManager10.java b/shadows/framework/src/main/java/org/robolectric/shadows/ShadowArscAssetManager10.java index 3c4ba90a1a1..a238141374a 100644 --- a/shadows/framework/src/main/java/org/robolectric/shadows/ShadowArscAssetManager10.java +++ b/shadows/framework/src/main/java/org/robolectric/shadows/ShadowArscAssetManager10.java @@ -3,6 +3,7 @@ import static android.os.Build.VERSION_CODES.P; import static android.os.Build.VERSION_CODES.Q; import static android.os.Build.VERSION_CODES.R; +import static android.os.Build.VERSION_CODES.S; import static org.robolectric.res.android.ApkAssetsCookie.K_INVALID_COOKIE; import static org.robolectric.res.android.ApkAssetsCookie.kInvalidCookie; import static org.robolectric.res.android.Asset.SEEK_CUR; @@ -1610,6 +1611,12 @@ protected static void nativeThemeDestroy(long theme_ptr) { Registries.NATIVE_THEME9_REGISTRY.unregister(theme_ptr); } + @Implementation(minSdk = S) + protected void releaseTheme(long ptr) { + Registries.NATIVE_THEME9_REGISTRY.unregister(ptr); + reflector(AssetManagerReflector.class, realAssetManager).releaseTheme(ptr); + } + // static void NativeThemeApplyStyle(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlong theme_ptr, // jint resid, jboolean force) { @Implementation(minSdk = P) @@ -1936,6 +1943,9 @@ interface AssetManagerReflector { @Static @Direct void createSystemAssetsInZygoteLocked(); + + @Direct + void releaseTheme(long ptr); } } ; // namespace android