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..65037c7c23 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) { @@ -279,7 +279,8 @@ private LeakTraceElement buildLeakElement(LeakNode node) { 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..dd8cb04c0d 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)); } 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..b5185dfc62 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,45 @@ 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); + 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 +231,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 +295,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 +312,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 +322,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..60434348ba 100644 --- a/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java +++ b/leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java @@ -18,6 +18,7 @@ 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; @@ -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,15 @@ public AsyncTaskLeakTest(TestUtil.HeapDumpFile heapDumpFile) { } @Test public void excludeStatic() { - excludedRefs.thread(ASYNC_TASK_THREAD); + excludedRefs.thread(ASYNC_TASK_THREAD).named("Ignored Async Thread"); excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_1); excludedRefs.staticField(ASYNC_TASK_CLASS, 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; + assertEquals("Ignored Async Thread", 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 ce33888c62..35f0ace4ab 100644 --- a/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java +++ b/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidExcludedRefs.java @@ -51,64 +51,62 @@ 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 ." + + " \n\nTo 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" + + " \n\nTo 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" + + " \n\nTo fix this, you could access TextLine.sCached and clear the pool every now" + + " and then (e.g. on activity destroy)."); } }, @@ -185,7 +183,7 @@ public enum AndroidExcludedRefs { // 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"); + "mActivityChoserModelPolicy"); excluded.instanceField("android.widget.ActivityChooserModel", "mActivityChoserModelPolicy"); } }, @@ -356,17 +354,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); + 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(); } }, @@ -388,7 +386,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(); } }, @@ -397,13 +395,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(); } }, @@ -414,7 +412,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(); } }; @@ -422,8 +420,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)); } /** @@ -437,10 +436,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-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/ExcludedRefs.java index 9660eda278..adc0ca0263 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,176 @@ */ 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); + } + + 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("GC Root 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("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("All fields 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("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..af0f695627 --- /dev/null +++ b/leakcanary-watcher/src/main/java/com/squareup/leakcanary/Exclusion.java @@ -0,0 +1,15 @@ +package com.squareup.leakcanary; + +public final class Exclusion { + 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;