Improve hprof storage

leakcanary-android-core/src/main/java/leakcanary/DefaultAnalysisResultListener.kt

@@ -4,6 +4,7 @@ import android.app.Application
 import com.squareup.leakcanary.core.R
 import leakcanary.Exclusion.Status.WEAKLY_REACHABLE
 import leakcanary.Exclusion.Status.WONT_FIX_LEAK
+import leakcanary.internal.LeakDirectoryProvider
 import leakcanary.internal.Notifications
 import leakcanary.internal.NotificationType.LEAKCANARY_RESULT
 import leakcanary.internal.activity.LeakActivity
@@ -85,9 +86,12 @@ object DefaultAnalysisResultListener : AnalysisResultListener {
   private fun renameHeapdump(heapDumpFile: File): File {
     val fileName = SimpleDateFormat("yyyy-MM-dd_HH-mm-ss_SSS'.hprof'", Locale.US).format(Date())
 
+    val path = heapDumpFile.absolutePath
     val newFile = File(heapDumpFile.parent, fileName)
     val renamed = heapDumpFile.renameTo(newFile)
-    if (!renamed) {
+    if (renamed) {
+     LeakDirectoryProvider.filesRenamedEndOfHeapAnalyzer += path
+    } else {
       CanaryLog.d(
           "Could not rename heap dump file %s to %s", heapDumpFile.path, newFile.path
       )

leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt

@@ -50,7 +50,22 @@ object LeakCanary {
      * (e.g. bitmaps).
      * Computing the retained heap size can slow down the leak analysis and is off by default.
      */
-    val computeRetainedHeapSize: Boolean = false
+    val computeRetainedHeapSize: Boolean = false,
+
+    /**
+     * How many heap dumps are kept locally. When this threshold is reached LeakCanary starts
+     * deleting the older heap dumps. As several heap dumps may be enqueued you should avoid
+     * going down to 1 or 2.
+     */
+    val maxStoredHeapDumps: Int = 7,
+
+    /**
+     * LeakCanary always attempts to store heap dumps on the external storage first. If the
+     * WRITE_EXTERNAL_STORAGE permission is not granted and [requestWriteExternalStoragePermission]
+     * is true, then LeakCanary will display a notification to ask for that permission.
+     */
+    val requestWriteExternalStoragePermission: Boolean = false
+
   )
 
   @Volatile

leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt

@@ -25,6 +25,7 @@ import leakcanary.CanaryLog
 import leakcanary.HeapAnalyzer
 import leakcanary.LeakCanary
 import java.io.File
+import java.lang.IllegalStateException
 
 /**
  * This service runs in a main app process.
@@ -43,6 +44,15 @@ internal class HeapAnalyzerService : ForegroundService(
     // Since we're running in the main process we should be careful not to impact it.
     Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND)
     val heapDumpFile = intent.getSerializableExtra(HEAPDUMP_FILE_EXTRA) as File
+
+    if (!heapDumpFile.exists()) {
+      throw IllegalStateException(
+          "Hprof file missing due to: [${LeakDirectoryProvider.hprofDeleteReason(
+              heapDumpFile
+          )}] $heapDumpFile"
+      )
+    }
+
     val heapAnalyzer = HeapAnalyzer(this)
     val config = LeakCanary.config
     val heapAnalysis =
@@ -54,7 +64,11 @@ internal class HeapAnalyzerService : ForegroundService(
     try {
       config.analysisResultListener(application, heapAnalysis)
     } finally {
-      heapAnalysis.heapDumpFile.delete()
+      val path = heapAnalysis.heapDumpFile.absolutePath
+      val deleted = heapAnalysis.heapDumpFile.delete()
+      if (deleted) {
+        LeakDirectoryProvider.filesDeletedEndOfHeapAnalyzer += path
+      }
     }
   }
 

leakcanary-android-core/src/main/java/leakcanary/internal/InternalLeakCanary.kt

@@ -30,7 +30,11 @@ internal object InternalLeakCanary : LeakSentryListener {
     private set
 
   val leakDirectoryProvider: LeakDirectoryProvider by lazy {
-    LeakDirectoryProvider(application)
+    LeakDirectoryProvider(application, {
+      LeakCanary.config.maxStoredHeapDumps
+    }, {
+      LeakCanary.config.requestWriteExternalStoragePermission
+    })
   }
 
   val leakDisplayActivityIntent: Intent

leakcanary-android-core/src/main/java/leakcanary/internal/LeakDirectoryProvider.kt

@@ -35,25 +35,19 @@ import java.util.UUID
 /**
  * Provides access to where heap dumps and analysis results will be stored.
  */
-internal class LeakDirectoryProvider @JvmOverloads constructor(
+internal class LeakDirectoryProvider constructor(
   context: Context,
-  private val maxStoredHeapDumps: Int = DEFAULT_MAX_STORED_HEAP_DUMPS
+  private val maxStoredHeapDumps: () -> Int,
+  private val requestExternalStoragePermission: () -> Boolean
 ) {
 
-  private val context: Context
+  private val context: Context = context.applicationContext
 
   @Volatile private var writeExternalStorageGranted: Boolean = false
   @Volatile private var permissionNotificationDisplayed: Boolean = false
 
-  init {
-    if (maxStoredHeapDumps < 1) {
-      throw IllegalArgumentException("maxStoredHeapDumps must be at least 1")
-    }
-    this.context = context.applicationContext
-  }
-
   fun listFiles(filter: FilenameFilter): MutableList<File> {
-    if (!hasStoragePermission()) {
+    if (!hasStoragePermission() && requestExternalStoragePermission()) {
       requestWritePermissionNotification()
     }
     val files = ArrayList<File>()
@@ -76,8 +70,12 @@ internal class LeakDirectoryProvider @JvmOverloads constructor(
     var storageDirectory = externalStorageDirectory()
     if (!directoryWritableAfterMkdirs(storageDirectory)) {
       if (!hasStoragePermission()) {
-        CanaryLog.d("WRITE_EXTERNAL_STORAGE permission not granted")
-        requestWritePermissionNotification()
+        if (requestExternalStoragePermission()) {
+          CanaryLog.d("WRITE_EXTERNAL_STORAGE permission not granted, requesting")
+          requestWritePermissionNotification()
+        } else {
+          CanaryLog.d("WRITE_EXTERNAL_STORAGE permission not granted, ignoring")
+        }
       } else {
         val state = Environment.getExternalStorageState()
         if (Environment.MEDIA_MOUNTED != state) {
@@ -112,7 +110,11 @@ internal class LeakDirectoryProvider @JvmOverloads constructor(
         )
       })
     for (file in allFilesExceptPending) {
+      val path = file.absolutePath
       val deleted = file.delete()
+      if (deleted) {
+        filesDeletedClearDirectory += path
+      }
       if (!deleted) {
         CanaryLog.d("Could not delete file %s", file.path)
       }
@@ -173,6 +175,11 @@ internal class LeakDirectoryProvider @JvmOverloads constructor(
           HPROF_SUFFIX
       )
     })
+    val maxStoredHeapDumps = maxStoredHeapDumps()
+    if (maxStoredHeapDumps < 1) {
+      throw IllegalArgumentException("maxStoredHeapDumps must be at least 1")
+    }
+
     val filesToRemove = hprofFiles.size - maxStoredHeapDumps
     if (filesToRemove > 0) {
       CanaryLog.d("Removing %d heap dumps", filesToRemove)
@@ -182,8 +189,11 @@ internal class LeakDirectoryProvider @JvmOverloads constructor(
             .compareTo(rhs.lastModified())
       })
       for (i in 0 until filesToRemove) {
+        val path = hprofFiles[i].absolutePath
         val deleted = hprofFiles[i].delete()
-        if (!deleted) {
+        if (deleted) {
+          filesDeletedTooOld += path
+        } else {
           CanaryLog.d("Could not delete old hprof file %s", hprofFiles[i].path)
         }
       }
@@ -192,9 +202,25 @@ internal class LeakDirectoryProvider @JvmOverloads constructor(
 
   companion object {
 
-    private const val DEFAULT_MAX_STORED_HEAP_DUMPS = 7
+    private val filesDeletedTooOld = mutableListOf<String>()
+    private val filesDeletedClearDirectory = mutableListOf<String>()
+    val filesDeletedRemoveLeak = mutableListOf<String>()
+    val filesDeletedEndOfHeapAnalyzer = mutableListOf<String>()
+    val filesRenamedEndOfHeapAnalyzer = mutableListOf<String>()
 
     private const val HPROF_SUFFIX = ".hprof"
     private const val PENDING_HEAPDUMP_SUFFIX = "_pending$HPROF_SUFFIX"
+
+    fun hprofDeleteReason(file: File): String {
+      val path = file.absolutePath
+      return when {
+        filesDeletedTooOld.contains(path) -> "Older than all other hprof files"
+        filesDeletedClearDirectory.contains(path) -> "Hprof directory cleared"
+        filesDeletedRemoveLeak.contains(path) -> "Leak manually removed"
+        filesDeletedEndOfHeapAnalyzer.contains(path) -> "Heap Analysis done & leak reported"
+        filesRenamedEndOfHeapAnalyzer.contains(path) -> "Heap Analysis done & hprof moved"
+        else -> "Unknown"
+      }
+    }
   }
 }

leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/HeapAnalysisTable.kt

@@ -10,6 +10,7 @@ import leakcanary.HeapAnalysisFailure
 import leakcanary.HeapAnalysisSuccess
 import leakcanary.Serializables
 import leakcanary.internal.InternalLeakCanary
+import leakcanary.internal.LeakDirectoryProvider
 import leakcanary.leakingInstances
 import leakcanary.toByteArray
 import org.intellij.lang.annotations.Language
@@ -120,8 +121,11 @@ internal object HeapAnalysisTable {
   ) {
     if (heapDumpFile != null) {
       AsyncTask.SERIAL_EXECUTOR.execute {
+        val path = heapDumpFile.absolutePath
         val heapDumpDeleted = heapDumpFile.delete()
-        if (!heapDumpDeleted) {
+        if (heapDumpDeleted) {
+          LeakDirectoryProvider.filesDeletedRemoveLeak += path
+        } else {
           CanaryLog.d("Could not delete heap dump file %s", heapDumpFile.path)
         }
       }