Skip to content

Commit

Permalink
Add reason for ignoring to analysis result
Browse files Browse the repository at this point in the history 
WIP, DO NOT MERGE

Fixes #365
  • Loading branch information
@pyricau
pyricau committed Jan 7, 2016
1 parent 80421bc commit 0f7fa8c
Show file tree
Hide file tree
Showing 13 changed files with 272 additions and 205 deletions.
5 changes: 3 additions & 2 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private boolean isIgnoredDominator(Instance dominator, Instance instance) {
private LeakTrace buildLeakTrace(LeakNode leakingNode) {
List<LeakTraceElement> 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) {
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> fields;

LeakTraceElement(String referenceName, Type type, Holder holder, String className, String extra,
List<String> fields) {
Exclusion exclusion, List<String> 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));
}

Expand Down
111 changes: 49 additions & 62 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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:
Expand All @@ -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());
Expand All @@ -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, "<Java Local>", LOCAL);
LeakNode parent = new LeakNode(null, holder, null, null, null);
enqueue(exclusion, parent, child, "<Java Local>", 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<String, Boolean> ignoredStaticFields =
Map<String, Exclusion> ignoredStaticFields =
excludedRefs.staticFieldNameByClassName.get(classObj.getClassName());
for (Map.Entry<Field, Object> entry : classObj.getStaticFieldValues().entrySet()) {
Field field = entry.getKey();
Expand All @@ -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<String, Boolean> ignoredFields = null;
Map<String, Exclusion> 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<String, Boolean> classIgnoredFields =
Map<String, Exclusion> 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);
}
}

Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
15 changes: 11 additions & 4 deletions leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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<LeakTraceElement> elements = leakTrace.elements;
Exclusion exclusion = elements.get(0).exclusion;
assertEquals("Ignored Async Thread", exclusion.name);
}
}
8 changes: 5 additions & 3 deletions leakcanary-analyzer/src/test/java/com/squareup/leakcanary/RetainedSizeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,18 @@ 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;
this.expectedRetainedHeapSize = expectedRetainedHeapSize;
}

@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() {
Expand Down
14 changes: 8 additions & 6 deletions leakcanary-analyzer/src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down
Loading

0 comments on commit 0f7fa8c

Please sign in to comment.