From b17215191193ca87aa940efc973f1ea657f4e709 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Fri, 20 Jul 2018 14:03:20 -0700 Subject: [PATCH] Work around null KeyedWeakReference.key - Make HahaHelper.asString() explicitly non nullable instead of crashing later with NPE - If KeyedWeakReference.key is null (which should never happen at runtime), then we skip and keep looking for keys, but report all the null keys if we couldn't find any. If this was caused by some malformed object then we have a chance of actually finding the right KeyedWeakReference instance. However, if this was caused by a heap dump parsing issue then we'll still crash but at least we'll have an exception closer to the source. Fixes #874 --- .../main/java/com/squareup/leakcanary/HahaHelper.java | 1 + .../java/com/squareup/leakcanary/HeapAnalyzer.java | 11 ++++++++++- .../src/main/res/values/leak_canary_strings.xml | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HahaHelper.java b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HahaHelper.java index bb09ad9211..b322c162ed 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HahaHelper.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HahaHelper.java @@ -83,6 +83,7 @@ static String valueAsString(Object value) { /** Given a string instance from the heap dump, this returns its actual string value. */ static String asString(Object stringObject) { + checkNotNull(stringObject, "stringObject"); Instance instance = (Instance) stringObject; List values = classInstanceValues(instance); 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 d57663a534..ebd5617f6e 100644 --- a/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java +++ b/leakcanary-analyzer/src/main/java/com/squareup/leakcanary/HeapAnalyzer.java @@ -211,10 +211,19 @@ private String generateRootKey(RootObj root) { private Instance findLeakingReference(String key, Snapshot snapshot) { ClassObj refClass = snapshot.findClass(KeyedWeakReference.class.getName()); + if (refClass == null) { + throw new IllegalStateException( + "Could not find the " + KeyedWeakReference.class.getName() + " class in the heap dump."); + } List keysFound = new ArrayList<>(); for (Instance instance : refClass.getInstancesList()) { List values = classInstanceValues(instance); - String keyCandidate = asString(fieldValue(values, "key")); + Object keyFieldValue = fieldValue(values, "key"); + if (keyFieldValue == null) { + keysFound.add(null); + continue; + } + String keyCandidate = asString(keyFieldValue); if (keyCandidate.equals(key)) { return fieldValue(values, "referent"); } 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 b4faa52d09..e39f1f5390 100644 --- a/leakcanary-android/src/main/res/values/leak_canary_strings.xml +++ b/leakcanary-android/src/main/res/values/leak_canary_strings.xml @@ -34,7 +34,7 @@ Storage permission Dumping memory, app will freeze. Brrrr. Delete - "Please report this failure to http://github.com/square/leakcanary\n" + "Please report this failure to http://github.com/square/leakcanary and share the heapdump file that caused it.\n" Delete all Are you sure you want to delete all leaks? Could not save result.