Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add reason for ignoring to analysis result #378

Merged
merged 1 commit into from
Jan 23, 2016
Merged

Add reason for ignoring to analysis result #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java
View file
Open in desktop
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 @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakNode.java
View file
Open in desktop
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
11 changes: 10 additions & 1 deletion leakcanary-analyzer/src/main/java/com/squareup/leakcanary/LeakTraceElement.java
View file
Open in desktop
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 All @@ -83,6 +87,11 @@ public enum Holder {
if (extra != null) {
string += " " + extra;
}

if (exclusion != null) {
string += " , matching exclusion " + exclusion.matching;
}

return string;
}

Expand Down
114 changes: 52 additions & 62 deletions leakcanary-analyzer/src/main/java/com/squareup/leakcanary/ShortestPathFinder.java
View file
Open in desktop
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,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, "<Java Local>", LOCAL);
LeakNode parent = new LeakNode(null, holder, null, null, null);
if (node.exclusion != null) {
exclusion = node.exclusion;
}
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 +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<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 +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;
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
25 changes: 17 additions & 8 deletions leakcanary-analyzer/src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -44,23 +45,25 @@ public class AsyncTaskLeakTest {
static final String EXECUTOR_FIELD_2 = "sDefaultExecutor";

@Parameterized.Parameters public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
return asList(new Object[][] {
{ ASYNC_TASK }, //
{ ASYNC_TASK_MPREVIEW2 }, //
{ ASYNC_TASK_M_POSTPREVIEW2 } //
});
}

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,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<LeakTraceElement> elements = leakTrace.elements;
Exclusion exclusion = elements.get(0).exclusion;

List<String> expectedExclusions = asList(ASYNC_TASK_THREAD, EXECUTOR_FIELD_1, EXECUTOR_FIELD_2);
assertTrue(expectedExclusions.contains(exclusion.name));
}
}
Loading