Heap dumps on SD card

* Heap dumps and analysis results were previously saved in the app directory. They are now saved on the external storage (sd card). * Centralized internal static helper methods to a dedicated class: `LeakCanaryInternals`. BREAKING CHANGES * When upgrading, previously saved heap dumps will be lost, but won't be removed from the app directory. You should probably uninstall your app. * Added permission WRITE_EXTERNAL_STORAGE * Public API change: Removed `Application` parameter in `LeakCanary.androidWatcher()` * Public API change: Removed `Application` parameter in `AndroidHeapDumper()` * Fixes square#21 (can't share heap dump) * This is a step towards fixing square#15 (strict mode violations), although there's still more work.
Yky · May 10, 2015 · 61fdceb · 61fdceb
1 parent 0207ff5
commit 61fdceb
Show file tree

Hide file tree

Showing 7 changed files with 184 additions and 122 deletions.
diff --git a/library/leakcanary-android/src/main/AndroidManifest.xml b/library/leakcanary-android/src/main/AndroidManifest.xml
@@ -19,6 +19,9 @@
     package="com.squareup.leakcanary"
     >
 
+  <!-- To store the heap dumps and leak analysis results. -->
+  <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
+
   <application>
     <service
         android:name=".internal.HeapAnalyzerService"

diff --git a/library/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidHeapDumper.java b/library/leakcanary-android/src/main/java/com/squareup/leakcanary/AndroidHeapDumper.java
@@ -15,45 +15,54 @@
  */
 package com.squareup.leakcanary;
 
-import android.content.Context;
 import android.os.Debug;
 import android.util.Log;
 import java.io.File;
 import java.io.IOException;
 
-public final class AndroidHeapDumper implements HeapDumper {
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.isExternalStorageWritable;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.storageDirectory;
 
-  private final File heapDumpFile;
+public final class AndroidHeapDumper implements HeapDumper {
 
-  public AndroidHeapDumper(Context context) {
-    heapDumpFile = new File(context.getFilesDir(), "suspected_leak_heapdump.hprof");
-  }
+  private static final String TAG = "AndroidHeapDumper";
 
   @Override public File dumpHeap() {
+    if (!isExternalStorageWritable()) {
+      Log.d(TAG, "Could not dump heap, external storage not mounted.");
+    }
+    File heapDumpFile = getHeapDumpFile();
     if (heapDumpFile.exists()) {
-      Log.d("AndroidHeapDumper", "Could not dump heap, previous analysis still is in progress.");
-      // Heap analysis in progress, let's not put to much pressure on the device.
+      Log.d(TAG, "Could not dump heap, previous analysis still is in progress.");
+      // Heap analysis in progress, let's not put too much pressure on the device.
       return null;
     }
     try {
       Debug.dumpHprofData(heapDumpFile.getAbsolutePath());
       return heapDumpFile;
     } catch (IOException e) {
       cleanup();
-      Log.e("AndroidHeapDumper", "Could not perform heap dump", e);
+      Log.e(TAG, "Could not perform heap dump", e);
       // Abort heap dump
       return null;
     }
   }
 
+  private File getHeapDumpFile() {
+    return new File(storageDirectory(), "suspected_leak_heapdump.hprof");
+  }
+
   /**
    * Call this on app startup to clean up all heap dump files that had not been handled yet when
    * the app process was killed.
    */
   public void cleanup() {
+    if (isExternalStorageWritable()) {
+      Log.d(TAG, "Could not attempt cleanup, external storage not mounted.");
+    }
+    File heapDumpFile = getHeapDumpFile();
     if (heapDumpFile.exists()) {
-      Log.d("AndroidHeapDumper",
-          "Previous analysis did not complete correctly, cleaning: " + heapDumpFile);
+      Log.d(TAG, "Previous analysis did not complete correctly, cleaning: " + heapDumpFile);
       heapDumpFile.delete();
     }
   }

diff --git a/library/leakcanary-android/src/main/java/com/squareup/leakcanary/DisplayLeakService.java b/library/leakcanary-android/src/main/java/com/squareup/leakcanary/DisplayLeakService.java
@@ -30,8 +30,10 @@
 import static android.os.Build.VERSION.SDK_INT;
 import static android.os.Build.VERSION_CODES.HONEYCOMB;
 import static android.os.Build.VERSION_CODES.JELLY_BEAN;
-import static com.squareup.leakcanary.LeakCanary.classSimpleName;
 import static com.squareup.leakcanary.LeakCanary.leakInfo;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.classSimpleName;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.findNextAvailableHprofFile;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.leakResultFile;
 
 /**
  * Logs leak analysis results, and then shows a notification which will start {@link
@@ -42,20 +44,6 @@
  */
 public class DisplayLeakService extends AbstractAnalysisResultService {
 
-  private static File findNextAvailableHprofFile(File directory, int maxFiles) {
-    if (!directory.exists()) {
-      directory.mkdir();
-    }
-    for (int i = 0; i < maxFiles; i++) {
-      String heapDumpName = "heap_dump_" + i + ".hprof";
-      File file = new File(directory, heapDumpName);
-      if (!file.exists()) {
-        return file;
-      }
-    }
-    return null;
-  }
-
   @TargetApi(HONEYCOMB) @Override
   protected final void onHeapAnalyzed(HeapDump heapDump, AnalysisResult result) {
     String leakInfo = leakInfo(this, heapDump, result);
@@ -66,9 +54,8 @@ protected final void onHeapAnalyzed(HeapDump heapDump, AnalysisResult result) {
       return;
     }
 
-    File leakDirectory = DisplayLeakActivity.leakDirectory(this);
     int maxStoredLeaks = getResources().getInteger(R.integer.__leak_canary_max_stored_leaks);
-    File renamedFile = findNextAvailableHprofFile(leakDirectory, maxStoredLeaks);
+    File renamedFile = findNextAvailableHprofFile(maxStoredLeaks);
 
     if (renamedFile == null) {
       // No file available.
@@ -80,7 +67,7 @@ protected final void onHeapAnalyzed(HeapDump heapDump, AnalysisResult result) {
 
     heapDump = heapDump.renameFile(renamedFile);
 
-    File resultFile = DisplayLeakActivity.leakResultFile(renamedFile);
+    File resultFile = leakResultFile(renamedFile);
     FileOutputStream fos = null;
     try {
       fos = new FileOutputStream(resultFile);

diff --git a/library/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java b/library/leakcanary-android/src/main/java/com/squareup/leakcanary/LeakCanary.java
@@ -15,23 +15,17 @@
  */
 package com.squareup.leakcanary;
 
-import android.app.ActivityManager;
 import android.app.Application;
-import android.app.Service;
-import android.content.ComponentName;
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
-import android.content.pm.ServiceInfo;
 import android.os.Build;
 import android.util.Log;
 import com.squareup.leakcanary.internal.DisplayLeakActivity;
 import com.squareup.leakcanary.internal.HeapAnalyzerService;
 
-import static android.content.pm.PackageManager.COMPONENT_ENABLED_STATE_DISABLED;
-import static android.content.pm.PackageManager.COMPONENT_ENABLED_STATE_ENABLED;
-import static android.content.pm.PackageManager.DONT_KILL_APP;
-import static android.content.pm.PackageManager.GET_SERVICES;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.isInServiceProcess;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.setEnabled;
 
 public final class LeakCanary {
 
@@ -55,17 +49,17 @@ public static RefWatcher install(Application application,
     enableDisplayLeakActivity(application);
     HeapDump.Listener heapDumpListener =
         new ServiceHeapDumpListener(application, listenerServiceClass);
-    RefWatcher refWatcher = androidWatcher(application, heapDumpListener);
+    RefWatcher refWatcher = androidWatcher(heapDumpListener);
     ActivityRefWatcher.installOnIcsPlus(application, refWatcher);
     return refWatcher;
   }
 
   /**
    * Creates a {@link RefWatcher} with a default configuration suitable for Android.
    */
-  public static RefWatcher androidWatcher(Application app, HeapDump.Listener heapDumpListener) {
+  public static RefWatcher androidWatcher(HeapDump.Listener heapDumpListener) {
     DebuggerControl debuggerControl = new AndroidDebuggerControl();
-    AndroidHeapDumper heapDumper = new AndroidHeapDumper(app);
+    AndroidHeapDumper heapDumper = new AndroidHeapDumper();
     heapDumper.cleanup();
     return new RefWatcher(new AndroidWatchExecutor(), debuggerControl, GcTrigger.DEFAULT,
         heapDumper, heapDumpListener);
@@ -141,70 +135,6 @@ public static boolean isInAnalyzerProcess(Context context) {
     return isInServiceProcess(context, HeapAnalyzerService.class);
   }
 
-  private static boolean isInServiceProcess(Context context,
-      Class<? extends Service> serviceClass) {
-    PackageManager packageManager = context.getPackageManager();
-    PackageInfo packageInfo;
-    try {
-      packageInfo = packageManager.getPackageInfo(context.getPackageName(), GET_SERVICES);
-    } catch (Exception e) {
-      Log.e("AndroidUtils", "Could not get package info for " + context.getPackageName(), e);
-      return false;
-    }
-    String mainProcess = packageInfo.applicationInfo.processName;
-
-    ComponentName component = new ComponentName(context, serviceClass);
-    ServiceInfo serviceInfo;
-    try {
-      serviceInfo = packageManager.getServiceInfo(component, 0);
-    } catch (PackageManager.NameNotFoundException ignored) {
-      // Service is disabled.
-      return false;
-    }
-
-    if (serviceInfo.processName.equals(mainProcess)) {
-      Log.e("AndroidUtils",
-          "Did not expect service " + serviceClass + " to run in main process " + mainProcess);
-      // Technically we are in the service process, but we're not in the service dedicated process.
-      return false;
-    }
-
-    int myPid = android.os.Process.myPid();
-    ActivityManager activityManager =
-        (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE);
-    ActivityManager.RunningAppProcessInfo myProcess = null;
-    for (ActivityManager.RunningAppProcessInfo process : activityManager.getRunningAppProcesses()) {
-      if (process.pid == myPid) {
-        myProcess = process;
-        break;
-      }
-    }
-    if (myProcess == null) {
-      Log.e("AndroidUtils", "Could not find running process for " + myPid);
-      return false;
-    }
-
-    return myProcess.processName.equals(serviceInfo.processName);
-  }
-
-  static void setEnabled(Context context, Class<?> componentClass, boolean enabled) {
-    ComponentName component = new ComponentName(context, componentClass);
-    PackageManager packageManager = context.getPackageManager();
-    int newState = enabled ? COMPONENT_ENABLED_STATE_ENABLED : COMPONENT_ENABLED_STATE_DISABLED;
-    // Blocks on IPC.
-    packageManager.setComponentEnabledSetting(component, newState, DONT_KILL_APP);
-  }
-
-  /** Extracts the class simple name out of a string containing a fully qualified class name. */
-  static String classSimpleName(String className) {
-    int separator = className.lastIndexOf('.');
-    if (separator == -1) {
-      return className;
-    } else {
-      return className.substring(separator + 1);
-    }
-  }
-
   private LeakCanary() {
     throw new AssertionError();
   }

diff --git a/...ary/leakcanary-android/src/main/java/com/squareup/leakcanary/ServiceHeapDumpListener.java b/...ary/leakcanary-android/src/main/java/com/squareup/leakcanary/ServiceHeapDumpListener.java
@@ -19,6 +19,7 @@
 import com.squareup.leakcanary.internal.HeapAnalyzerService;
 
 import static com.squareup.leakcanary.Preconditions.checkNotNull;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.setEnabled;
 
 public final class ServiceHeapDumpListener implements HeapDump.Listener {
 
@@ -27,8 +28,8 @@ public final class ServiceHeapDumpListener implements HeapDump.Listener {
 
   public ServiceHeapDumpListener(Context context,
       Class<? extends AbstractAnalysisResultService> listenerServiceClass) {
-    LeakCanary.setEnabled(context, listenerServiceClass, true);
-    LeakCanary.setEnabled(context, HeapAnalyzerService.class, true);
+    setEnabled(context, listenerServiceClass, true);
+    setEnabled(context, HeapAnalyzerService.class, true);
     this.listenerServiceClass = checkNotNull(listenerServiceClass, "listenerServiceClass");
     this.context = checkNotNull(context, "context").getApplicationContext();
   }

diff --git a/...eakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakActivity.java b/...eakcanary-android/src/main/java/com/squareup/leakcanary/internal/DisplayLeakActivity.java
@@ -57,21 +57,15 @@
 import static android.text.format.DateUtils.FORMAT_SHOW_DATE;
 import static android.text.format.DateUtils.FORMAT_SHOW_TIME;
 import static com.squareup.leakcanary.LeakCanary.leakInfo;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.detectedLeakDirectory;
+import static com.squareup.leakcanary.internal.LeakCanaryInternals.leakResultFile;
 
 @SuppressWarnings("ConstantConditions") @TargetApi(Build.VERSION_CODES.HONEYCOMB)
 public final class DisplayLeakActivity extends Activity {
 
   private static final String TAG = "DisplayLeakActivity";
   private static final String SHOW_LEAK_EXTRA = "show_latest";
 
-  public static File leakDirectory(Context context) {
-    return new File(context.getFilesDir(), "detected_leaks");
-  }
-
-  public static File leakResultFile(File heapdumpFile) {
-    return new File(heapdumpFile.getParentFile(), heapdumpFile.getName() + ".result");
-  }
-
   public static PendingIntent createPendingIntent(Context context, String referenceKey) {
     Intent intent = new Intent(context, DisplayLeakActivity.class);
     intent.putExtra(SHOW_LEAK_EXTRA, referenceKey);
@@ -257,9 +251,9 @@ public void onItemClick(AdapterView<?> parent, View view, int position, long id)
         actionButton.setText("Remove all leaks");
         actionButton.setOnClickListener(new View.OnClickListener() {
           @Override public void onClick(View v) {
-            File directory = leakDirectory(DisplayLeakActivity.this);
-            if (directory.exists()) {
-              for (File file : directory.listFiles()) {
+            File[] files = detectedLeakDirectory().listFiles();
+            if (files != null) {
+              for (File file : files) {
                 file.delete();
               }
             }
@@ -359,7 +353,7 @@ static void forgetActivity() {
 
     LoadLeaks(DisplayLeakActivity activity) {
       this.activityOrNull = activity;
-      leakDirectory = leakDirectory(activity);
+      leakDirectory = detectedLeakDirectory();
       mainHandler = new Handler(Looper.getMainLooper());
     }
 
@@ -371,8 +365,8 @@ static void forgetActivity() {
         }
       });
       if (files != null) {
-        for (File file : files) {
-          File resultFile = leakResultFile(file);
+        for (File heapDumpFile : files) {
+          File resultFile = leakResultFile(heapDumpFile);
           FileInputStream fis = null;
           try {
             fis = new FileInputStream(resultFile);
@@ -383,9 +377,10 @@ static void forgetActivity() {
           } catch (IOException | ClassNotFoundException e) {
             // Likely a change in the serializable result class.
             // Let's remove the files, we can't read them anymore.
-            file.delete();
+            heapDumpFile.delete();
             resultFile.delete();
-            Log.e(TAG, "Could not read result file, deleted result and heap dump:" + file, e);
+            Log.e(TAG, "Could not read result file, deleted result and heap dump:" + heapDumpFile,
+                e);
           } finally {
             if (fis != null) {
               try {