diff --git a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java index a972ef206a..eb2082f964 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java @@ -190,7 +190,7 @@ private boolean isIgnoredDominator(Instance dominator, Instance instance) { private LeakTrace buildLeakTrace(LeakNode leakingNode) { List elements = new ArrayList<>(); // We iterate from the leak to the GC root - LeakNode node = new LeakNode(null, leakingNode, null, null); + LeakNode node = new LeakNode(null, null, leakingNode, null, null); while (node != null) { LeakTraceElement element = buildLeakElement(node); if (element != null) { @@ -264,22 +264,23 @@ private LeakTraceElement buildLeakElement(LeakNode node) { Class[] interfaces = actualClass.getInterfaces(); if (interfaces.length > 0) { Class implementedInterface = interfaces[0]; - extra = "(anonymous class implements " + implementedInterface.getName() + ")"; + extra = "(anonymous implementation of " + implementedInterface.getName() + ")"; } else { - extra = "(anonymous class extends java.lang.Object)"; + extra = "(anonymous subclass of java.lang.Object)"; } } catch (ClassNotFoundException ignored) { } } else { holderType = OBJECT; // Makes it easier to figure out which anonymous class we're looking at. - extra = "(anonymous class extends " + parentClassName + ")"; + extra = "(anonymous subclass of " + parentClassName + ")"; } } else { holderType = OBJECT; } } - return new LeakTraceElement(referenceName, type, holderType, className, extra, fields); + return new LeakTraceElement(referenceName, type, holderType, className, extra, node.exclusion, + fields); } private long since(long analysisStartNanoTime) { diff --git a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java index 69ec8717a0..b30df2782c 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java @@ -18,13 +18,16 @@ import com.squareup.haha.perflib.Instance; final class LeakNode { + /** May be null. */ + final Exclusion exclusion; final Instance instance; final LeakNode parent; final String referenceName; final LeakTraceElement.Type referenceType; - LeakNode(Instance instance, LeakNode parent, String referenceName, - LeakTraceElement.Type referenceType) { + LeakNode(Exclusion exclusion, Instance instance, LeakNode parent, + String referenceName, LeakTraceElement.Type referenceType) { + this.exclusion = exclusion; this.instance = instance; this.parent = parent; this.referenceName = referenceName; diff --git a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java index 02f85f6477..89ef31e35c 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java @@ -48,16 +48,20 @@ public enum Holder { /** Additional information, may be null. */ public final String extra; + /** If not null, there was no path that could exclude this element. */ + public final Exclusion exclusion; + /** List of all fields (member and static) for that object. */ public final List fields; LeakTraceElement(String referenceName, Type type, Holder holder, String className, String extra, - List fields) { + Exclusion exclusion, List fields) { this.referenceName = referenceName; this.type = type; this.holder = holder; this.className = className; this.extra = extra; + this.exclusion = exclusion; this.fields = unmodifiableList(new ArrayList<>(fields)); } @@ -83,6 +87,11 @@ public enum Holder { if (extra != null) { string += " " + extra; } + + if (exclusion != null) { + string += " , matching exclusion " + exclusion.matching; + } + return string; } diff --git a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java index 82028a181b..db6100ef55 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java @@ -89,6 +89,9 @@ Result findPath(Snapshot snapshot, Instance leakingRef) { node = toVisitQueue.poll(); } else { node = toVisitIfNoPathQueue.poll(); + if (node.exclusion == null) { + throw new IllegalStateException("Expected node to have an exclusion " + node); + } excludingKnownLeaks = true; } @@ -131,9 +134,9 @@ private void enqueueGcRoots(Snapshot snapshot) { case JAVA_LOCAL: Instance thread = HahaSpy.allocatingThread(rootObj); String threadName = threadName(thread); - Boolean alwaysIgnore = excludedRefs.threadNames.get(threadName); - if (alwaysIgnore == null || !alwaysIgnore) { - enqueue(alwaysIgnore == null, null, rootObj, null, null); + Exclusion params = excludedRefs.threadNames.get(threadName); + if (params == null || !params.alwaysExclude) { + enqueue(params, null, rootObj, null, null); } break; case INTERNED_STRING: @@ -160,7 +163,7 @@ private void enqueueGcRoots(Snapshot snapshot) { // Input or output parameters in native code. case NATIVE_STACK: case JAVA_STATIC: - enqueue(true, null, rootObj, null, null); + enqueue(null, null, rootObj, null, null); break; default: throw new UnsupportedOperationException("Unknown root type:" + rootObj.getRootType()); @@ -176,50 +179,48 @@ private void visitRootObj(LeakNode node) { RootObj rootObj = (RootObj) node.instance; Instance child = rootObj.getReferredInstance(); - boolean visitRootNow = true; - if (child != null) { - // true = ignore root, false = visit root later, null = visit root now. - Boolean rootSuperClassAlwaysIgnored = rootSuperClassAlwaysIgnored(child); - if (rootSuperClassAlwaysIgnored != null) { - if (rootSuperClassAlwaysIgnored) { - return; - } else { - visitRootNow = false; - } - } + Exclusion exclusion = rootSuperClassAlwaysIgnored(child); + + if (exclusion != null && exclusion.alwaysExclude) { + return; } if (rootObj.getRootType() == RootType.JAVA_LOCAL) { Instance holder = HahaSpy.allocatingThread(rootObj); // We switch the parent node with the thread instance that holds // the local reference. - LeakNode parent = new LeakNode(holder, null, null, null); - enqueue(visitRootNow, parent, child, "", LOCAL); + LeakNode parent = new LeakNode(null, holder, null, null, null); + if (node.exclusion != null) { + exclusion = node.exclusion; + } + enqueue(exclusion, parent, child, "", LOCAL); } else { - enqueue(visitRootNow, node, child, null, null); + enqueue(exclusion, node, child, null, null); } } - private Boolean rootSuperClassAlwaysIgnored(Instance child) { - Boolean alwaysIgnoreClassHierarchy = null; + private Exclusion rootSuperClassAlwaysIgnored(Instance child) { + if (child == null) { + return null; + } + Exclusion matchingParams = null; ClassObj superClassObj = child.getClassObj(); while (superClassObj != null) { - Boolean alwaysIgnoreClass = - excludedRefs.rootSuperClassNames.get(superClassObj.getClassName()); - if (alwaysIgnoreClass != null) { + Exclusion params = excludedRefs.rootClassNames.get(superClassObj.getClassName()); + if (params != null) { // true overrides null or false. - if (alwaysIgnoreClassHierarchy == null || !alwaysIgnoreClassHierarchy) { - alwaysIgnoreClassHierarchy = alwaysIgnoreClass; + if (matchingParams == null || !matchingParams.alwaysExclude) { + matchingParams = params; } } superClassObj = superClassObj.getSuperClassObj(); } - return alwaysIgnoreClassHierarchy; + return matchingParams; } private void visitClassObj(LeakNode node) { ClassObj classObj = (ClassObj) node.instance; - Map ignoredStaticFields = + Map ignoredStaticFields = excludedRefs.staticFieldNameByClassName.get(classObj.getClassName()); for (Map.Entry entry : classObj.getStaticFieldValues().entrySet()) { Field field = entry.getKey(); @@ -233,72 +234,60 @@ private void visitClassObj(LeakNode node) { Instance child = (Instance) entry.getValue(); boolean visit = true; if (ignoredStaticFields != null) { - Boolean alwaysIgnore = ignoredStaticFields.get(fieldName); - if (alwaysIgnore != null) { + Exclusion params = ignoredStaticFields.get(fieldName); + if (params != null) { visit = false; - if (!alwaysIgnore) { - enqueue(false, node, child, fieldName, STATIC_FIELD); + if (!params.alwaysExclude) { + enqueue(params, node, child, fieldName, STATIC_FIELD); } } } if (visit) { - enqueue(true, node, child, fieldName, STATIC_FIELD); + enqueue(null, node, child, fieldName, STATIC_FIELD); } } } private void visitClassInstance(LeakNode node) { ClassInstance classInstance = (ClassInstance) node.instance; - Map ignoredFields = null; + Map ignoredFields = new LinkedHashMap<>(); ClassObj superClassObj = classInstance.getClassObj(); - Boolean alwaysIgnoreClassHierarchy = null; + Exclusion classExclusion = null; while (superClassObj != null) { - Boolean alwaysIgnoreClass = excludedRefs.classNames.get(superClassObj.getClassName()); - if (alwaysIgnoreClass != null) { + Exclusion params = excludedRefs.classNames.get(superClassObj.getClassName()); + if (params != null) { // true overrides null or false. - if (alwaysIgnoreClassHierarchy == null || !alwaysIgnoreClassHierarchy) { - alwaysIgnoreClassHierarchy = alwaysIgnoreClass; + if (classExclusion == null || !classExclusion.alwaysExclude) { + classExclusion = params; } } - Map classIgnoredFields = + Map classIgnoredFields = excludedRefs.fieldNameByClassName.get(superClassObj.getClassName()); if (classIgnoredFields != null) { - if (ignoredFields == null) { - ignoredFields = new LinkedHashMap<>(); - } ignoredFields.putAll(classIgnoredFields); } superClassObj = superClassObj.getSuperClassObj(); } - if (alwaysIgnoreClassHierarchy != null && alwaysIgnoreClassHierarchy) { + if (classExclusion != null && classExclusion.alwaysExclude) { return; } for (ClassInstance.FieldValue fieldValue : classInstance.getValues()) { + Exclusion fieldExclusion = classExclusion; Field field = fieldValue.getField(); if (field.getType() != Type.OBJECT) { continue; } Instance child = (Instance) fieldValue.getValue(); - boolean visit = true; - boolean visitIfNoPath = false; - // We don't even get here if alwaysIgnoreClassHierarchy is false. - if (alwaysIgnoreClassHierarchy != null) { - visit = false; - visitIfNoPath = true; - } String fieldName = field.getName(); - if (ignoredFields != null) { - Boolean alwaysIgnore = ignoredFields.get(fieldName); - if (alwaysIgnore != null) { - visit = false; - visitIfNoPath = !alwaysIgnore; - } - } - if (visit || visitIfNoPath) { - enqueue(visit, node, child, fieldName, INSTANCE_FIELD); + Exclusion params = ignoredFields.get(fieldName); + // If we found a field exclusion and it's stronger than a class exclusion + if (params != null && (fieldExclusion == null || (params.alwaysExclude + && !fieldExclusion.alwaysExclude))) { + fieldExclusion = params; } + enqueue(fieldExclusion, node, child, fieldName, INSTANCE_FIELD); } } @@ -309,12 +298,12 @@ private void visitArrayInstance(LeakNode node) { Object[] values = arrayInstance.getValues(); for (int i = 0; i < values.length; i++) { Instance child = (Instance) values[i]; - enqueue(true, node, child, "[" + i + "]", ARRAY_ENTRY); + enqueue(null, node, child, "[" + i + "]", ARRAY_ENTRY); } } } - private void enqueue(boolean visitNow, LeakNode parent, Instance child, String referenceName, + private void enqueue(Exclusion exclusion, LeakNode parent, Instance child, String referenceName, LeakTraceElement.Type referenceType) { if (child == null) { return; @@ -326,6 +315,7 @@ private void enqueue(boolean visitNow, LeakNode parent, Instance child, String r if (toVisitSet.contains(child)) { return; } + boolean visitNow = exclusion == null; if (!visitNow && toVisitIfNoPathSet.contains(child)) { return; } @@ -335,7 +325,7 @@ private void enqueue(boolean visitNow, LeakNode parent, Instance child, String r if (visitedSet.contains(child)) { return; } - LeakNode childNode = new LeakNode(child, parent, referenceName, referenceType); + LeakNode childNode = new LeakNode(exclusion, child, parent, referenceName, referenceType); if (visitNow) { toVisitSet.add(child); toVisitQueue.add(childNode); diff --git a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java index df8a9fd2f5..ad43d493f1 100644 --- a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java +++ b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java @@ -16,8 +16,8 @@ package com.squareup.leakcanary; import java.lang.ref.WeakReference; -import java.util.Arrays; import java.util.Collection; +import java.util.List; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -29,6 +29,7 @@ import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_MPREVIEW2; import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_M_POSTPREVIEW2; import static com.squareup.leakcanary.TestUtil.analyze; +import static java.util.Arrays.asList; import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -44,7 +45,7 @@ public class AsyncTaskLeakTest { static final String EXECUTOR_FIELD_2 = "sDefaultExecutor"; @Parameterized.Parameters public static Collection data() { - return Arrays.asList(new Object[][] { + return asList(new Object[][] { { ASYNC_TASK }, // { ASYNC_TASK_MPREVIEW2 }, // { ASYNC_TASK_M_POSTPREVIEW2 } // @@ -52,15 +53,17 @@ public class AsyncTaskLeakTest { } private final TestUtil.HeapDumpFile heapDumpFile; - ExcludedRefs.Builder excludedRefs; + ExcludedRefs.BuilderWithParams excludedRefs; public AsyncTaskLeakTest(TestUtil.HeapDumpFile heapDumpFile) { this.heapDumpFile = heapDumpFile; } @Before public void setUp() { - excludedRefs = new ExcludedRefs.Builder().clazz(WeakReference.class.getName(), true) - .clazz("java.lang.ref.FinalizerReference", true); + excludedRefs = new ExcludedRefs.BuilderWithParams().clazz(WeakReference.class.getName()) + .alwaysExclude() + .clazz("java.lang.ref.FinalizerReference") + .alwaysExclude(); } @Test public void leakFound() { @@ -86,11 +89,17 @@ public AsyncTaskLeakTest(TestUtil.HeapDumpFile heapDumpFile) { } @Test public void excludeStatic() { - excludedRefs.thread(ASYNC_TASK_THREAD); - excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_1); - excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_2); + excludedRefs.thread(ASYNC_TASK_THREAD).named(ASYNC_TASK_THREAD); + excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_1).named(EXECUTOR_FIELD_1); + excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_2).named(EXECUTOR_FIELD_2); AnalysisResult result = analyze(heapDumpFile, excludedRefs); assertTrue(result.leakFound); assertTrue(result.excludedLeak); + LeakTrace leakTrace = result.leakTrace; + List elements = leakTrace.elements; + Exclusion exclusion = elements.get(0).exclusion; + + List expectedExclusions = asList(ASYNC_TASK_THREAD, EXECUTOR_FIELD_1, EXECUTOR_FIELD_2); + assertTrue(expectedExclusions.contains(exclusion.name)); } } diff --git a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/RetainedSizeTest.java b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/RetainedSizeTest.java index d3f2cd4cea..a414687dee 100644 --- a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/RetainedSizeTest.java +++ b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/RetainedSizeTest.java @@ -34,7 +34,7 @@ public class RetainedSizeTest { private final TestUtil.HeapDumpFile heapDumpFile; private final long expectedRetainedHeapSize; - ExcludedRefs.Builder excludedRefs; + ExcludedRefs.BuilderWithParams excludedRefs; public RetainedSizeTest(TestUtil.HeapDumpFile heapDumpFile, long expectedRetainedHeapSize) { this.heapDumpFile = heapDumpFile; @@ -42,8 +42,10 @@ public RetainedSizeTest(TestUtil.HeapDumpFile heapDumpFile, long expectedRetaine } @Before public void setUp() { - excludedRefs = new ExcludedRefs.Builder().clazz(WeakReference.class.getName(), true) - .clazz("java.lang.ref.FinalizerReference", true); + excludedRefs = new ExcludedRefs.BuilderWithParams().clazz(WeakReference.class.getName()) + .alwaysExclude() + .clazz("java.lang.ref.FinalizerReference") + .alwaysExclude(); } @Test public void leakFound() { diff --git a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java index 1851ce755d..a54feeb1fd 100644 --- a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java +++ b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java @@ -36,15 +36,17 @@ */ public class ServiceBinderLeakTest { - ExcludedRefs.Builder excludedRefs; + ExcludedRefs.BuilderWithParams excludedRefs; @Before public void setUp() { - excludedRefs = new ExcludedRefs.Builder().clazz(WeakReference.class.getName(), true) - .clazz("java.lang.ref.FinalizerReference", true); + excludedRefs = new ExcludedRefs.BuilderWithParams().clazz(WeakReference.class.getName()) + .alwaysExclude() + .clazz("java.lang.ref.FinalizerReference") + .alwaysExclude(); } @Test public void realBinderLeak() { - excludedRefs.rootSuperClass("android.os.Binder", true); + excludedRefs.rootClass("android.os.Binder").alwaysExclude(); AnalysisResult result = analyze(SERVICE_BINDER, excludedRefs); @@ -57,7 +59,7 @@ public class ServiceBinderLeakTest { } @Test public void ignorableBinderLeak() { - excludedRefs.rootSuperClass("android.os.Binder", false); + excludedRefs.rootClass("android.os.Binder"); AnalysisResult result = analyze(SERVICE_BINDER_IGNORED, excludedRefs); @@ -70,7 +72,7 @@ public class ServiceBinderLeakTest { } @Test public void alwaysIgnorableBinderLeak() { - excludedRefs.rootSuperClass("android.os.Binder", true); + excludedRefs.rootClass("android.os.Binder").alwaysExclude(); AnalysisResult result = analyze(SERVICE_BINDER_IGNORED, excludedRefs); diff --git a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/TestUtil.java b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/TestUtil.java index d58d0aa332..e3666e5d62 100644 --- a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/TestUtil.java +++ b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/TestUtil.java @@ -46,7 +46,7 @@ static File fileFromName(String filename) { return new File(url.getPath()); } - static AnalysisResult analyze(HeapDumpFile heapDumpFile, ExcludedRefs.Builder excludedRefs) { + static AnalysisResult analyze(HeapDumpFile heapDumpFile, ExcludedRefs.BuilderWithParams excludedRefs) { File file = fileFromName(heapDumpFile.filename); String referenceKey = heapDumpFile.referenceKey; HeapAnalyzer heapAnalyzer = new HeapAnalyzer(excludedRefs.build()); diff --git a/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java b/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java index 0db6e64c67..236c716096 100644 --- a/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java +++ b/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java @@ -52,238 +52,246 @@ public enum AndroidExcludedRefs { ACTIVITY_CLIENT_RECORD__NEXT_IDLE(SDK_INT >= KITKAT && SDK_INT <= LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // Android AOSP sometimes keeps a reference to a destroyed activity as a "nextIdle" client - // record in the android.app.ActivityThread.mActivities map. - // Not sure what's going on there, input welcome. - excluded.instanceField("android.app.ActivityThread$ActivityClientRecord", "nextIdle"); + excluded.instanceField("android.app.ActivityThread$ActivityClientRecord", "nextIdle") + .reason("Android AOSP sometimes keeps a reference to a destroyed activity as a" + + " nextIdle client record in the android.app.ActivityThread.mActivities map." + + " Not sure what's going on there, input welcome."); } }, SPAN_CONTROLLER(SDK_INT <= KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // Editor inserts a special span, which has a reference to the EditText. That span is a - // NoCopySpan, which makes sure it gets dropped when creating a new SpannableStringBuilder - // from a given CharSequence. - // TextView.onSaveInstanceState() does a copy of its mText before saving it in the bundle. - // Prior to KitKat, that copy was done using the SpannableString constructor, instead of - // SpannableStringBuilder. The SpannableString constructor does not drop NoCopySpan spans. - // So we end up with a saved state that holds a reference to the textview and therefore the - // entire view hierarchy & activity context. - // Fix: https://github.com/android/platform_frameworks_base/commit - // /af7dcdf35a37d7a7dbaad7d9869c1c91bce2272b - - // Hack: to fix this, you could override TextView.onSaveInstanceState(), and then use - // reflection to access TextView.SavedState.mText and clear the NoCopySpan spans. - excluded.instanceField("android.widget.Editor$EasyEditSpanController", "this$0"); - excluded.instanceField("android.widget.Editor$SpanController", "this$0"); + String reason = + "Editor inserts a special span, which has a reference to the EditText. That span is a" + + " NoCopySpan, which makes sure it gets dropped when creating a new" + + " SpannableStringBuilder from a given CharSequence." + + " TextView.onSaveInstanceState() does a copy of its mText before saving it in the" + + " bundle. Prior to KitKat, that copy was done using the SpannableString" + + " constructor, instead of SpannableStringBuilder. The SpannableString constructor" + + " does not drop NoCopySpan spans. So we end up with a saved state that holds a" + + " reference to the textview and therefore the entire view hierarchy & activity" + + " context. Fix: https://github.com/android/platform_frameworks_base/commit" + + "/af7dcdf35a37d7a7dbaad7d9869c1c91bce2272b ." + + " To fix this, you could override TextView.onSaveInstanceState(), and then use" + + " reflection to access TextView.SavedState.mText and clear the NoCopySpan spans."; + excluded.instanceField("android.widget.Editor$EasyEditSpanController", "this$0") + .reason(reason); + excluded.instanceField("android.widget.Editor$SpanController", "this$0").reason(reason); } }, MEDIA_SESSION_LEGACY_HELPER__SINSTANCE(SDK_INT == LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // MediaSessionLegacyHelper is a static singleton that is lazily instantiated and keeps a - // reference to the context it's given the first time MediaSessionLegacyHelper.getHelper() - // is called. - // This leak was introduced in android-5.0.1_r1 and fixed in Android 5.1.0_r1 by calling - // context.getApplicationContext(). - // Fix: https://github.com/android/platform_frameworks_base/commit - // /9b5257c9c99c4cb541d8e8e78fb04f008b1a9091 - - // Hack: to fix this, you could call MediaSessionLegacyHelper.getHelper() early in - // Application.onCreate() and pass it the application context. - excluded.staticField("android.media.session.MediaSessionLegacyHelper", "sInstance"); + excluded.staticField("android.media.session.MediaSessionLegacyHelper", "sInstance") + .reason("MediaSessionLegacyHelper is a static singleton that is lazily instantiated and" + + " keeps a reference to the context it's given the first time" + + " MediaSessionLegacyHelper.getHelper() is called." + + " This leak was introduced in android-5.0.1_r1 and fixed in Android 5.1.0_r1 by" + + " calling context.getApplicationContext()." + + " Fix: https://github.com/android/platform_frameworks_base/commit" + + "/9b5257c9c99c4cb541d8e8e78fb04f008b1a9091" + + " To fix this, you could call MediaSessionLegacyHelper.getHelper() early" + + " in Application.onCreate() and pass it the application context."); } }, TEXT_LINE__SCACHED(SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // TextLine.sCached is a pool of 3 TextLine instances. TextLine.recycle() has had at least two - // bugs that created memory leaks by not correctly clearing the recycled TextLine instances. - // The first was fixed in android-5.1.0_r1: - // https://github.com/android/platform_frameworks_base/commit - // /893d6fe48d37f71e683f722457bea646994a10bf - - // The second was fixed, not released yet: - // https://github.com/android/platform_frameworks_base/commit - // /b3a9bc038d3a218b1dbdf7b5668e3d6c12be5ee4 - - // Hack: to fix this, you could access TextLine.sCached and clear the pool every now and then - // (e.g. on activity destroy). - excluded.staticField("android.text.TextLine", "sCached"); + excluded.staticField("android.text.TextLine", "sCached") + .reason("TextLine.sCached is a pool of 3 TextLine instances. TextLine.recycle() has had" + + " at least two bugs that created memory leaks by not correctly clearing the" + + " recycled TextLine instances. The first was fixed in android-5.1.0_r1:" + + " https://github.com/android/platform_frameworks_base/commit" + + "/893d6fe48d37f71e683f722457bea646994a10" + + " The second was fixed, not released yet:" + + " https://github.com/android/platform_frameworks_base/commit" + + "/b3a9bc038d3a218b1dbdf7b5668e3d6c12be5e" + + " To fix this, you could access TextLine.sCached and clear the pool every now" + + " and then (e.g. on activity destroy)."); } }, BLOCKING_QUEUE(SDK_INT < LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // Prior to ART, a thread waiting on a blocking queue will leak the last dequeued object - // as a stack local reference. - // So when a HandlerThread becomes idle, it keeps a local reference to the last message it - // received. That message then gets recycled and can be used again. - // As long as all messages are recycled after being used, this won't be a problem, because - // there references are cleared when being recycled. - // However, dialogs create template Message instances to be copied when a message needs to be - // sent. These Message templates holds references to the dialog listeners, which most likely - // leads to holding a reference onto the activity in some way. Dialogs never recycle their - // template Message, assuming these Message instances will get GCed when the dialog is GCed. - // The combination of these two things creates a high potential for memory leaks as soon - // as you use dialogs. These memory leaks might be temporary, but some handler threads sleep - // for a long time. - - // Hack: to fix this, you could post empty messages to the idle handler threads from time to - // time. This won't be easy because you cannot access all handler threads, but a library - // that is widely used should consider doing this for its own handler threads. - excluded.instanceField("android.os.Message", "obj"); - excluded.instanceField("android.os.Message", "next"); - excluded.instanceField("android.os.Message", "target"); + String reason = "Prior to ART, a thread waiting on a blocking queue will leak the last" + + " dequeued object as a stack local reference. So when a HandlerThread becomes idle, it" + + " keeps a local reference to the last message it received. That message then gets" + + " recycled and can be used again. As long as all messages are recycled after being" + + "used, this won't be a problem, because these references are cleared when being" + + "recycled. However, dialogs create template Message instances to be copied when a" + + "message needs to be sent. These Message templates holds references to the dialog" + + "listeners, which most likely leads to holding a reference onto the activity in some" + + "way. Dialogs never recycle their template Message, assuming these Message instances" + + " will get GCed when the dialog is GCed." + + " The combination of these two things creates a high potential for memory leaks as soon" + + " as you use dialogs. These memory leaks might be temporary, but some handler threads" + + " sleep for a long time." + + " To fix this, you could post empty messages to the idle handler threads from time to" + + " time. This won't be easy because you cannot access all handler threads, but a library" + + "that is widely used should consider doing this for its own handler threads."; + excluded.instanceField("android.os.Message", "obj").reason(reason); + excluded.instanceField("android.os.Message", "next").reason(reason); + excluded.instanceField("android.os.Message", "target").reason(reason); } }, INPUT_METHOD_MANAGER__SERVED_VIEW(SDK_INT >= ICE_CREAM_SANDWICH_MR1 && SDK_INT <= M) { @Override void add(ExcludedRefs.Builder excluded) { - // When we detach a view that receives keyboard input, the InputMethodManager leaks a - // reference to it until a new view asks for keyboard input. - // Tracked here: https://code.google.com/p/android/issues/detail?id=171190 - // Hack: https://gist.github.com/pyricau/4df64341cc978a7de414 - excluded.instanceField("android.view.inputmethod.InputMethodManager", "mNextServedView"); - excluded.instanceField("android.view.inputmethod.InputMethodManager", "mServedView"); + String reason = "When we detach a view that receives keyboard input, the InputMethodManager" + + " leaks a reference to it until a new view asks for keyboard input." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=171190" + + " Hack: https://gist.github.com/pyricau/4df64341cc978a7de414"; + excluded.instanceField("android.view.inputmethod.InputMethodManager", "mNextServedView") + .reason(reason); + excluded.instanceField("android.view.inputmethod.InputMethodManager", "mServedView") + .reason(reason); excluded.instanceField("android.view.inputmethod.InputMethodManager", - "mServedInputConnection"); + "mServedInputConnection").reason(reason); } }, INPUT_METHOD_MANAGER__ROOT_VIEW(SDK_INT >= ICE_CREAM_SANDWICH_MR1 && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // The singleton InputMethodManager is holding a reference to mCurRootView long after the - // activity has been destroyed. - // Observed on ICS MR1: https://github.com/square/leakcanary/issues/1#issuecomment-100579429 - // Hack: https://gist.github.com/pyricau/4df64341cc978a7de414 - excluded.instanceField("android.view.inputmethod.InputMethodManager", "mCurRootView"); + excluded.instanceField("android.view.inputmethod.InputMethodManager", "mCurRootView") + .reason("The singleton InputMethodManager is holding a reference to mCurRootView long" + + " after the activity has been destroyed." + + " Observed on ICS MR1: https://github.com/square/leakcanary/issues/1" + + "#issuecomment-100579429" + + " Hack: https://gist.github.com/pyricau/4df64341cc978a7de414"); } }, LAYOUT_TRANSITION(SDK_INT >= ICE_CREAM_SANDWICH && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // LayoutTransition leaks parent ViewGroup through ViewTreeObserver.OnPreDrawListener - // When triggered, this leaks stays until the window is destroyed. - // Tracked here: https://code.google.com/p/android/issues/detail?id=171830 - excluded.instanceField("android.animation.LayoutTransition$1", "val$parent"); + excluded.instanceField("android.animation.LayoutTransition$1", "val$parent") + .reason("LayoutTransition leaks parent ViewGroup through" + + " ViewTreeObserver.OnPreDrawListener When triggered, this leaks stays until the" + + " window is destroyed. Tracked here:" + + " https://code.google.com/p/android/issues/detail?id=171830"); } }, SPELL_CHECKER_SESSION(SDK_INT >= JELLY_BEAN && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // SpellCheckerSessionListenerImpl.mHandler is leaking destroyed Activity when the - // SpellCheckerSession is closed before the service is connected. - // Tracked here: https://code.google.com/p/android/issues/detail?id=172542 - excluded.instanceField("android.view.textservice.SpellCheckerSession$1", "this$0"); + excluded.instanceField("android.view.textservice.SpellCheckerSession$1", "this$0") + .reason("SpellCheckerSessionListenerImpl.mHandler is leaking destroyed Activity when the" + + " SpellCheckerSession is closed before the service is connected." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=172542"); } }, ACTIVITY_CHOOSE_MODEL(SDK_INT > ICE_CREAM_SANDWICH && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // ActivityChooserModel holds a static reference to the last set ActivityChooserModelPolicy - // which can be an activity context. - // Tracked here : https://code.google.com/p/android/issues/detail?id=172659 - // Hack : https://gist.github.com/andaag/b05ab66ed0f06167d6e0 + String reason = "ActivityChooserModel holds a static reference to the last set" + + " ActivityChooserModelPolicy which can be an activity context." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=172659" + + " Hack: https://gist.github.com/andaag/b05ab66ed0f06167d6e0"; excluded.instanceField("android.support.v7.internal.widget.ActivityChooserModel", - "mActivityChoserModelPolicy"); - excluded.instanceField("android.widget.ActivityChooserModel", "mActivityChoserModelPolicy"); + "mActivityChoserModelPolicy").reason(reason); + excluded.instanceField("android.widget.ActivityChooserModel", "mActivityChoserModelPolicy") + .reason(reason); } }, SPEECH_RECOGNIZER(SDK_INT < LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // Prior to Android 5, SpeechRecognizer.InternalListener was a non static inner class and - // leaked the SpeechRecognizer which leaked an activity context. - // Fixed in AOSP: https://github.com/android/platform_frameworks_base/commit - // /b37866db469e81aca534ff6186bdafd44352329b - excluded.instanceField("android.speech.SpeechRecognizer$InternalListener", "this$0"); + excluded.instanceField("android.speech.SpeechRecognizer$InternalListener", "this$0") + .reason("Prior to Android 5, SpeechRecognizer.InternalListener was a non static inner" + + " class and leaked the SpeechRecognizer which leaked an activity context." + + " Fixed in AOSP: https://github.com/android/platform_frameworks_base/commit" + + " /b37866db469e81aca534ff6186bdafd44352329b"); } }, ACCOUNT_MANAGER(SDK_INT > ECLAIR && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // AccountManager$AmsTask$Response is a stub and is held in memory by native code, probably - // because the reference to the response in the other process hasn't been cleared. - // AccountManager$AmsTask is holding on to the activity reference to use for launching a new - // sub- Activity. - // Tracked here: https://code.google.com/p/android/issues/detail?id=173689 - // Fix: Pass a null activity reference to the AccountManager methods and then deal with the - // returned future to to get the result and correctly start an activity when it's available. - excluded.instanceField("android.accounts.AccountManager$AmsTask$Response", "this$1"); + excluded.instanceField("android.accounts.AccountManager$AmsTask$Response", "this$1") + .reason("AccountManager$AmsTask$Response is a stub and is held in memory by native code," + + " probably because the reference to the response in the other process hasn't been" + + " cleared." + + " AccountManager$AmsTask is holding on to the activity reference to use for" + + " launching a new sub- Activity." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=173689" + + " Fix: Pass a null activity reference to the AccountManager methods and then deal" + + " with the returned future to to get the result and correctly start an activity" + + " when it's available."); } }, MEDIA_SCANNER_CONNECTION(SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // The static method MediaScannerConnection.scanFile() takes an activity context but the - // service might not disconnect after the activity has been destroyed. - // Tracked here: https://code.google.com/p/android/issues/detail?id=173788 - // Fix: Create an instance of MediaScannerConnection yourself and pass in the application - // context. Call connect() and disconnect() manually. - excluded.instanceField("android.media.MediaScannerConnection", "mContext"); + excluded.instanceField("android.media.MediaScannerConnection", "mContext") + .reason("The static method MediaScannerConnection.scanFile() takes an activity context" + + " but the service might not disconnect after the activity has been destroyed." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=173788" + + " Fix: Create an instance of MediaScannerConnection yourself and pass in the" + + " application context. Call connect() and disconnect() manually."); } }, USER_MANAGER__SINSTANCE(SDK_INT >= JELLY_BEAN && SDK_INT <= LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // UserManager has a static sInstance field that creates an instance and caches it the first - // time UserManager.get() is called. This instance is created with the outer context (which - // is an activity base context). - // Tracked here: https://code.google.com/p/android/issues/detail?id=173789 - // Introduced by: https://github.com/android/platform_frameworks_base/commit - // /27db46850b708070452c0ce49daf5f79503fbde6 - // Fix: trigger a call to UserManager.get() in Application.onCreate(), so that the - // UserManager instance gets cached with a reference to the application context. - excluded.instanceField("android.os.UserManager", "mContext"); + excluded.instanceField("android.os.UserManager", "mContext") + .reason("UserManager has a static sInstance field that creates an instance and caches it" + + " the first time UserManager.get() is called. This instance is created with the" + + " outer context (which is an activity base context)." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=173789" + + " Introduced by: https://github.com/android/platform_frameworks_base/commit" + + "/27db46850b708070452c0ce49daf5f79503fbde6" + + " Fix: trigger a call to UserManager.get() in Application.onCreate(), so that the" + + " UserManager instance gets cached with a reference to the application context."); } }, APP_WIDGET_HOST_CALLBACKS(SDK_INT < LOLLIPOP_MR1) { @Override void add(ExcludedRefs.Builder excluded) { - // android.appwidget.AppWidgetHost$Callbacks is a stub and is held in memory native code - // The reference to the `mContext` was not being cleared, which caused the Callbacks instance to retain this reference - // Fixed in AOSP: https://github.com/android/platform_frameworks_base/commit/7a96f3c917e0001ee739b65da37b2fadec7d7765 - excluded.instanceField("android.appwidget.AppWidgetHost$Callbacks", "this$0"); + excluded.instanceField("android.appwidget.AppWidgetHost$Callbacks", "this$0") + .reason("android.appwidget.AppWidgetHost$Callbacks is a stub and is held in memory native" + + " code. The reference to the `mContext` was not being cleared, which caused the" + + " Callbacks instance to retain this reference" + + " Fixed in AOSP: https://github.com/android/platform_frameworks_base/commit" + + "/7a96f3c917e0001ee739b65da37b2fadec7d7765"); } }, DEVICE_POLICY_MANAGER__SETTINGS_OBSERVER(MOTOROLA.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { if (MOTOROLA.equals(MANUFACTURER) && SDK_INT == KITKAT) { - // DevicePolicyManager keeps a reference to the context it has been created with instead of - // extracting the application context. In this Motorola build, DevicePolicyManager has an - // inner SettingsObserver class that is a content observer, which is held into memory - // by a binder transport object. - excluded.instanceField("android.app.admin.DevicePolicyManager$SettingsObserver", "this$0"); + excluded.instanceField("android.app.admin.DevicePolicyManager$SettingsObserver", "this$0") + .reason("DevicePolicyManager keeps a reference to the context it has been created with" + + " instead of extracting the application context. In this Motorola build," + + " DevicePolicyManager has an inner SettingsObserver class that is a content" + + " observer, which is held into memory by a binder transport object."); } } }, SPEN_GESTURE_MANAGER(SAMSUNG.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // SpenGestureManager has a static mContext field that leaks a reference to the activity. - // Yes, a STATIC "mContext" field. - excluded.staticField("com.samsung.android.smartclip.SpenGestureManager", "mContext"); + excluded.staticField("com.samsung.android.smartclip.SpenGestureManager", "mContext") + .reason("SpenGestureManager has a static mContext field that leaks a reference to the" + + " activity. Yes, a STATIC mContext field."); } }, CLIPBOARD_UI_MANAGER__SINSTANCE( SAMSUNG.equals(MANUFACTURER) && SDK_INT >= KITKAT && SDK_INT <= LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // ClipboardUIManager is a static singleton that leaks an activity context. - // Fix: trigger a call to ClipboardUIManager.getInstance() in Application.onCreate(), so - // that the ClipboardUIManager instance gets cached with a reference to the - // application context. Example: https://gist.github.com/pepyakin/8d2221501fd572d4a61c - excluded.instanceField("android.sec.clipboard.ClipboardUIManager", "mContext"); + excluded.instanceField("android.sec.clipboard.ClipboardUIManager", "mContext") + .reason("ClipboardUIManager is a static singleton that leaks an activity context." + + " Fix: trigger a call to ClipboardUIManager.getInstance() in Application.onCreate()" + + " , so that the ClipboardUIManager instance gets cached with a reference to the" + + " application context. Example: https://gist.github.com/pepyakin" + + "/8d2221501fd572d4a61c"); } }, BUBBLE_POPUP_HELPER__SHELPER( LG.equals(MANUFACTURER) && SDK_INT >= KITKAT && SDK_INT <= LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // A static helper for EditText "bubble popups" leaks a reference to the latest focused view. - excluded.staticField("android.widget.BubblePopupHelper", "sHelper"); + excluded.staticField("android.widget.BubblePopupHelper", "sHelper") + .reason("A static helper for EditText bubble popups leaks a reference to the latest" + + "focused view."); } }, @@ -298,56 +306,72 @@ public enum AndroidExcludedRefs { MAPPER_CLIENT(NVIDIA.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // Not sure exactly what ControllerMapper is about, but there is an anonymous Handler in - // ControllerMapper.MapperClient.ServiceClient, which leaks ControllerMapper.MapperClient - // which leaks the activity context. - excluded.instanceField("com.nvidia.ControllerMapper.MapperClient$ServiceClient", "this$0"); + excluded.instanceField("com.nvidia.ControllerMapper.MapperClient$ServiceClient", "this$0") + .reason("Not sure exactly what ControllerMapper is about, but there is an anonymous" + + " Handler in ControllerMapper.MapperClient.ServiceClient, which leaks" + + " ControllerMapper.MapperClient which leaks the activity context."); } }, TEXT_VIEW__MLAST_HOVERED_VIEW( SAMSUNG.equals(MANUFACTURER) && SDK_INT >= KITKAT && SDK_INT <= LOLLIPOP) { @Override void add(ExcludedRefs.Builder excluded) { - // mLastHoveredView is a static field in TextView that leaks the last hovered view. - excluded.staticField("android.widget.TextView", "mLastHoveredView"); + excluded.staticField("android.widget.TextView", "mLastHoveredView") + .reason("mLastHoveredView is a static field in TextView that leaks the last hovered" + + " view."); } }, PERSONA_MANAGER(SAMSUNG.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // android.app.LoadedApk.mResources has a reference to - // android.content.res.Resources.mPersonaManager which has a reference to - // android.os.PersonaManager.mContext which is an activity. - excluded.instanceField("android.os.PersonaManager", "mContext"); + excluded.instanceField("android.os.PersonaManager", "mContext") + .reason("android.app.LoadedApk.mResources has a reference to" + + " android.content.res.Resources.mPersonaManager which has a reference to" + + " android.os.PersonaManager.mContext which is an activity."); } }, RESOURCES__MCONTEXT(SAMSUNG.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // In AOSP the Resources class does not have a context. - // Here we have ZygoteInit.mResources (static field) holding on to a Resources instance that - // has a context that is the activity. - // Observed here: https://github.com/square/leakcanary/issues/1#issue-74450184 - excluded.instanceField("android.content.res.Resources", "mContext"); + excluded.instanceField("android.content.res.Resources", "mContext") + .reason("In AOSP the Resources class does not have a context." + + " Here we have ZygoteInit.mResources (static field) holding on to a Resources" + + " instance that has a context that is the activity." + + " Observed here: https://github.com/square/leakcanary/issues/1#issue-74450184"); } }, VIEW_CONFIGURATION__MCONTEXT(SAMSUNG.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // In AOSP the ViewConfiguration class does not have a context. - // Here we have ViewConfiguration.sConfigurations (static field) holding on to a - // ViewConfiguration instance that has a context that is the activity. - // Observed here: https://github.com/square/leakcanary/issues/1#issuecomment-100324683 - excluded.instanceField("android.view.ViewConfiguration", "mContext"); + excluded.instanceField("android.view.ViewConfiguration", "mContext") + .reason("In AOSP the ViewConfiguration class does not have a context." + + " Here we have ViewConfiguration.sConfigurations (static field) holding on to a" + + " ViewConfiguration instance that has a context that is the activity." + + " Observed here: https://github.com/square/leakcanary/issues" + + "/1#issuecomment-100324683"); } }, AUDIO_MANAGER__MCONTEXT_STATIC(SAMSUNG.equals(MANUFACTURER) && SDK_INT == KITKAT) { @Override void add(ExcludedRefs.Builder excluded) { - // Samsung added a static mContext_static field to AudioManager, holds a reference to the - // activity. - // Observed here: https://github.com/square/leakcanary/issues/32 - excluded.staticField("android.media.AudioManager", "mContext_static"); + excluded.staticField("android.media.AudioManager", "mContext_static") + .reason("Samsung added a static mContext_static field to AudioManager, holds a reference" + + " to the activity." + + " Observed here: https://github.com/square/leakcanary/issues/32"); + } + }, + + AUDIO_MANAGER(SDK_INT <= LOLLIPOP_MR1) { + @Override void add(ExcludedRefs.Builder excluded) { + excluded.instanceField("android.media.AudioManager$1", "this$0") + .reason("Prior to Android M, VideoView required audio focus from AudioManager and" + + " never abandoned it, which leaks the Activity context through the AudioManager." + + " The root of the problem is that AudioManager uses whichever" + + " context it receives, which in the case of the VideoView example is an Activity," + + " even though it only needs the application's context. The issue is fixed in" + + " Android M, and the AudioManager now uses the application's context." + + " Tracked here: https://code.google.com/p/android/issues/detail?id=152173" + + " Fix: https://gist.github.com/jankovd/891d96f476f7a9ce24e2"); } }, @@ -357,31 +381,17 @@ public enum AndroidExcludedRefs { // When you bind and unbind from a Service, the OS will keep a reference to the Binder // until the client binder has been GC'ed. This means the Binder can be retained after // Service.onDestroy() is called. - excluded.rootSuperClass("android.os.Binder", true); + excluded.rootClass("android.os.Binder").alwaysExclude(); } }, SOFT_REFERENCES { @Override void add(ExcludedRefs.Builder excluded) { - excluded.clazz(WeakReference.class.getName(), true); - excluded.clazz(SoftReference.class.getName(), true); - excluded.clazz(PhantomReference.class.getName(), true); - excluded.clazz("java.lang.ref.Finalizer", true); - excluded.clazz("java.lang.ref.FinalizerReference", true); - } - }, - - AUDIO_MANAGER(SDK_INT <= LOLLIPOP_MR1) { - @Override void add(ExcludedRefs.Builder excluded) { - // Prior to Android M, VideoView required audio focus from AudioManager and - // never abandoned it, which leaks the Activity context through the AudioManager. - // The root of the problem is that AudioManager uses whichever - // context it receives, which in the case of the VideoView example is an Activity, - // even though it only needs the application's context. The issue is fixed in - // Android M, and the AudioManager now uses the application's context. - // Tracked here: https://code.google.com/p/android/issues/detail?id=152173 - // Fix: https://gist.github.com/jankovd/891d96f476f7a9ce24e2 - excluded.instanceField("android.media.AudioManager$1", "this$0"); + excluded.clazz(WeakReference.class.getName()).alwaysExclude(); + excluded.clazz(SoftReference.class.getName()).alwaysExclude(); + excluded.clazz(PhantomReference.class.getName()).alwaysExclude(); + excluded.clazz("java.lang.ref.Finalizer").alwaysExclude(); + excluded.clazz("java.lang.ref.FinalizerReference").alwaysExclude(); } }, @@ -389,7 +399,7 @@ public enum AndroidExcludedRefs { @Override void add(ExcludedRefs.Builder excluded) { // If the FinalizerWatchdogDaemon thread is on the shortest path, then there was no other // reference to the object and it was about to be GCed. - excluded.thread("FinalizerWatchdogDaemon", true); + excluded.thread("FinalizerWatchdogDaemon").alwaysExclude(); } }, @@ -398,13 +408,13 @@ public enum AndroidExcludedRefs { // The main thread stack is ever changing so local variables aren't likely to hold references // for long. If this is on the shortest path, it's probably that there's a longer path with // a real leak. - excluded.thread("main", true); + excluded.thread("main").alwaysExclude(); } }, LEAK_CANARY_THREAD { @Override void add(ExcludedRefs.Builder excluded) { - excluded.thread(LEAK_CANARY_THREAD_NAME, true); + excluded.thread(LEAK_CANARY_THREAD_NAME).alwaysExclude(); } }, @@ -415,7 +425,7 @@ public enum AndroidExcludedRefs { // The main thread message queue is held on by the main Looper, but that might be a longer // path. Let's not confuse people with a shorter path that is less meaningful. excluded.instanceField("android.view.Choreographer$FrameDisplayEventReceiver", - "mMessageQueue", true); + "mMessageQueue").alwaysExclude(); } }; @@ -423,8 +433,9 @@ public enum AndroidExcludedRefs { * This returns the references in the leak path that should be ignored by all on Android. */ public static ExcludedRefs.Builder createAndroidDefaults() { - return createBuilder(EnumSet.of(SOFT_REFERENCES, FINALIZER_WATCHDOG_DAEMON, MAIN, LEAK_CANARY_THREAD, - EVENT_RECEIVER__MMESSAGE_QUEUE, SERVICE_BINDER)); + return createBuilder( + EnumSet.of(SOFT_REFERENCES, FINALIZER_WATCHDOG_DAEMON, MAIN, LEAK_CANARY_THREAD, + EVENT_RECEIVER__MMESSAGE_QUEUE, SERVICE_BINDER)); } /** @@ -438,10 +449,11 @@ public static ExcludedRefs.Builder createAppDefaults() { } public static ExcludedRefs.Builder createBuilder(EnumSet refs) { - ExcludedRefs.Builder excluded = new ExcludedRefs.Builder(); + ExcludedRefs.Builder excluded = ExcludedRefs.builder(); for (AndroidExcludedRefs ref : refs) { if (ref.applies) { ref.add(excluded); + ((ExcludedRefs.BuilderWithParams) excluded).named(ref.name()); } } return excluded; diff --git a/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java b/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java index 39ffd8f3c7..69f02ba259 100644 --- a/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java +++ b/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java @@ -100,7 +100,7 @@ public static String leakInfo(Context context, HeapDump heapDump, AnalysisResult String detailedString = ""; if (result.leakFound) { if (result.excludedLeak) { - info += "* LEAK CAN BE IGNORED.\n"; + info += "* EXCLUDED LEAK.\n"; } info += "* " + result.className; if (!heapDump.referenceName.equals("")) { diff --git a/leakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakAdapter.java b/leakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakAdapter.java index cb16c7fecc..929e6badeb 100644 --- a/leakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakAdapter.java +++ b/leakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakAdapter.java @@ -22,6 +22,7 @@ import android.view.ViewGroup; import android.widget.BaseAdapter; import android.widget.TextView; +import com.squareup.leakcanary.Exclusion; import com.squareup.leakcanary.LeakTrace; import com.squareup.leakcanary.LeakTraceElement; import com.squareup.leakcanary.R; @@ -132,6 +133,23 @@ private String elementToHtmlString(LeakTraceElement element, boolean root, boole if (opened && element.extra != null) { htmlString += " " + element.extra + ""; } + + Exclusion exclusion = element.exclusion; + if (exclusion != null) { + if (opened) { + htmlString += "

Excluded by rule"; + if (exclusion.name != null) { + htmlString += " " + exclusion.name + ""; + } + htmlString += " matching " + exclusion.matching + ""; + if (exclusion.reason != null) { + htmlString += " because " + exclusion.reason + ""; + } + } else { + htmlString += " (excluded)"; + } + } + return htmlString; } diff --git a/leakcanary-android/src/main/res/values/leak_canary_strings.xml b/leakcanary-android/src/main/res/values/leak_canary_strings.xml index 458c1f4b4a..b53e29ce16 100644 --- a/leakcanary-android/src/main/res/values/leak_canary_strings.xml +++ b/leakcanary-android/src/main/res/values/leak_canary_strings.xml @@ -15,8 +15,8 @@ ~ limitations under the License. --> - %1$s has leaked %2$s - %1$s has leaked %2$s (excluded leak) + %1$s leaked %2$s + [Excluded] %1$s leaked %2$s Leak analysis failed Leaks in %s Click for more details diff --git a/leakcanary-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java index 9660eda278..47662445ec 100644 --- a/leakcanary-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java +++ b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java @@ -32,130 +32,178 @@ */ public final class ExcludedRefs implements Serializable { - public final Map> fieldNameByClassName; - public final Map> staticFieldNameByClassName; - public final Map threadNames; - public final Map classNames; - public final Map rootSuperClassNames; - - ExcludedRefs(Map> fieldNameByClassName, - Map> staticFieldNameByClassName, - Map threadNames, Map classNames, - Map rootSuperClassNames) { - // Copy + unmodifiable. - this.fieldNameByClassName = unmodifiableMap(new LinkedHashMap<>(fieldNameByClassName)); - this.staticFieldNameByClassName = - unmodifiableMap(new LinkedHashMap<>(staticFieldNameByClassName)); - this.threadNames = unmodifiableMap(new LinkedHashMap<>(threadNames)); - this.classNames = unmodifiableMap(new LinkedHashMap<>(classNames)); - this.rootSuperClassNames = unmodifiableMap(new LinkedHashMap<>(rootSuperClassNames)); + public static Builder builder() { + return new BuilderWithParams(); + } + + public final Map> fieldNameByClassName; + public final Map> staticFieldNameByClassName; + public final Map threadNames; + public final Map classNames; + public final Map rootClassNames; + + ExcludedRefs(BuilderWithParams builder) { + this.fieldNameByClassName = unmodifiableRefStringMap(builder.fieldNameByClassName); + this.staticFieldNameByClassName = unmodifiableRefStringMap(builder.staticFieldNameByClassName); + this.threadNames = unmodifiableRefMap(builder.threadNames); + this.classNames = unmodifiableRefMap(builder.classNames); + this.rootClassNames = unmodifiableRefMap(builder.rootClassNames); + } + + private Map> unmodifiableRefStringMap( + Map> mapmap) { + LinkedHashMap> fieldNameByClassName = new LinkedHashMap<>(); + for (Map.Entry> entry : mapmap.entrySet()) { + fieldNameByClassName.put(entry.getKey(), unmodifiableRefMap(entry.getValue())); + } + return unmodifiableMap(fieldNameByClassName); + } + + private Map unmodifiableRefMap(Map fieldBuilderMap) { + Map fieldMap = new LinkedHashMap<>(); + for (Map.Entry fieldEntry : fieldBuilderMap.entrySet()) { + fieldMap.put(fieldEntry.getKey(), new Exclusion(fieldEntry.getValue())); + } + return unmodifiableMap(fieldMap); } @Override public String toString() { String string = ""; - for (Map.Entry> classes : fieldNameByClassName.entrySet()) { + for (Map.Entry> classes : fieldNameByClassName.entrySet()) { String clazz = classes.getKey(); - for (Map.Entry field : classes.getValue().entrySet()) { - String always = field.getValue() ? " (always)" : ""; + for (Map.Entry field : classes.getValue().entrySet()) { + String always = field.getValue().alwaysExclude ? " (always)" : ""; string += "| Field: " + clazz + "." + field.getKey() + always + "\n"; } } - for (Map.Entry> classes : staticFieldNameByClassName.entrySet()) { + for (Map.Entry> classes : staticFieldNameByClassName.entrySet()) { String clazz = classes.getKey(); - for (Map.Entry field : classes.getValue().entrySet()) { - String always = field.getValue() ? " (always)" : ""; + for (Map.Entry field : classes.getValue().entrySet()) { + String always = field.getValue().alwaysExclude ? " (always)" : ""; string += "| Static field: " + clazz + "." + field.getKey() + always + "\n"; } } - for (Map.Entry thread : threadNames.entrySet()) { - String always = thread.getValue() ? " (always)" : ""; + for (Map.Entry thread : threadNames.entrySet()) { + String always = thread.getValue().alwaysExclude ? " (always)" : ""; string += "| Thread:" + thread.getKey() + always + "\n"; } - for (Map.Entry clazz : classNames.entrySet()) { - String always = clazz.getValue() ? " (always)" : ""; + for (Map.Entry clazz : classNames.entrySet()) { + String always = clazz.getValue().alwaysExclude ? " (always)" : ""; string += "| Class:" + clazz.getKey() + always + "\n"; } - for (Map.Entry clazz : rootSuperClassNames.entrySet()) { - String always = clazz.getValue() ? " (always)" : ""; + for (Map.Entry clazz : rootClassNames.entrySet()) { + String always = clazz.getValue().alwaysExclude ? " (always)" : ""; string += "| Root Class:" + clazz.getKey() + always + "\n"; } return string; } - public static final class Builder { - private final Map> fieldNameByClassName = new LinkedHashMap<>(); - private final Map> staticFieldNameByClassName = + static final class ParamsBuilder { + String name; + String reason; + boolean alwaysExclude; + final String matching; + + ParamsBuilder(String matching) { + this.matching = matching; + } + } + + public interface Builder { + BuilderWithParams instanceField(String className, String fieldName); + + BuilderWithParams staticField(String className, String fieldName); + + BuilderWithParams thread(String threadName); + + BuilderWithParams clazz(String className); + + BuilderWithParams rootClass(String rootSuperClassName); + + ExcludedRefs build(); + } + + public static final class BuilderWithParams implements Builder { + + private final Map> fieldNameByClassName = new LinkedHashMap<>(); - private final Map threadNames = new LinkedHashMap<>(); - private final Map classNames = new LinkedHashMap<>(); - private final Map rootSuperClassNames = new LinkedHashMap<>(); + private final Map> staticFieldNameByClassName = + new LinkedHashMap<>(); + private final Map threadNames = new LinkedHashMap<>(); + private final Map classNames = new LinkedHashMap<>(); + private final Map rootClassNames = new LinkedHashMap<>(); + + private ParamsBuilder lastParams; - public Builder instanceField(String className, String fieldName) { - return instanceField(className, fieldName, false); + BuilderWithParams() { } - public Builder instanceField(String className, String fieldName, boolean always) { + @Override public BuilderWithParams instanceField(String className, String fieldName) { checkNotNull(className, "className"); checkNotNull(fieldName, "fieldName"); - Map excludedFields = fieldNameByClassName.get(className); + Map excludedFields = fieldNameByClassName.get(className); if (excludedFields == null) { excludedFields = new LinkedHashMap<>(); fieldNameByClassName.put(className, excludedFields); } - excludedFields.put(fieldName, always); + lastParams = new ParamsBuilder("field " + className + "#" + fieldName); + excludedFields.put(fieldName, lastParams); return this; } - public Builder staticField(String className, String fieldName) { - return staticField(className, fieldName, false); - } - - public Builder staticField(String className, String fieldName, boolean always) { + @Override public BuilderWithParams staticField(String className, String fieldName) { checkNotNull(className, "className"); checkNotNull(fieldName, "fieldName"); - Map excludedFields = staticFieldNameByClassName.get(className); + Map excludedFields = staticFieldNameByClassName.get(className); if (excludedFields == null) { excludedFields = new LinkedHashMap<>(); staticFieldNameByClassName.put(className, excludedFields); } - excludedFields.put(fieldName, always); + lastParams = new ParamsBuilder("static field " + className + "#" + fieldName); + excludedFields.put(fieldName, lastParams); return this; } - public Builder thread(String threadName) { - return thread(threadName, false); + @Override public BuilderWithParams thread(String threadName) { + checkNotNull(threadName, "threadName"); + lastParams = new ParamsBuilder("any threads named " + threadName); + threadNames.put(threadName, lastParams); + return this; } - public Builder thread(String threadName, boolean always) { - checkNotNull(threadName, "threadName"); - threadNames.put(threadName, always); + /** Ignores all fields and static fields of all subclasses of the provided class name. */ + @Override public BuilderWithParams clazz(String className) { + checkNotNull(className, "className"); + lastParams = new ParamsBuilder("any subclass of " + className); + classNames.put(className, lastParams); return this; } - public Builder clazz(String className) { - return thread(className, false); + /** Ignores any GC root that belongs to a subclass of the provided class name. */ + @Override public BuilderWithParams rootClass(String rootClassName) { + checkNotNull(rootClassName, "rootClassName"); + lastParams = new ParamsBuilder("any GC root subclass of " + rootClassName); + rootClassNames.put(rootClassName, lastParams); + return this; } - public Builder clazz(String className, boolean always) { - checkNotNull(className, "className"); - classNames.put(className, always); + public BuilderWithParams named(String name) { + lastParams.name = name; return this; } - public Builder rootSuperClass(String rootSuperClassName) { - return rootSuperClass(rootSuperClassName, false); + public BuilderWithParams reason(String reason) { + lastParams.reason = reason; + return this; } - /** Ignores any GC root that is a subclass of the provided class name. */ - public Builder rootSuperClass(String rootSuperClassName, boolean always) { - checkNotNull(rootSuperClassName, "rootSuperClassName"); - rootSuperClassNames.put(rootSuperClassName, always); + public BuilderWithParams alwaysExclude() { + lastParams.alwaysExclude = true; return this; } public ExcludedRefs build() { - return new ExcludedRefs(fieldNameByClassName, staticFieldNameByClassName, threadNames, - classNames, rootSuperClassNames); + return new ExcludedRefs(this); } } } diff --git a/leakcanary-watcher/src/main/java/com/squareup/leakcanary/Exclusion.java b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/Exclusion.java new file mode 100644 index 0000000000..4d87d05e6c --- /dev/null +++ b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/Exclusion.java @@ -0,0 +1,17 @@ +package com.squareup.leakcanary; + +import java.io.Serializable; + +public final class Exclusion implements Serializable { + public final String name; + public final String reason; + public final boolean alwaysExclude; + public final String matching; + + Exclusion(ExcludedRefs.ParamsBuilder builder) { + this.name = builder.name; + this.reason = builder.reason; + this.alwaysExclude = builder.alwaysExclude; + this.matching = builder.matching; + } +} diff --git a/leakcanary-watcher/src/main/java/com/squareup/leakcanary/RefWatcher.java b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/RefWatcher.java index 7d2b9fc43f..5d124535ee 100644 --- a/leakcanary-watcher/src/main/java/com/squareup/leakcanary/RefWatcher.java +++ b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/RefWatcher.java @@ -48,7 +48,7 @@ public final class RefWatcher { }, new HeapDump.Listener() { @Override public void analyze(HeapDump heapDump) { } - }, new ExcludedRefs.Builder().build()); + }, new ExcludedRefs.BuilderWithParams().build()); private final Executor watchExecutor; private final DebuggerControl debuggerControl; diff --git a/leakcanary-watcher/src/test/java/com/squareup/leakcanary/RefWatcherTest.java b/leakcanary-watcher/src/test/java/com/squareup/leakcanary/RefWatcherTest.java index 31e6531700..15b44def50 100644 --- a/leakcanary-watcher/src/test/java/com/squareup/leakcanary/RefWatcherTest.java +++ b/leakcanary-watcher/src/test/java/com/squareup/leakcanary/RefWatcherTest.java @@ -24,7 +24,7 @@ public class RefWatcherTest { - static final ExcludedRefs NO_REF = new ExcludedRefs.Builder().build(); + static final ExcludedRefs NO_REF = new ExcludedRefs.BuilderWithParams().build(); static class TestDumper implements HeapDumper { boolean called;