From bb8c264ec776ca0b643f82a7c0ed3ce3d2697740 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 | 35 +++++++++++++++ 2 files changed, 79 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 470df765bb9..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; @@ -73,6 +74,7 @@ import org.robolectric.res.android.ResXMLTree; import org.robolectric.res.android.ResourceTypes.Res_value; import org.robolectric.shadow.api.Shadow; +import org.robolectric.util.PerfStatsCollector; import org.robolectric.util.ReflectionHelpers; import org.robolectric.util.ReflectionHelpers.ClassParameter; import org.robolectric.util.reflector.Direct; @@ -1388,6 +1390,30 @@ protected static void nativeApplyStyle( @NonNull int[] java_attrs, long out_values_ptr, long out_indices_ptr) { + PerfStatsCollector.getInstance() + .measure( + "applyStyle", + () -> + nativeApplyStyle_measured( + ptr, + theme_ptr, + def_style_attr, + def_style_resid, + xml_parser_ptr, + java_attrs, + out_values_ptr, + out_indices_ptr)); + } + + private static void nativeApplyStyle_measured( + long ptr, + long theme_ptr, + @AttrRes int def_style_attr, + @StyleRes int def_style_resid, + long xml_parser_ptr, + @NonNull int[] java_attrs, + long out_values_ptr, + long out_indices_ptr) { CppAssetManager2 assetmanager = AssetManagerFromLong(ptr); Theme theme = Registries.NATIVE_THEME9_REGISTRY.getNativeObject(theme_ptr); CHECK(theme.GetAssetManager() == assetmanager); @@ -1585,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) @@ -1911,6 +1943,9 @@ interface AssetManagerReflector { @Static @Direct void createSystemAssetsInZygoteLocked(); + + @Direct + void releaseTheme(long ptr); } } ; // namespace android