From 0c40611d099be343fb271b6641ecdb7d56255385 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Tue, 10 Sep 2019 18:48:59 -0700 Subject: [PATCH] Extract metadata from heap dump Fixes #1519 --- docs/changelog.md | 3 +- docs/recipes.md | 36 +++++++++++++++ .../src/main/java/leakcanary/LeakCanary.kt | 11 +++++ .../internal/HeapAnalyzerService.kt | 8 +++- .../leakcanary/internal/InternalLeakCanary.kt | 8 ++++ .../internal/activity/db/LeaksDbHelper.kt | 3 +- .../screen/HeapAnalysisSuccessScreen.kt | 2 + .../java/shark/AndroidMetadataExtractor.kt | 36 +++++++++++++++ .../src/test/java/shark/LegacyHprofTest.kt | 14 +++++- shark/src/main/java/shark/HeapAnalysis.kt | 1 + shark/src/main/java/shark/HeapAnalyzer.kt | 7 ++- .../src/main/java/shark/MetadataExtractor.kt | 39 ++++++++++++++++ .../java/shark/OnAnalysisProgressListener.kt | 6 +-- .../test/java/shark/MetadataExtractorTest.kt | 46 +++++++++++++++++++ shark/src/test/java/shark/TestUtil.kt | 10 ++-- 15 files changed, 215 insertions(+), 15 deletions(-) create mode 100644 shark-android/src/main/java/shark/AndroidMetadataExtractor.kt create mode 100644 shark/src/main/java/shark/MetadataExtractor.kt create mode 100644 shark/src/test/java/shark/MetadataExtractorTest.kt diff --git a/docs/changelog.md b/docs/changelog.md index 373bb5226a..bd34e97a78 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,8 @@ ## Next release -* Adding support for deobfuscation using Proguard mapping files in Shark [#1499](https://github.com/square/leakcanary/issues/1499) +* Added support for deobfuscation using Proguard mapping files in Shark [#1499](https://github.com/square/leakcanary/issues/1499) +* Added support for extracting metadata from the heap dump (see the[recipe](recipes.md#extracting-additional-metadata-from-the-heap-dump)) [#1519](https://github.com/square/leakcanary/issues/1519) * Several performance improvements when parsing heap dumps * Fixed several bugs and crashes * Added new known leak patterns diff --git a/docs/recipes.md b/docs/recipes.md index 1f549f67a9..03bca5a4dc 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -298,3 +298,39 @@ dependencies { devDebugImplementation "com.squareup.leakcanary:leakcanary-android:${version}" } ``` + +## Extracting additional metadata from the heap dump + +[LeakCanary.Config.metatadaExtractor](/leakcanary/api/leakcanary-android-core/leakcanary/-leak-canary/-config/metatadaExtractor/) extracts metadata from a hprof. The metadata is then available in `HeapAnalysisSuccess.metadata`. If defaults to `AndroidMetadataExtractor` but you can override it to extract additional metadata from the hprof. + +For example, if you want to include the app version name in your heap analysis reports, you need to first store it in memory (e.g. in a static field) and then you can retrieve it in `MetadataExtractor`. + +```kotlin +class DebugExampleApplication : ExampleApplication() { + + companion object { + @JvmStatic + lateinit var savedVersionName: String + } + + override fun onCreate() { + super.onCreate() + + val packageInfo = packageManager.getPackageInfo(packageName, 0) + savedVersionName = packageInfo.versionName + + LeakCanary.config = LeakCanary.config.copy( + metatadaExtractor = MetadataExtractor { graph -> + val companionClass = + graph.findClassByName("com.example.DebugExampleApplication")!! + + val versionNameField = companionClass["savedVersionName"]!! + val versionName = versionNameField.valueAsInstance!!.readAsJavaString()!! + + val defaultMetadata = AndroidMetadataExtractor.extractMetadata(graph) + + mapOf("App Version Name" to versionName) + defaultMetadata + }) + } +} +``` \ No newline at end of file diff --git a/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt b/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt index 6ef368ed40..9682609491 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt @@ -3,10 +3,13 @@ package leakcanary import android.content.Intent import leakcanary.LeakCanary.config import leakcanary.internal.InternalLeakCanary +import shark.AndroidMetadataExtractor import shark.AndroidObjectInspectors import shark.AndroidReferenceMatchers +import shark.HeapAnalysisSuccess import shark.IgnoredReferenceMatcher import shark.LibraryLeakReferenceMatcher +import shark.MetadataExtractor import shark.ObjectInspector import shark.ReferenceMatcher import shark.SharkLog @@ -92,6 +95,14 @@ object LeakCanary { */ val onHeapAnalyzedListener: OnHeapAnalyzedListener = DefaultOnHeapAnalyzedListener.create(), + /** + * Extracts metadata from a hprof to be reported in [HeapAnalysisSuccess.metadata]. + * Called on a background thread during heap analysis. + * + * Defaults to [AndroidMetadataExtractor] + */ + val metatadaExtractor: MetadataExtractor = AndroidMetadataExtractor, + /** * Whether to compute the retained heap size, which is the total number of bytes in memory that * would be reclaimed if the detected leaks didn't happen. This includes native memory diff --git a/leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt b/leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt index 131b9cde7d..f8face9346 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/internal/HeapAnalyzerService.kt @@ -60,10 +60,14 @@ internal class HeapAnalyzerService : ForegroundService( val heapAnalysis = heapAnalyzer.analyze( - heapDumpFile, config.referenceMatchers, config.computeRetainedHeapSize, config.objectInspectors, + heapDumpFile, + config.referenceMatchers, + config.computeRetainedHeapSize, + config.objectInspectors, if (config.useExperimentalLeakFinders) config.objectInspectors else listOf( ObjectInspectors.KEYED_WEAK_REFERENCE - ) + ), + config.metatadaExtractor ) config.onHeapAnalyzedListener.onHeapAnalyzed(heapAnalysis) diff --git a/leakcanary-android-core/src/main/java/leakcanary/internal/InternalLeakCanary.kt b/leakcanary-android-core/src/main/java/leakcanary/internal/InternalLeakCanary.kt index af8c148c1a..dbf7e33baf 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/internal/InternalLeakCanary.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/internal/InternalLeakCanary.kt @@ -14,6 +14,7 @@ import android.os.Build.VERSION import android.os.Build.VERSION_CODES import android.os.Handler import android.os.HandlerThread +import com.squareup.leakcanary.core.BuildConfig import com.squareup.leakcanary.core.R import leakcanary.GcTrigger import leakcanary.LeakCanary @@ -34,6 +35,13 @@ internal object InternalLeakCanary : (Application) -> Unit, OnObjectRetainedList private lateinit var heapDumpTrigger: HeapDumpTrigger lateinit var application: Application + + // BuildConfig.LIBRARY_VERSION is stripped so this static var is how we keep it around to find + // it later when parsing the heap dump. + @Suppress("unused") + @JvmStatic + private var version = BuildConfig.LIBRARY_VERSION + @Volatile var applicationVisible = false private set diff --git a/leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeaksDbHelper.kt b/leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeaksDbHelper.kt index aed738d903..80888c0906 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeaksDbHelper.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/internal/activity/db/LeaksDbHelper.kt @@ -25,7 +25,6 @@ internal class LeaksDbHelper(context: Context) : SQLiteOpenHelper( } companion object { - // Last updated for next after 2.0-alpha-3 - private const val VERSION = 16 + private const val VERSION = 17 } } \ No newline at end of file diff --git a/leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisSuccessScreen.kt b/leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisSuccessScreen.kt index 546a1d6393..3f3525072d 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisSuccessScreen.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/internal/activity/screen/HeapAnalysisSuccessScreen.kt @@ -123,6 +123,8 @@ internal class HeapAnalysisSuccessScreen( titleText to timeText }) + rowList.addAll(heapAnalysis.metadata.map { it.key to it.value }) + listView.adapter = SimpleListAdapter(R.layout.leak_canary_leak_row, rowList) { view, position -> val titleView = view.findViewById(R.id.leak_canary_row_text) diff --git a/shark-android/src/main/java/shark/AndroidMetadataExtractor.kt b/shark-android/src/main/java/shark/AndroidMetadataExtractor.kt new file mode 100644 index 0000000000..842f86af99 --- /dev/null +++ b/shark-android/src/main/java/shark/AndroidMetadataExtractor.kt @@ -0,0 +1,36 @@ +package shark + +object AndroidMetadataExtractor : MetadataExtractor { + override fun extractMetadata(graph: HeapGraph): Map { + val build = AndroidBuildMirror.fromHeapGraph(graph) + + val leakCanaryVersion = readLeakCanaryVersion(graph) + val processName = readProcessName(graph) + + return mapOf( + "Build.VERSION.SDK_INT" to build.sdkInt.toString(), + "Build.MANUFACTURER" to build.manufacturer, + "LeakCanary version" to leakCanaryVersion, + "App process name" to processName + ) + } + + private fun readLeakCanaryVersion(graph: HeapGraph): String { + val versionHolderClass = graph.findClassByName("leakcanary.internal.InternalLeakCanary") + return versionHolderClass?.get("version")?.value?.readAsJavaString() ?: "Unknown" + } + + private fun readProcessName(graph: HeapGraph): String { + val activityThread = graph.findClassByName("android.app.ActivityThread") + ?.get("sCurrentActivityThread") + ?.valueAsInstance + val appBindData = activityThread?.get("android.app.ActivityThread", "mBoundApplication") + ?.valueAsInstance + val appInfo = appBindData?.get("android.app.ActivityThread\$AppBindData", "appInfo") + ?.valueAsInstance + + return appInfo?.get( + "android.content.pm.ApplicationInfo", "processName" + )?.valueAsInstance?.readAsJavaString() ?: "Unknown" + } +} \ No newline at end of file diff --git a/shark-android/src/test/java/shark/LegacyHprofTest.kt b/shark-android/src/test/java/shark/LegacyHprofTest.kt index f2a1278ae1..ac9da3631e 100644 --- a/shark-android/src/test/java/shark/LegacyHprofTest.kt +++ b/shark-android/src/test/java/shark/LegacyHprofTest.kt @@ -16,6 +16,14 @@ class LegacyHprofTest { val leak2 = analysis.applicationLeaks[1] assertThat(leak1.className).isEqualTo("android.graphics.Bitmap") assertThat(leak2.className).isEqualTo("com.example.leakcanary.MainActivity") + assertThat(analysis.metadata).isEqualTo( + mapOf( + "App process name" to "com.example.leakcanary", + "Build.MANUFACTURER" to "Genymotion", + "Build.VERSION.SDK_INT" to "19", + "LeakCanary version" to "Unknown" + ) + ) } @Test fun androidM() { @@ -119,7 +127,11 @@ class LegacyHprofTest { private fun analyzeHprof(hprofFile: File): HeapAnalysisSuccess { val heapAnalyzer = HeapAnalyzer(OnAnalysisProgressListener.NO_OP) val analysis = heapAnalyzer.analyze( - hprofFile, AndroidReferenceMatchers.appDefaults, false, AndroidObjectInspectors.appDefaults + heapDumpFile = hprofFile, + referenceMatchers = AndroidReferenceMatchers.appDefaults, + computeRetainedHeapSize = false, + objectInspectors = AndroidObjectInspectors.appDefaults, + metadataExtractor = AndroidMetadataExtractor ) println(analysis) return analysis as HeapAnalysisSuccess diff --git a/shark/src/main/java/shark/HeapAnalysis.kt b/shark/src/main/java/shark/HeapAnalysis.kt index 8aeb101d4d..bc677a779b 100644 --- a/shark/src/main/java/shark/HeapAnalysis.kt +++ b/shark/src/main/java/shark/HeapAnalysis.kt @@ -46,6 +46,7 @@ data class HeapAnalysisSuccess( override val heapDumpFile: File, override val createdAtTimeMillis: Long, override val analysisDurationMillis: Long, + val metadata: Map, /** * The list of [ApplicationLeak] found in the heap dump by [HeapAnalyzer]. */ diff --git a/shark/src/main/java/shark/HeapAnalyzer.kt b/shark/src/main/java/shark/HeapAnalyzer.kt index 1ad4ca5bb9..20771955c9 100644 --- a/shark/src/main/java/shark/HeapAnalyzer.kt +++ b/shark/src/main/java/shark/HeapAnalyzer.kt @@ -41,6 +41,7 @@ import shark.LeakTraceElement.Holder.THREAD import shark.OnAnalysisProgressListener.Step.BUILDING_LEAK_TRACES import shark.OnAnalysisProgressListener.Step.COMPUTING_NATIVE_RETAINED_SIZE import shark.OnAnalysisProgressListener.Step.COMPUTING_RETAINED_SIZE +import shark.OnAnalysisProgressListener.Step.EXTRACTING_METADATA import shark.OnAnalysisProgressListener.Step.FINDING_LEAKING_INSTANCES import shark.OnAnalysisProgressListener.Step.PARSING_HEAP_DUMP import shark.OnAnalysisProgressListener.Step.REPORTING_HEAP_ANALYSIS @@ -82,6 +83,7 @@ class HeapAnalyzer constructor( computeRetainedHeapSize: Boolean = false, objectInspectors: List = emptyList(), leakFinders: List = objectInspectors, + metadataExtractor: MetadataExtractor = MetadataExtractor.NO_OP, proguardMapping: ProguardMapping? = null ): HeapAnalysis { val analysisStartNanoTime = System.nanoTime() @@ -101,13 +103,16 @@ class HeapAnalyzer constructor( .use { hprof -> val graph = HprofHeapGraph.indexHprof(hprof, proguardMapping) + listener.onAnalysisProgress(EXTRACTING_METADATA) + val metadata = metadataExtractor.extractMetadata(graph) + val findLeakInput = FindLeakInput( graph, leakFinders, referenceMatchers, computeRetainedHeapSize, objectInspectors ) val (applicationLeaks, libraryLeaks) = findLeakInput.findLeaks() listener.onAnalysisProgress(REPORTING_HEAP_ANALYSIS) return HeapAnalysisSuccess( - heapDumpFile, System.currentTimeMillis(), since(analysisStartNanoTime), + heapDumpFile, System.currentTimeMillis(), since(analysisStartNanoTime), metadata, applicationLeaks, libraryLeaks ) } diff --git a/shark/src/main/java/shark/MetadataExtractor.kt b/shark/src/main/java/shark/MetadataExtractor.kt new file mode 100644 index 0000000000..33aba17201 --- /dev/null +++ b/shark/src/main/java/shark/MetadataExtractor.kt @@ -0,0 +1,39 @@ +package shark + +import shark.MetadataExtractor.Companion.invoke +import shark.ObjectInspector.Companion.invoke + +/** + * Extracts metadata from a hprof to be reported in [HeapAnalysisSuccess.metadata]. + * + * You can create a [MetadataExtractor] from a lambda by calling [invoke]. + */ +interface MetadataExtractor { + fun extractMetadata(graph: HeapGraph): Map + + companion object { + + /** + * A no-op [MetadataExtractor] + */ + val NO_OP = MetadataExtractor { emptyMap() } + + /** + * Utility function to create a [MetadataExtractor] from the passed in [block] lambda instead of + * using the anonymous `object : MetadataExtractor` syntax. + * + * Usage: + * + * ```kotlin + * val inspector = MetadataExtractor { graph -> + * + * } + * ``` + */ + inline operator fun invoke(crossinline block: (HeapGraph) -> Map): MetadataExtractor = + object : MetadataExtractor { + override fun extractMetadata(graph: HeapGraph): Map = block(graph) + } + } + +} \ No newline at end of file diff --git a/shark/src/main/java/shark/OnAnalysisProgressListener.kt b/shark/src/main/java/shark/OnAnalysisProgressListener.kt index c2c99fef30..9f5659dd62 100644 --- a/shark/src/main/java/shark/OnAnalysisProgressListener.kt +++ b/shark/src/main/java/shark/OnAnalysisProgressListener.kt @@ -8,6 +8,7 @@ interface OnAnalysisProgressListener { // These steps are defined in the order in which they occur. enum class Step { PARSING_HEAP_DUMP, + EXTRACTING_METADATA, FINDING_LEAKING_INSTANCES, FINDING_PATHS_TO_LEAKING_OBJECTS, FINDING_DOMINATORS, @@ -24,10 +25,7 @@ interface OnAnalysisProgressListener { /** * A no-op [OnAnalysisProgressListener] */ - val NO_OP = object : OnAnalysisProgressListener { - override fun onAnalysisProgress(step: Step) { - } - } + val NO_OP = OnAnalysisProgressListener {} /** * Utility function to create a [OnAnalysisProgressListener] from the passed in [block] lambda diff --git a/shark/src/test/java/shark/MetadataExtractorTest.kt b/shark/src/test/java/shark/MetadataExtractorTest.kt new file mode 100644 index 0000000000..e809274e04 --- /dev/null +++ b/shark/src/test/java/shark/MetadataExtractorTest.kt @@ -0,0 +1,46 @@ +package shark + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +class MetadataExtractorTest { + + @get:Rule + var testFolder = TemporaryFolder() + private lateinit var hprofFile: File + + @Before + fun setUp() { + hprofFile = testFolder.newFile("temp.hprof") + } + + @Test fun extractStaticStringField() { + HprofWriter.open(hprofFile) + .helper { + val helloString = string("Hello") + clazz( + "World", staticFields = listOf( + "message" to helloString + ) + ) + } + + val extractor = object : MetadataExtractor { + override fun extractMetadata(graph: HeapGraph): Map { + val message = + graph.findClassByName("World")!!["message"]!!.valueAsInstance!!.readAsJavaString()!! + return mapOf("World message" to message) + } + } + + val analysis = hprofFile.checkForLeaks(metadataExtractor = extractor) + + val metadata = analysis.metadata + + assertThat(metadata).isEqualTo(mapOf("World message" to "Hello")) + } +} \ No newline at end of file diff --git a/shark/src/test/java/shark/TestUtil.kt b/shark/src/test/java/shark/TestUtil.kt index a55b5df501..491871ffb8 100644 --- a/shark/src/test/java/shark/TestUtil.kt +++ b/shark/src/test/java/shark/TestUtil.kt @@ -12,6 +12,7 @@ fun File.checkForLeaks( objectInspectors: List = emptyList(), computeRetainedHeapSize: Boolean = false, referenceMatchers: List = defaultReferenceMatchers, + metadataExtractor: MetadataExtractor = MetadataExtractor.NO_OP, proguardMapping: ProguardMapping? = null ): T { val inspectors = if (ObjectInspectors.KEYED_WEAK_REFERENCE !in objectInspectors) { @@ -21,10 +22,11 @@ fun File.checkForLeaks( } val heapAnalyzer = HeapAnalyzer(OnAnalysisProgressListener.NO_OP) val result = heapAnalyzer.analyze( - this, - referenceMatchers, - computeRetainedHeapSize, - inspectors, + heapDumpFile = this, + referenceMatchers = referenceMatchers, + computeRetainedHeapSize = computeRetainedHeapSize, + objectInspectors = inspectors, + metadataExtractor = metadataExtractor, proguardMapping = proguardMapping ) if (result is HeapAnalysisFailure) {