Makeshift analog of Strictmode leak detector
Strictmode leak detector is still a goal, but we might not be able to
achieve it in R. Strictmode has several framework-side bugs that perhaps
hide Launcher-side strictmode violations, while the time to fix
everything is limited, and new leaks get introduced all the time.
For now, implementing a check that is slightly more relaxed than
Strictmode, but still ensures the absence of leaks. I’ll keep
eliminating Strictmode violations as well as keep strengthening the
makeshift checker conditions until we’ll be able to enable Strictmode in
continuous testing.
I’m disabling Strictmode checks for now so that they don’t generate
unnecessary hprof dumps, but leaving the code dealing with strictmode.
Bug: 139137636
Change-Id: Ib10136b0d4e9892f70a19cd052ae5a54cf0a4efb
diff --git a/quickstep/tests/src/com/android/quickstep/NavigationModeSwitchRule.java b/quickstep/tests/src/com/android/quickstep/NavigationModeSwitchRule.java
index 0afe4a8..40265c4 100644
--- a/quickstep/tests/src/com/android/quickstep/NavigationModeSwitchRule.java
+++ b/quickstep/tests/src/com/android/quickstep/NavigationModeSwitchRule.java
@@ -203,7 +203,7 @@
+ launcher.getNavigationModeMismatchError(),
() -> launcher.getNavigationModeMismatchError() == null,
60000 /* b/148422894 */, launcher);
- AbstractLauncherUiTest.checkDetectedLeaks();
+ AbstractLauncherUiTest.checkDetectedLeaks(launcher);
return true;
}
diff --git a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java
index 86faddb..b8c650c 100644
--- a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java
+++ b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java
@@ -100,9 +100,10 @@
public static final long DEFAULT_UI_TIMEOUT = 10000;
private static final String TAG = "AbstractLauncherUiTest";
- private static String sDetectedActivityLeak;
+ private static String sStrictmodeDetectedActivityLeak;
private static boolean sActivityLeakReported;
private static final String SYSTEMUI_PACKAGE = "com.android.systemui";
+ private static final ActivityLeakTracker ACTIVITY_LEAK_TRACKER = new ActivityLeakTracker();
protected LooperExecutor mMainThreadExecutor = MAIN_EXECUTOR;
protected final UiDevice mDevice = UiDevice.getInstance(getInstrumentation());
@@ -115,30 +116,51 @@
if (TestHelpers.isInLauncherProcess()) {
StrictMode.VmPolicy.Builder builder =
new StrictMode.VmPolicy.Builder()
- .detectActivityLeaks()
+// b/154772063
+// .detectActivityLeaks()
.penaltyLog()
.penaltyListener(Runnable::run, violation -> {
- // Runs in the main thread. We can't dumpheap in the main thread,
- // so let's just mark the fact that the leak has happened.
- if (sDetectedActivityLeak == null) {
- sDetectedActivityLeak = violation.toString();
- try {
- Debug.dumpHprofData(
- getInstrumentation().getTargetContext()
- .getFilesDir().getPath()
- + "/ActivityLeakHeapDump.hprof");
- } catch (Throwable e) {
- Log.e(TAG, "dumpHprofData failed", e);
- }
+ if (sStrictmodeDetectedActivityLeak == null) {
+ sStrictmodeDetectedActivityLeak = violation.toString() + ", "
+ + dumpHprofData() + ".";
}
});
StrictMode.setVmPolicy(builder.build());
}
}
- public static void checkDetectedLeaks() {
- if (sDetectedActivityLeak != null && !sActivityLeakReported) {
+ public static void checkDetectedLeaks(LauncherInstrumentation launcher) {
+ if (sActivityLeakReported) return;
+
+ if (sStrictmodeDetectedActivityLeak != null) {
+ // Report from the test thread strictmode violations detected in the main thread.
sActivityLeakReported = true;
+ Assert.fail(sStrictmodeDetectedActivityLeak);
+ }
+
+ // Check whether activity leak detector has found leaked activities.
+ Wait.atMost(AbstractLauncherUiTest::getActivityLeakErrorMessage,
+ () -> {
+ launcher.getTotalPssKb(); // Triggers GC
+ return MAIN_EXECUTOR.submit(
+ () -> ACTIVITY_LEAK_TRACKER.noLeakedActivities()).get();
+ }, DEFAULT_UI_TIMEOUT, launcher);
+ }
+
+ private static String getActivityLeakErrorMessage() {
+ sActivityLeakReported = true;
+ return "Activity leak detector has found leaked activities, " + dumpHprofData() + ".";
+ }
+
+ private static String dumpHprofData() {
+ try {
+ final String fileName = getInstrumentation().getTargetContext().getFilesDir().getPath()
+ + "/ActivityLeakHeapDump.hprof";
+ Debug.dumpHprofData(fileName);
+ return "memory dump filename: " + fileName;
+ } catch (Throwable e) {
+ Log.e(TAG, "dumpHprofData failed", e);
+ return "failed to save memory dump";
}
}
@@ -265,7 +287,7 @@
if (mLauncherPid != 0) {
assertEquals("Launcher crashed, pid mismatch:", mLauncherPid, mLauncher.getPid());
}
- checkDetectedLeaks();
+ checkDetectedLeaks(mLauncher);
}
protected void clearLauncherData() throws IOException, InterruptedException {
diff --git a/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java b/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java
new file mode 100644
index 0000000..0b5ce02
--- /dev/null
+++ b/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.launcher3.ui;
+
+import android.app.Activity;
+import android.app.Application;
+import android.os.Bundle;
+
+import androidx.test.InstrumentationRegistry;
+
+import com.android.launcher3.tapl.TestHelpers;
+
+import java.util.WeakHashMap;
+
+class ActivityLeakTracker implements Application.ActivityLifecycleCallbacks {
+ private final WeakHashMap<Activity, Boolean> mActivities = new WeakHashMap<>();
+
+ ActivityLeakTracker() {
+ if (!TestHelpers.isInLauncherProcess()) return;
+ final Application app =
+ (Application) InstrumentationRegistry.getTargetContext().getApplicationContext();
+ app.registerActivityLifecycleCallbacks(this);
+ }
+
+ @Override
+ public void onActivityCreated(Activity activity, Bundle bundle) {
+ mActivities.put(activity, true);
+ }
+
+ @Override
+ public void onActivityStarted(Activity activity) {
+ }
+
+ @Override
+ public void onActivityResumed(Activity activity) {
+ }
+
+ @Override
+ public void onActivityPaused(Activity activity) {
+ }
+
+ @Override
+ public void onActivityStopped(Activity activity) {
+ }
+
+ @Override
+ public void onActivitySaveInstanceState(Activity activity, Bundle bundle) {
+ }
+
+ @Override
+ public void onActivityDestroyed(Activity activity) {
+ }
+
+ public boolean noLeakedActivities() {
+ int liveActivities = 0;
+ int destroyedActivities = 0;
+
+ for (Activity activity : mActivities.keySet()) {
+ if (activity.isDestroyed()) {
+ ++destroyedActivities;
+ } else {
+ ++liveActivities;
+ }
+ }
+
+ // It's OK to have 1 leaked activity if no active activities exist.
+ return liveActivities == 0 ? destroyedActivities <= 1 : destroyedActivities == 0;
+ }
+}
diff --git a/tests/src/com/android/launcher3/ui/PortraitLandscapeRunner.java b/tests/src/com/android/launcher3/ui/PortraitLandscapeRunner.java
index 38f50c1..266f0ae 100644
--- a/tests/src/com/android/launcher3/ui/PortraitLandscapeRunner.java
+++ b/tests/src/com/android/launcher3/ui/PortraitLandscapeRunner.java
@@ -56,7 +56,7 @@
private void evaluateInPortrait() throws Throwable {
mTest.mDevice.setOrientationNatural();
mTest.mLauncher.setExpectedRotation(Surface.ROTATION_0);
- AbstractLauncherUiTest.checkDetectedLeaks();
+ AbstractLauncherUiTest.checkDetectedLeaks(mTest.mLauncher);
base.evaluate();
mTest.getDevice().pressHome();
}
@@ -64,7 +64,7 @@
private void evaluateInLandscape() throws Throwable {
mTest.mDevice.setOrientationLeft();
mTest.mLauncher.setExpectedRotation(Surface.ROTATION_90);
- AbstractLauncherUiTest.checkDetectedLeaks();
+ AbstractLauncherUiTest.checkDetectedLeaks(mTest.mLauncher);
base.evaluate();
mTest.getDevice().pressHome();
}
diff --git a/tests/src/com/android/launcher3/ui/TaplTestsLauncher3.java b/tests/src/com/android/launcher3/ui/TaplTestsLauncher3.java
index 57000a0..34e425d 100644
--- a/tests/src/com/android/launcher3/ui/TaplTestsLauncher3.java
+++ b/tests/src/com/android/launcher3/ui/TaplTestsLauncher3.java
@@ -64,7 +64,7 @@
test.waitForResumed("Launcher internal state is still Background");
// Check that we switched to home.
test.mLauncher.getWorkspace();
- AbstractLauncherUiTest.checkDetectedLeaks();
+ AbstractLauncherUiTest.checkDetectedLeaks(test.mLauncher);
}
// Please don't add negative test cases for methods that fail only after a long wait.