From c908c11620128db8b0c3cb206225536ba13c28b8 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 18 Dec 2024 14:41:29 -0800 Subject: [PATCH 01/20] Fix BroadcastReceiver ANR --- .../main/java/leakcanary/ScreenOffTrigger.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 60f8136d37..5c36b81779 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -42,20 +42,20 @@ class ScreenOffTrigger( context: Context, intent: Intent ) { - if (intent.action == ACTION_SCREEN_OFF) { - if (currentJob == null) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - currentJob = job - analysisExecutor.execute { + analysisExecutor.execute { + if (intent.action == ACTION_SCREEN_OFF) { + if (currentJob == null) { + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + currentJob = job val result = job.execute() currentJob = null analysisCallback(result) } + } else { + currentJob?.cancel("screen on again") + currentJob = null } - } else { - currentJob?.cancel("screen on again") - currentJob = null } } } From 23c0fda6933de25de234ce8a61202c2af496719d Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Thu, 26 Dec 2024 22:57:19 -0800 Subject: [PATCH 02/20] Fix BroadcastReceiver ANR --- .../main/java/leakcanary/ScreenOffTrigger.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 60f8136d37..5c36b81779 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -42,20 +42,20 @@ class ScreenOffTrigger( context: Context, intent: Intent ) { - if (intent.action == ACTION_SCREEN_OFF) { - if (currentJob == null) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - currentJob = job - analysisExecutor.execute { + analysisExecutor.execute { + if (intent.action == ACTION_SCREEN_OFF) { + if (currentJob == null) { + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + currentJob = job val result = job.execute() currentJob = null analysisCallback(result) } + } else { + currentJob?.cancel("screen on again") + currentJob = null } - } else { - currentJob?.cancel("screen on again") - currentJob = null } } } From e657cdb2fda2991599050b3466a612380acede1d Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 15:20:35 -0800 Subject: [PATCH 03/20] PR feedback Add a delayedScheduledExecutorService --- .../main/java/leakcanary/ScreenOffTrigger.kt | 78 +++++++++++++++---- .../leakcanary/internal/friendly/Friendly.kt | 3 +- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 5c36b81779..471a4c63b2 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -9,7 +9,11 @@ import android.content.Intent.ACTION_SCREEN_ON import android.content.IntentFilter import android.os.Build import java.util.concurrent.Executor +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit import leakcanary.internal.friendly.checkMainThread +import leakcanary.internal.friendly.checkNotMainThread import shark.SharkLog class ScreenOffTrigger( @@ -19,7 +23,7 @@ class ScreenOffTrigger( * The executor on which the analysis is performed and on which [analysisCallback] is called. * This should likely be a single thread executor with a background thread priority. */ - private val analysisExecutor: Executor, + analysisExecutor: Executor, /** * Called back with a [HeapAnalysisJob.Result] after the screen went off and a @@ -32,7 +36,19 @@ class ScreenOffTrigger( private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> SharkLog.d { "$result" } }, -) { + + /** + * Initial executor delay to wait for analysisExecutor to start analysis. + * + * If not provided initial delay is 100ms + */ + private val analysisExecutorDelayMillis: Long = + TimeUnit.SECONDS.toMillis( + INITIAL_EXECUTOR_DELAY_IN_MILLI + ), + ) { + + private val delayedScheduledExecutorService = DelayedScheduledExecutorService(analysisExecutor) @Volatile private var currentJob: HeapAnalysisJob? = null @@ -42,20 +58,24 @@ class ScreenOffTrigger( context: Context, intent: Intent ) { - analysisExecutor.execute { - if (intent.action == ACTION_SCREEN_OFF) { - if (currentJob == null) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - currentJob = job - val result = job.execute() - currentJob = null - analysisCallback(result) - } - } else { - currentJob?.cancel("screen on again") - currentJob = null + if (intent.action == ACTION_SCREEN_OFF) { + if (currentJob == null) { + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + currentJob = job + delayedScheduledExecutorService.schedule( + { + checkNotMainThread() + val result = job.execute() + currentJob = null + analysisCallback(result) + }, + analysisExecutorDelayMillis + ) } + } else { + currentJob?.cancel("screen on again") + currentJob = null } } } @@ -77,5 +97,33 @@ class ScreenOffTrigger( fun stop() { checkMainThread() application.unregisterReceiver(screenReceiver) + delayedScheduledExecutorService.shutDown() + } + + private class DelayedScheduledExecutorService(private val analysisExecutor: Executor) { + private val scheduledExecutor: ScheduledExecutorService by lazy { + Executors.newScheduledThreadPool(1) + } + + /** + * Runs the specified [action] after the an [analysisExecutorDelayMillis] + */ + fun schedule(action: Runnable, analysisExecutorDelayMillis: Long) { + scheduledExecutor.schedule( + { + analysisExecutor.execute(action) + }, + analysisExecutorDelayMillis, + TimeUnit.MILLISECONDS + ) + } + + fun shutDown() { + scheduledExecutor.shutdown() + } + } + + private companion object { + private const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 100L } } diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt index cf621fbfbb..e70a81a460 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt @@ -7,8 +7,9 @@ internal inline val mainHandler get() = leakcanary.internal.mainHandler internal inline fun checkMainThread() = leakcanary.internal.checkMainThread() +internal inline fun checkNotMainThread() = leakcanary.internal.checkNotMainThread() internal inline fun noOpDelegate(): T = leakcanary.internal.noOpDelegate() internal inline fun measureDurationMillis(block: () -> Unit) = - leakcanary.internal.measureDurationMillis(block) \ No newline at end of file + leakcanary.internal.measureDurationMillis(block) From 12901034429714e20381edf31e57207e377f0148 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 15:28:31 -0800 Subject: [PATCH 04/20] Fix formatting --- .../main/java/leakcanary/ScreenOffTrigger.kt | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index f5c3f7507a..964e0ea974 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -25,6 +25,16 @@ class ScreenOffTrigger( */ analysisExecutor: Executor, + /** + * The initial delay (in milliseconds) before the [analysisExecutor] starts + * + * If not specified, the default initial delay is set to 100 milliseconds. + */ + analysisExecutorDelayMillis: Long = + TimeUnit.SECONDS.toMillis( + INITIAL_EXECUTOR_DELAY_IN_MILLI + ), + /** * Called back with a [HeapAnalysisJob.Result] after the screen went off and a * heap analysis was attempted. This is called on the same thread that the analysis was @@ -36,19 +46,10 @@ class ScreenOffTrigger( private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> SharkLog.d { "$result" } }, +) { - /** - * Initial executor delay to wait for analysisExecutor to start analysis. - * - * If not provided initial delay is 100ms - */ - analysisExecutorDelayMillis: Long = - TimeUnit.SECONDS.toMillis( - INITIAL_EXECUTOR_DELAY_IN_MILLI - ), - ) { - - private val delayedScheduledExecutorService = DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) + private val delayedScheduledExecutorService = + DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) @Volatile private var currentJob: HeapAnalysisJob? = null @@ -69,10 +70,10 @@ class ScreenOffTrigger( currentJob = null analysisCallback(result) } - }else { - currentJob?.cancel("screen on again") - currentJob = null } + } else { + currentJob?.cancel("screen on again") + currentJob = null } } } @@ -99,7 +100,8 @@ class ScreenOffTrigger( private class DelayedScheduledExecutorService( private val analysisExecutor: Executor, - private val analysisExecutorDelayMillis: Long) { + private val analysisExecutorDelayMillis: Long + ) { private val scheduledExecutor: ScheduledExecutorService by lazy { Executors.newScheduledThreadPool(1) From 5b0eb49f87df0c7091e0c32615f27b0e684c714b Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 15:43:58 -0800 Subject: [PATCH 05/20] Fix typo --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 964e0ea974..440aff5397 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -108,7 +108,7 @@ class ScreenOffTrigger( } /** - * Runs the specified [action] after the an [analysisExecutorDelayMillis] + * Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ fun schedule(action: Runnable) { scheduledExecutor.schedule( From 4dc4a8cfd268fb159c90c816c0a636be679d14ec Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 15:20:35 -0800 Subject: [PATCH 06/20] PR feedback Adding delay Add a delayedScheduledExecutorService Fix BroadcastReceiver ANR --- .../main/java/leakcanary/ScreenOffTrigger.kt | 70 ++++++++++++++++--- .../leakcanary/internal/friendly/Friendly.kt | 3 +- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 5c36b81779..964e0ea974 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -9,7 +9,11 @@ import android.content.Intent.ACTION_SCREEN_ON import android.content.IntentFilter import android.os.Build import java.util.concurrent.Executor +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit import leakcanary.internal.friendly.checkMainThread +import leakcanary.internal.friendly.checkNotMainThread import shark.SharkLog class ScreenOffTrigger( @@ -19,7 +23,17 @@ class ScreenOffTrigger( * The executor on which the analysis is performed and on which [analysisCallback] is called. * This should likely be a single thread executor with a background thread priority. */ - private val analysisExecutor: Executor, + analysisExecutor: Executor, + + /** + * The initial delay (in milliseconds) before the [analysisExecutor] starts + * + * If not specified, the default initial delay is set to 100 milliseconds. + */ + analysisExecutorDelayMillis: Long = + TimeUnit.SECONDS.toMillis( + INITIAL_EXECUTOR_DELAY_IN_MILLI + ), /** * Called back with a [HeapAnalysisJob.Result] after the screen went off and a @@ -34,6 +48,9 @@ class ScreenOffTrigger( }, ) { + private val delayedScheduledExecutorService = + DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) + @Volatile private var currentJob: HeapAnalysisJob? = null @@ -42,20 +59,21 @@ class ScreenOffTrigger( context: Context, intent: Intent ) { - analysisExecutor.execute { - if (intent.action == ACTION_SCREEN_OFF) { - if (currentJob == null) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - currentJob = job + if (intent.action == ACTION_SCREEN_OFF) { + if (currentJob == null) { + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + currentJob = job + delayedScheduledExecutorService.schedule { + checkNotMainThread() val result = job.execute() currentJob = null analysisCallback(result) } - } else { - currentJob?.cancel("screen on again") - currentJob = null } + } else { + currentJob?.cancel("screen on again") + currentJob = null } } } @@ -77,5 +95,37 @@ class ScreenOffTrigger( fun stop() { checkMainThread() application.unregisterReceiver(screenReceiver) + delayedScheduledExecutorService.shutDown() + } + + private class DelayedScheduledExecutorService( + private val analysisExecutor: Executor, + private val analysisExecutorDelayMillis: Long + ) { + + private val scheduledExecutor: ScheduledExecutorService by lazy { + Executors.newScheduledThreadPool(1) + } + + /** + * Runs the specified [action] after the an [analysisExecutorDelayMillis] + */ + fun schedule(action: Runnable) { + scheduledExecutor.schedule( + { + analysisExecutor.execute(action) + }, + analysisExecutorDelayMillis, + TimeUnit.MILLISECONDS + ) + } + + fun shutDown() { + scheduledExecutor.shutdown() + } + } + + private companion object { + private const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 100L } } diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt index cf621fbfbb..e70a81a460 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt @@ -7,8 +7,9 @@ internal inline val mainHandler get() = leakcanary.internal.mainHandler internal inline fun checkMainThread() = leakcanary.internal.checkMainThread() +internal inline fun checkNotMainThread() = leakcanary.internal.checkNotMainThread() internal inline fun noOpDelegate(): T = leakcanary.internal.noOpDelegate() internal inline fun measureDurationMillis(block: () -> Unit) = - leakcanary.internal.measureDurationMillis(block) \ No newline at end of file + leakcanary.internal.measureDurationMillis(block) From 3bb0e921dded6bb8adbdff14a7aaa515b6dff83f Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 15:43:58 -0800 Subject: [PATCH 07/20] Fix typo --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 964e0ea974..440aff5397 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -108,7 +108,7 @@ class ScreenOffTrigger( } /** - * Runs the specified [action] after the an [analysisExecutorDelayMillis] + * Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ fun schedule(action: Runnable) { scheduledExecutor.schedule( From bd726041bf4028ed77a33bd5496e499ecf3c6de9 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Fri, 27 Dec 2024 16:29:02 -0800 Subject: [PATCH 08/20] Handle edge case for DelayedScheduledExecutorService with start/stop/start --- .../main/java/leakcanary/ScreenOffTrigger.kt | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 440aff5397..cfb48cd57f 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -23,14 +23,14 @@ class ScreenOffTrigger( * The executor on which the analysis is performed and on which [analysisCallback] is called. * This should likely be a single thread executor with a background thread priority. */ - analysisExecutor: Executor, + private val analysisExecutor: Executor, /** * The initial delay (in milliseconds) before the [analysisExecutor] starts * * If not specified, the default initial delay is set to 100 milliseconds. */ - analysisExecutorDelayMillis: Long = + private val analysisExecutorDelayMillis: Long = TimeUnit.SECONDS.toMillis( INITIAL_EXECUTOR_DELAY_IN_MILLI ), @@ -48,8 +48,7 @@ class ScreenOffTrigger( }, ) { - private val delayedScheduledExecutorService = - DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) + private var delayedScheduledExecutorService: DelayedScheduledExecutorService? = null @Volatile private var currentJob: HeapAnalysisJob? = null @@ -64,7 +63,7 @@ class ScreenOffTrigger( val job = analysisClient.newJob(JobContext(ScreenOffTrigger::class)) currentJob = job - delayedScheduledExecutorService.schedule { + delayedScheduledExecutorService?.schedule { checkNotMainThread() val result = job.execute() currentJob = null @@ -80,6 +79,7 @@ class ScreenOffTrigger( fun start() { checkMainThread() + initializeDelayedExecutor() val intentFilter = IntentFilter().apply { addAction(ACTION_SCREEN_ON) addAction(ACTION_SCREEN_OFF) @@ -94,8 +94,19 @@ class ScreenOffTrigger( fun stop() { checkMainThread() + shutDownDelayedExecutor() application.unregisterReceiver(screenReceiver) - delayedScheduledExecutorService.shutDown() + } + + private fun initializeDelayedExecutor(){ + if (delayedScheduledExecutorService == null) { + delayedScheduledExecutorService = DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis ) + } + } + + private fun shutDownDelayedExecutor(){ + delayedScheduledExecutorService?.shutdownNow() + delayedScheduledExecutorService = null } private class DelayedScheduledExecutorService( @@ -120,8 +131,8 @@ class ScreenOffTrigger( ) } - fun shutDown() { - scheduledExecutor.shutdown() + fun shutdownNow() { + scheduledExecutor.shutdownNow() } } From 14286c848aac116846cbc05bb47a856d6b188af0 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Tue, 31 Dec 2024 11:56:47 -0800 Subject: [PATCH 09/20] Minor polishes --- .../main/java/leakcanary/ScreenOffTrigger.kt | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index cfb48cd57f..c8a4373be1 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -11,6 +11,7 @@ import android.os.Build import java.util.concurrent.Executor import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit import leakcanary.internal.friendly.checkMainThread import leakcanary.internal.friendly.checkNotMainThread @@ -23,17 +24,14 @@ class ScreenOffTrigger( * The executor on which the analysis is performed and on which [analysisCallback] is called. * This should likely be a single thread executor with a background thread priority. */ - private val analysisExecutor: Executor, + analysisExecutor: Executor, /** * The initial delay (in milliseconds) before the [analysisExecutor] starts * * If not specified, the default initial delay is set to 100 milliseconds. */ - private val analysisExecutorDelayMillis: Long = - TimeUnit.SECONDS.toMillis( - INITIAL_EXECUTOR_DELAY_IN_MILLI - ), + analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI, /** * Called back with a [HeapAnalysisJob.Result] after the screen went off and a @@ -48,7 +46,8 @@ class ScreenOffTrigger( }, ) { - private var delayedScheduledExecutorService: DelayedScheduledExecutorService? = null + private var delayedScheduledExecutorService: DelayedScheduledExecutorService = + DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) @Volatile private var currentJob: HeapAnalysisJob? = null @@ -63,7 +62,7 @@ class ScreenOffTrigger( val job = analysisClient.newJob(JobContext(ScreenOffTrigger::class)) currentJob = job - delayedScheduledExecutorService?.schedule { + delayedScheduledExecutorService.schedule { checkNotMainThread() val result = job.execute() currentJob = null @@ -79,7 +78,6 @@ class ScreenOffTrigger( fun start() { checkMainThread() - initializeDelayedExecutor() val intentFilter = IntentFilter().apply { addAction(ACTION_SCREEN_ON) addAction(ACTION_SCREEN_OFF) @@ -94,21 +92,10 @@ class ScreenOffTrigger( fun stop() { checkMainThread() - shutDownDelayedExecutor() + delayedScheduledExecutorService.stop() application.unregisterReceiver(screenReceiver) } - private fun initializeDelayedExecutor(){ - if (delayedScheduledExecutorService == null) { - delayedScheduledExecutorService = DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis ) - } - } - - private fun shutDownDelayedExecutor(){ - delayedScheduledExecutorService?.shutdownNow() - delayedScheduledExecutorService = null - } - private class DelayedScheduledExecutorService( private val analysisExecutor: Executor, private val analysisExecutorDelayMillis: Long @@ -118,11 +105,13 @@ class ScreenOffTrigger( Executors.newScheduledThreadPool(1) } + private var scheduledFuture:ScheduledFuture<*>? = null + /** * Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ fun schedule(action: Runnable) { - scheduledExecutor.schedule( + scheduledFuture = scheduledExecutor.schedule( { analysisExecutor.execute(action) }, @@ -131,8 +120,8 @@ class ScreenOffTrigger( ) } - fun shutdownNow() { - scheduledExecutor.shutdownNow() + fun stop() { + scheduledFuture?.cancel(true) } } From dcbea4f7dba7dd2b676876aec5aa6092e0502396 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Tue, 31 Dec 2024 11:57:39 -0800 Subject: [PATCH 10/20] Minor clean up --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index c8a4373be1..190b673bc7 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -46,7 +46,7 @@ class ScreenOffTrigger( }, ) { - private var delayedScheduledExecutorService: DelayedScheduledExecutorService = + private val delayedScheduledExecutorService: DelayedScheduledExecutorService = DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) @Volatile @@ -106,7 +106,7 @@ class ScreenOffTrigger( } private var scheduledFuture:ScheduledFuture<*>? = null - + /** * Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ From 82a0b7193cbc0b148cfb66fb7d7716cb462c4387 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Tue, 31 Dec 2024 16:47:47 -0800 Subject: [PATCH 11/20] AnalysisJobHandler Introducing AnalysisJobHandler to share Job execution logic between triggers --- docs/leakcanary-for-releases.md | 15 ++- .../java/leakcanary/AnalysisJobHandler.kt | 120 ++++++++++++++++++ .../main/java/leakcanary/BackgroundTrigger.kt | 49 ++----- .../main/java/leakcanary/ScreenOffTrigger.kt | 96 ++------------ .../leakcanary/internal/friendly/Friendly.kt | 1 - 5 files changed, 147 insertions(+), 134 deletions(-) create mode 100644 leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt diff --git a/docs/leakcanary-for-releases.md b/docs/leakcanary-for-releases.md index 0f92591f5b..641848709a 100644 --- a/docs/leakcanary-for-releases.md +++ b/docs/leakcanary-for-releases.md @@ -57,17 +57,13 @@ class ReleaseExampleApplication : ExampleApplication() { // Starts heap analysis on background importance BackgroundTrigger( application = this, - analysisClient = analysisClient, - analysisExecutor = analysisExecutor, - analysisCallback = analysisCallback + analysisJobHandler = analysisJobHandler ).start() // Starts heap analysis when screen off ScreenOffTrigger( application = this, - analysisClient = analysisClient, - analysisExecutor = analysisExecutor, - analysisCallback = analysisCallback + analysisJobHandler = analysisJobHandler ).start() } @@ -98,6 +94,13 @@ class ReleaseExampleApplication : ExampleApplication() { ) } + private val analysisJobHandler by lazy { + AnalysisJobHandler( + analysisClient = analysisClient, + analysisExecutor = analysisExecutor, + analysisCallback = analysisCallback + ) + } // Cancels heap analysis if "heap_analysis_flag" is false. private val flagInterceptor = object : HeapAnalysisInterceptor { val remoteConfig by lazy { FirebaseRemoteConfig.getInstance() } diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt new file mode 100644 index 0000000000..77b8fa087b --- /dev/null +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt @@ -0,0 +1,120 @@ +package leakcanary + +import java.util.concurrent.Executor +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledFuture +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import kotlin.reflect.KClass +import shark.SharkLog + +class AnalysisJobHandler( + + private val analysisClient: HeapAnalysisClient, + /** + * The executor on which the analysis is performed and on which [analysisCallback] is called. + * This should likely be a single thread executor with a background thread priority. + */ + analysisExecutor: Executor, + + /** + * The initial delay (in milliseconds) before the [analysisExecutor] starts + * + * If not specified, the default initial delay is set to 100 milliseconds. + */ + analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI, + + /** + * Called back with a [HeapAnalysisJob.Result] after the screen went off and a + * heap analysis was attempted. This is called on the same thread that the analysis was + * performed on. + * + * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see + * logs). + */ + private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> + SharkLog.d { "$result" } + }, +) { + + private val delayedScheduledExecutorService: DelayedScheduledExecutorService = + DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) + + private var currentJob = AtomicReference() + + internal fun updateJobState( + jobStarterPoint: KClass<*>, + jobState: JobState + ) { + when (jobState) { + JobState.STARTED -> executeJobIfNeeded(jobStarterPoint) + JobState.STOPPED -> cancelExistingJob() + } + } + + internal fun shutdown() { + delayedScheduledExecutorService.shutdown() + } + + private fun executeJobIfNeeded(jobStarterPoint: KClass<*>) { + if (currentJob.get() != null) { + return + } + + val job = + analysisClient.newJob(JobContext(jobStarterPoint)) + if (currentJob.compareAndSet(null, job)) { + delayedScheduledExecutorService.schedule { + val result = job.execute() + analysisCallback(result) + } + } + } + + private fun cancelExistingJob() { + currentJob.getAndUpdate { job -> + job?.cancel("screen on again") + null + } + } + + private class DelayedScheduledExecutorService( + private val analysisExecutor: Executor, + private val analysisExecutorDelayMillis: Long + ) { + + private val scheduledExecutor: ScheduledExecutorService by lazy { + Executors.newScheduledThreadPool(1) + } + + private var scheduledFuture: ScheduledFuture<*>? = null + + /** + * Runs the specified [action] after an initial [analysisExecutorDelayMillis] + */ + fun schedule(action: Runnable) { + scheduledFuture = scheduledExecutor.schedule( + { + analysisExecutor.execute(action) + }, + analysisExecutorDelayMillis, + TimeUnit.MILLISECONDS + ) + } + + fun shutdown() { + scheduledFuture?.cancel(true) + scheduledExecutor.shutdownNow() + } + } + + internal enum class JobState { + STARTED, + STOPPED + } + + companion object { + const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 100L + } +} diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt index 9d4100367b..fd4534e497 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt @@ -3,54 +3,23 @@ package leakcanary import android.app.Application import leakcanary.internal.BackgroundListener import leakcanary.internal.friendly.checkMainThread -import shark.SharkLog -import java.util.concurrent.Executor + +import leakcanary.AnalysisJobHandler.JobState.STARTED +import leakcanary.AnalysisJobHandler.JobState.STOPPED class BackgroundTrigger( private val application: Application, - private val analysisClient: HeapAnalysisClient, - /** - * The executor on which the analysis is performed and on which [analysisCallback] is called. - * This should likely be a single thread executor with a background thread priority. - */ - private val analysisExecutor: Executor, - + private val analysisJobHandler: AnalysisJobHandler, processInfo: ProcessInfo = ProcessInfo.Real, - - /** - * Called back with a [HeapAnalysisJob.Result] after the app has entered background and a - * heap analysis was attempted. This is called on the same thread that the analysis was - * performed on. - * - * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see - * logs). - */ - private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> - SharkLog.d { "$result" } - }, ) { - @Volatile - private var currentJob: HeapAnalysisJob? = null - private val backgroundListener = BackgroundListener(processInfo) { appInBackgroundNow -> - if (appInBackgroundNow) { - check(currentJob == null) { - "Current job set to null when leaving background" - } - - val job = - analysisClient.newJob(JobContext(BackgroundTrigger::class)) - currentJob = job - analysisExecutor.execute { - val result = job.execute() - currentJob = null - analysisCallback(result) - } + val jobState = if (appInBackgroundNow) { + STARTED } else { - currentJob?.cancel("app left background") - currentJob = null + STOPPED } + analysisJobHandler.updateJobState(BackgroundTrigger::class, jobState) } fun start() { @@ -62,4 +31,4 @@ class BackgroundTrigger( checkMainThread() backgroundListener.uninstall(application) } -} \ No newline at end of file +} diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 190b673bc7..5420dd067a 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -8,71 +8,26 @@ import android.content.Intent.ACTION_SCREEN_OFF import android.content.Intent.ACTION_SCREEN_ON import android.content.IntentFilter import android.os.Build -import java.util.concurrent.Executor -import java.util.concurrent.Executors -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledFuture -import java.util.concurrent.TimeUnit import leakcanary.internal.friendly.checkMainThread -import leakcanary.internal.friendly.checkNotMainThread -import shark.SharkLog +import leakcanary.AnalysisJobHandler.JobState.STARTED +import leakcanary.AnalysisJobHandler.JobState.STOPPED class ScreenOffTrigger( private val application: Application, - private val analysisClient: HeapAnalysisClient, - /** - * The executor on which the analysis is performed and on which [analysisCallback] is called. - * This should likely be a single thread executor with a background thread priority. - */ - analysisExecutor: Executor, - - /** - * The initial delay (in milliseconds) before the [analysisExecutor] starts - * - * If not specified, the default initial delay is set to 100 milliseconds. - */ - analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI, - - /** - * Called back with a [HeapAnalysisJob.Result] after the screen went off and a - * heap analysis was attempted. This is called on the same thread that the analysis was - * performed on. - * - * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see - * logs). - */ - private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> - SharkLog.d { "$result" } - }, + private val analysisJobHandler: AnalysisJobHandler ) { - private val delayedScheduledExecutorService: DelayedScheduledExecutorService = - DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) - - @Volatile - private var currentJob: HeapAnalysisJob? = null - private val screenReceiver = object : BroadcastReceiver() { override fun onReceive( context: Context, intent: Intent ) { - if (intent.action == ACTION_SCREEN_OFF) { - if (currentJob == null) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - currentJob = job - delayedScheduledExecutorService.schedule { - checkNotMainThread() - val result = job.execute() - currentJob = null - analysisCallback(result) - } - } - } else { - currentJob?.cancel("screen on again") - currentJob = null + val jobState = if (intent.action == ACTION_SCREEN_OFF) + STARTED + else { + STOPPED } + analysisJobHandler.updateJobState(ScreenOffTrigger::class, jobState) } } @@ -92,40 +47,7 @@ class ScreenOffTrigger( fun stop() { checkMainThread() - delayedScheduledExecutorService.stop() application.unregisterReceiver(screenReceiver) - } - - private class DelayedScheduledExecutorService( - private val analysisExecutor: Executor, - private val analysisExecutorDelayMillis: Long - ) { - - private val scheduledExecutor: ScheduledExecutorService by lazy { - Executors.newScheduledThreadPool(1) - } - - private var scheduledFuture:ScheduledFuture<*>? = null - - /** - * Runs the specified [action] after an initial [analysisExecutorDelayMillis] - */ - fun schedule(action: Runnable) { - scheduledFuture = scheduledExecutor.schedule( - { - analysisExecutor.execute(action) - }, - analysisExecutorDelayMillis, - TimeUnit.MILLISECONDS - ) - } - - fun stop() { - scheduledFuture?.cancel(true) - } - } - - private companion object { - private const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 100L + analysisJobHandler.shutdown() } } diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt index e70a81a460..4ddd8bfae4 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt @@ -7,7 +7,6 @@ internal inline val mainHandler get() = leakcanary.internal.mainHandler internal inline fun checkMainThread() = leakcanary.internal.checkMainThread() -internal inline fun checkNotMainThread() = leakcanary.internal.checkNotMainThread() internal inline fun noOpDelegate(): T = leakcanary.internal.noOpDelegate() From a4a140dd300f28f414a72ddfd5b8000a3c965a31 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 00:26:03 -0800 Subject: [PATCH 12/20] Reduce scope of changes and go back to original approach of DelayedScheduledExecutorService --- .../java/leakcanary/AnalysisJobHandler.kt | 120 ------------------ .../main/java/leakcanary/BackgroundTrigger.kt | 47 +++++-- .../main/java/leakcanary/ScreenOffTrigger.kt | 93 ++++++++++++-- 3 files changed, 123 insertions(+), 137 deletions(-) delete mode 100644 leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt deleted file mode 100644 index 77b8fa087b..0000000000 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/AnalysisJobHandler.kt +++ /dev/null @@ -1,120 +0,0 @@ -package leakcanary - -import java.util.concurrent.Executor -import java.util.concurrent.Executors -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledFuture -import java.util.concurrent.TimeUnit -import java.util.concurrent.atomic.AtomicReference -import kotlin.reflect.KClass -import shark.SharkLog - -class AnalysisJobHandler( - - private val analysisClient: HeapAnalysisClient, - /** - * The executor on which the analysis is performed and on which [analysisCallback] is called. - * This should likely be a single thread executor with a background thread priority. - */ - analysisExecutor: Executor, - - /** - * The initial delay (in milliseconds) before the [analysisExecutor] starts - * - * If not specified, the default initial delay is set to 100 milliseconds. - */ - analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI, - - /** - * Called back with a [HeapAnalysisJob.Result] after the screen went off and a - * heap analysis was attempted. This is called on the same thread that the analysis was - * performed on. - * - * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see - * logs). - */ - private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> - SharkLog.d { "$result" } - }, -) { - - private val delayedScheduledExecutorService: DelayedScheduledExecutorService = - DelayedScheduledExecutorService(analysisExecutor, analysisExecutorDelayMillis) - - private var currentJob = AtomicReference() - - internal fun updateJobState( - jobStarterPoint: KClass<*>, - jobState: JobState - ) { - when (jobState) { - JobState.STARTED -> executeJobIfNeeded(jobStarterPoint) - JobState.STOPPED -> cancelExistingJob() - } - } - - internal fun shutdown() { - delayedScheduledExecutorService.shutdown() - } - - private fun executeJobIfNeeded(jobStarterPoint: KClass<*>) { - if (currentJob.get() != null) { - return - } - - val job = - analysisClient.newJob(JobContext(jobStarterPoint)) - if (currentJob.compareAndSet(null, job)) { - delayedScheduledExecutorService.schedule { - val result = job.execute() - analysisCallback(result) - } - } - } - - private fun cancelExistingJob() { - currentJob.getAndUpdate { job -> - job?.cancel("screen on again") - null - } - } - - private class DelayedScheduledExecutorService( - private val analysisExecutor: Executor, - private val analysisExecutorDelayMillis: Long - ) { - - private val scheduledExecutor: ScheduledExecutorService by lazy { - Executors.newScheduledThreadPool(1) - } - - private var scheduledFuture: ScheduledFuture<*>? = null - - /** - * Runs the specified [action] after an initial [analysisExecutorDelayMillis] - */ - fun schedule(action: Runnable) { - scheduledFuture = scheduledExecutor.schedule( - { - analysisExecutor.execute(action) - }, - analysisExecutorDelayMillis, - TimeUnit.MILLISECONDS - ) - } - - fun shutdown() { - scheduledFuture?.cancel(true) - scheduledExecutor.shutdownNow() - } - } - - internal enum class JobState { - STARTED, - STOPPED - } - - companion object { - const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 100L - } -} diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt index fd4534e497..77394a9bd7 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt @@ -3,23 +3,54 @@ package leakcanary import android.app.Application import leakcanary.internal.BackgroundListener import leakcanary.internal.friendly.checkMainThread - -import leakcanary.AnalysisJobHandler.JobState.STARTED -import leakcanary.AnalysisJobHandler.JobState.STOPPED +import shark.SharkLog +import java.util.concurrent.Executor class BackgroundTrigger( private val application: Application, - private val analysisJobHandler: AnalysisJobHandler, + private val analysisClient: HeapAnalysisClient, + /** + * The executor on which the analysis is performed and on which [analysisCallback] is called. + * This should likely be a single thread executor with a background thread priority. + */ + private val analysisExecutor: Executor, + processInfo: ProcessInfo = ProcessInfo.Real, + + /** + * Called back with a [HeapAnalysisJob.Result] after the app has entered background and a + * heap analysis was attempted. This is called on the same thread that the analysis was + * performed on. + * + * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see + * logs). + */ + private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> + SharkLog.d { "$result" } + }, ) { + @Volatile + private var currentJob: HeapAnalysisJob? = null + private val backgroundListener = BackgroundListener(processInfo) { appInBackgroundNow -> - val jobState = if (appInBackgroundNow) { - STARTED + if (appInBackgroundNow) { + check(currentJob == null) { + "Current job set to null when leaving background" + } + + val job = + analysisClient.newJob(JobContext(BackgroundTrigger::class)) + currentJob = job + analysisExecutor.execute { + val result = job.execute() + currentJob = null + analysisCallback(result) + } } else { - STOPPED + currentJob?.cancel("app left background") + currentJob = null } - analysisJobHandler.updateJobState(BackgroundTrigger::class, jobState) } fun start() { diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 5420dd067a..c1fddeb515 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -8,26 +8,68 @@ import android.content.Intent.ACTION_SCREEN_OFF import android.content.Intent.ACTION_SCREEN_ON import android.content.IntentFilter import android.os.Build +import java.util.concurrent.Executor +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledFuture +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference import leakcanary.internal.friendly.checkMainThread -import leakcanary.AnalysisJobHandler.JobState.STARTED -import leakcanary.AnalysisJobHandler.JobState.STOPPED +import shark.SharkLog class ScreenOffTrigger( private val application: Application, - private val analysisJobHandler: AnalysisJobHandler + private val analysisClient: HeapAnalysisClient, + /** + * The executor on which the analysis is performed and on which [analysisCallback] is called. + * This should likely be a single thread executor with a background thread priority. + */ + analysisExecutor: Executor, + + /** + * Called back with a [HeapAnalysisJob.Result] after the screen went off and a + * heap analysis was attempted. This is called on the same thread that the analysis was + * performed on. + * + * Defaults to logging to [SharkLog] (don't forget to set [SharkLog.logger] if you do want to see + * logs). + */ + private val analysisCallback: (HeapAnalysisJob.Result) -> Unit = { result -> + SharkLog.d { "$result" } + }, + + /** + * Initial delay to wait for analysisExecutor to start analysis + * + * If not provided, initial delay is 500ms + */ + private val analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI ) { + private var currentJob = AtomicReference() + + private val delayedScheduledExecutorService = DelayedScheduledExecutorService() private val screenReceiver = object : BroadcastReceiver() { override fun onReceive( context: Context, intent: Intent ) { - val jobState = if (intent.action == ACTION_SCREEN_OFF) - STARTED - else { - STOPPED + if (intent.shouldStartAnalysis()) { + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + if(currentJob.compareAndSet(null, job)){ + delayedScheduledExecutorService.schedule { + val result = job.execute() + analysisCallback(result) + } + } + } else { + delayedScheduledExecutorService.cancel() + currentJob.getAndUpdate { job -> + job?.cancel("screen on again") + null + } } - analysisJobHandler.updateJobState(ScreenOffTrigger::class, jobState) } } @@ -48,6 +90,39 @@ class ScreenOffTrigger( fun stop() { checkMainThread() application.unregisterReceiver(screenReceiver) - analysisJobHandler.shutdown() + } + + private fun Intent.shouldStartAnalysis():Boolean{ + return this.action == ACTION_SCREEN_OFF && currentJob.get() == null + } + + private class DelayedScheduledExecutorService( + private val analysisExecutorDelayMillis:Long, + private val analysisExecutor: Executor){ + + private var scheduledFuture:ScheduledFuture<*>?=null + + private val scheduledExecutorService:ScheduledExecutorService by lazy { + Executors.newScheduledThreadPool(1) + } + + /** Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ + fun schedule(action:Runnable){ + scheduledFuture = + scheduledExecutorService.schedule( + { analysisExecutor.execute(action)}, + analysisExecutorDelayMillis, + TimeUnit.MILLISECONDS + ) + } + + /** Cancels prior ScheduledExecutorService */ + fun cancel(){ + scheduledFuture?.cancel(true) + } + } + + companion object{ + const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 500L } } From 6c9778193125bb6732a174707f1903a6e3183343 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 00:27:14 -0800 Subject: [PATCH 13/20] Revert unwanted changes at docs --- docs/leakcanary-for-releases.md | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/docs/leakcanary-for-releases.md b/docs/leakcanary-for-releases.md index 641848709a..2aa153c57c 100644 --- a/docs/leakcanary-for-releases.md +++ b/docs/leakcanary-for-releases.md @@ -12,9 +12,9 @@ having a release crash reporting pipeline, with counts to prioritize fixes. LeakCanary for releases exposes APIs to run a heap analysis in release builds, in production. !!! danger - Everything about this is experimental. Running a heap analysis in production is not a very - common thing to do, and we're still learning and experimenting with this. Also, both the - artifact name and the APIs may change. +Everything about this is experimental. Running a heap analysis in production is not a very +common thing to do, and we're still learning and experimenting with this. Also, both the +artifact name and the APIs may change. ## Getting started @@ -57,13 +57,17 @@ class ReleaseExampleApplication : ExampleApplication() { // Starts heap analysis on background importance BackgroundTrigger( application = this, - analysisJobHandler = analysisJobHandler + analysisClient = analysisClient, + analysisExecutor = analysisExecutor, + analysisCallback = analysisCallback ).start() // Starts heap analysis when screen off ScreenOffTrigger( application = this, - analysisJobHandler = analysisJobHandler + analysisClient = analysisClient, + analysisExecutor = analysisExecutor, + analysisCallback = analysisCallback ).start() } @@ -94,13 +98,6 @@ class ReleaseExampleApplication : ExampleApplication() { ) } - private val analysisJobHandler by lazy { - AnalysisJobHandler( - analysisClient = analysisClient, - analysisExecutor = analysisExecutor, - analysisCallback = analysisCallback - ) - } // Cancels heap analysis if "heap_analysis_flag" is false. private val flagInterceptor = object : HeapAnalysisInterceptor { val remoteConfig by lazy { FirebaseRemoteConfig.getInstance() } From 70bcd8422895cf82b625c5dc71c8b4257ab300bd Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 00:28:00 -0800 Subject: [PATCH 14/20] Update formatting of leak-canary-for-releases.md --- docs/leakcanary-for-releases.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/leakcanary-for-releases.md b/docs/leakcanary-for-releases.md index 2aa153c57c..0f92591f5b 100644 --- a/docs/leakcanary-for-releases.md +++ b/docs/leakcanary-for-releases.md @@ -12,9 +12,9 @@ having a release crash reporting pipeline, with counts to prioritize fixes. LeakCanary for releases exposes APIs to run a heap analysis in release builds, in production. !!! danger -Everything about this is experimental. Running a heap analysis in production is not a very -common thing to do, and we're still learning and experimenting with this. Also, both the -artifact name and the APIs may change. + Everything about this is experimental. Running a heap analysis in production is not a very + common thing to do, and we're still learning and experimenting with this. Also, both the + artifact name and the APIs may change. ## Getting started From 7c3d83cdf1a7d1f696c869deae3e89c76c390e65 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 00:34:52 -0800 Subject: [PATCH 15/20] Revert unwanted indentation --- .../src/main/java/leakcanary/BackgroundTrigger.kt | 2 +- .../src/main/java/leakcanary/internal/friendly/Friendly.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt index 77394a9bd7..9d4100367b 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/BackgroundTrigger.kt @@ -62,4 +62,4 @@ class BackgroundTrigger( checkMainThread() backgroundListener.uninstall(application) } -} +} \ No newline at end of file diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt index 4ddd8bfae4..cf621fbfbb 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/friendly/Friendly.kt @@ -11,4 +11,4 @@ internal inline fun checkMainThread() = leakcanary.internal.checkMainThread() internal inline fun noOpDelegate(): T = leakcanary.internal.noOpDelegate() internal inline fun measureDurationMillis(block: () -> Unit) = - leakcanary.internal.measureDurationMillis(block) + leakcanary.internal.measureDurationMillis(block) \ No newline at end of file From 85c7c3d7e6fe97b7cdf55d91f3ac50f1d3890269 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 00:35:17 -0800 Subject: [PATCH 16/20] Convert currentJob AtomicReference to val --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index c1fddeb515..ed4921bce6 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -46,7 +46,7 @@ class ScreenOffTrigger( private val analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI ) { - private var currentJob = AtomicReference() + private val currentJob = AtomicReference() private val delayedScheduledExecutorService = DelayedScheduledExecutorService() private val screenReceiver = object : BroadcastReceiver() { From 63d563bc6a9086a7b7fd787fd470c7f575bce875 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Wed, 15 Jan 2025 23:55:31 -0800 Subject: [PATCH 17/20] PR feedback - INITIAL_EXECUTOR_DELAY_IN_MILLI -> INITIAL_EXECUTOR_DELAY_MILLIS - Remove accidental change on viz of analysisExecutor - Rely on Handler.postDelayed instead of DelayedScheduledExecutorService --- .../main/java/leakcanary/ScreenOffTrigger.kt | 67 +++++++------------ 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index ed4921bce6..29736148f4 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -8,11 +8,9 @@ import android.content.Intent.ACTION_SCREEN_OFF import android.content.Intent.ACTION_SCREEN_ON import android.content.IntentFilter import android.os.Build +import android.os.Handler +import android.os.Looper import java.util.concurrent.Executor -import java.util.concurrent.Executors -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledFuture -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicReference import leakcanary.internal.friendly.checkMainThread import shark.SharkLog @@ -24,7 +22,7 @@ class ScreenOffTrigger( * The executor on which the analysis is performed and on which [analysisCallback] is called. * This should likely be a single thread executor with a background thread priority. */ - analysisExecutor: Executor, + private val analysisExecutor: Executor, /** * Called back with a [HeapAnalysisJob.Result] after the screen went off and a @@ -41,30 +39,31 @@ class ScreenOffTrigger( /** * Initial delay to wait for analysisExecutor to start analysis * - * If not provided, initial delay is 500ms + * If not specified, the initial delay is 500 ms */ - private val analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_IN_MILLI + private val analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_MILLIS ) { private val currentJob = AtomicReference() + private val analysisHandler = Handler(Looper.getMainLooper()) + private var analysisRunnable: Runnable? = null - private val delayedScheduledExecutorService = DelayedScheduledExecutorService() private val screenReceiver = object : BroadcastReceiver() { override fun onReceive( context: Context, intent: Intent ) { if (intent.shouldStartAnalysis()) { - val job = - analysisClient.newJob(JobContext(ScreenOffTrigger::class)) - if(currentJob.compareAndSet(null, job)){ - delayedScheduledExecutorService.schedule { - val result = job.execute() - analysisCallback(result) - } + val job = + analysisClient.newJob(JobContext(ScreenOffTrigger::class)) + if (currentJob.compareAndSet(null, job)) { + schedule { + val result = job.execute() + analysisCallback(result) } + } } else { - delayedScheduledExecutorService.cancel() + cancelScheduledAction() currentJob.getAndUpdate { job -> job?.cancel("screen on again") null @@ -92,37 +91,21 @@ class ScreenOffTrigger( application.unregisterReceiver(screenReceiver) } - private fun Intent.shouldStartAnalysis():Boolean{ + private fun Intent.shouldStartAnalysis(): Boolean { return this.action == ACTION_SCREEN_OFF && currentJob.get() == null } - private class DelayedScheduledExecutorService( - private val analysisExecutorDelayMillis:Long, - private val analysisExecutor: Executor){ - - private var scheduledFuture:ScheduledFuture<*>?=null - - private val scheduledExecutorService:ScheduledExecutorService by lazy { - Executors.newScheduledThreadPool(1) - } - - /** Runs the specified [action] after an initial [analysisExecutorDelayMillis] */ - fun schedule(action:Runnable){ - scheduledFuture = - scheduledExecutorService.schedule( - { analysisExecutor.execute(action)}, - analysisExecutorDelayMillis, - TimeUnit.MILLISECONDS - ) - } + private fun schedule(action: Runnable) { + analysisRunnable = Runnable { + analysisExecutor.execute(action) + }.also { analysisHandler.postDelayed(it, analysisExecutorDelayMillis) } + } - /** Cancels prior ScheduledExecutorService */ - fun cancel(){ - scheduledFuture?.cancel(true) - } + private fun cancelScheduledAction() { + analysisRunnable?.let { analysisHandler.removeCallbacks(it) } } - companion object{ - const val INITIAL_EXECUTOR_DELAY_IN_MILLI = 500L + companion object { + private const val INITIAL_EXECUTOR_DELAY_MILLIS = 500L } } From e88dea6b8698b3432cad6aac588ecae2a5927e14 Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Thu, 16 Jan 2025 15:08:15 -0800 Subject: [PATCH 18/20] PR feedback - Use Duration type for analysisExecutorDelay - Rename to submitAnalysisToExecutor --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 29736148f4..4d2472f800 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -12,6 +12,8 @@ import android.os.Handler import android.os.Looper import java.util.concurrent.Executor import java.util.concurrent.atomic.AtomicReference +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds import leakcanary.internal.friendly.checkMainThread import shark.SharkLog @@ -41,12 +43,12 @@ class ScreenOffTrigger( * * If not specified, the initial delay is 500 ms */ - private val analysisExecutorDelayMillis: Long = INITIAL_EXECUTOR_DELAY_MILLIS + private val analysisExecutorDelay: Duration = DEFAULT_ANALYSIS_DELAY ) { private val currentJob = AtomicReference() private val analysisHandler = Handler(Looper.getMainLooper()) - private var analysisRunnable: Runnable? = null + private var submitAnalysisToExecutor: Runnable? = null private val screenReceiver = object : BroadcastReceiver() { override fun onReceive( @@ -96,16 +98,16 @@ class ScreenOffTrigger( } private fun schedule(action: Runnable) { - analysisRunnable = Runnable { + submitAnalysisToExecutor = Runnable { analysisExecutor.execute(action) - }.also { analysisHandler.postDelayed(it, analysisExecutorDelayMillis) } + }.also { analysisHandler.postDelayed(it, analysisExecutorDelay.inWholeMilliseconds) } } private fun cancelScheduledAction() { - analysisRunnable?.let { analysisHandler.removeCallbacks(it) } + submitAnalysisToExecutor?.let { analysisHandler.removeCallbacks(it) } } companion object { - private const val INITIAL_EXECUTOR_DELAY_MILLIS = 500L + private val DEFAULT_ANALYSIS_DELAY = 500.milliseconds } } From 3b192e795f7288fb4c9fb626efb76e5613f44dab Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Thu, 16 Jan 2025 16:01:31 -0800 Subject: [PATCH 19/20] Rename shouldStartAnalysis to shouldScheduleAnalysis --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 4d2472f800..2e0167da6d 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -55,7 +55,7 @@ class ScreenOffTrigger( context: Context, intent: Intent ) { - if (intent.shouldStartAnalysis()) { + if (intent.shouldScheduleAnalysis()) { val job = analysisClient.newJob(JobContext(ScreenOffTrigger::class)) if (currentJob.compareAndSet(null, job)) { @@ -93,7 +93,7 @@ class ScreenOffTrigger( application.unregisterReceiver(screenReceiver) } - private fun Intent.shouldStartAnalysis(): Boolean { + private fun Intent.shouldScheduleAnalysis(): Boolean { return this.action == ACTION_SCREEN_OFF && currentJob.get() == null } From 266866877e8dd76d4e6c781c1e9c0a4db09eface Mon Sep 17 00:00:00 2001 From: Fran Aguilera Date: Thu, 16 Jan 2025 16:02:48 -0800 Subject: [PATCH 20/20] Rename cancelScheduledAction to cancelScheduledAnalysis --- .../src/main/java/leakcanary/ScreenOffTrigger.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt index 2e0167da6d..7aed68c3b7 100644 --- a/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt +++ b/leakcanary/leakcanary-android-release/src/main/java/leakcanary/ScreenOffTrigger.kt @@ -65,7 +65,7 @@ class ScreenOffTrigger( } } } else { - cancelScheduledAction() + cancelScheduledAnalysis() currentJob.getAndUpdate { job -> job?.cancel("screen on again") null @@ -103,7 +103,7 @@ class ScreenOffTrigger( }.also { analysisHandler.postDelayed(it, analysisExecutorDelay.inWholeMilliseconds) } } - private fun cancelScheduledAction() { + private fun cancelScheduledAnalysis() { submitAnalysisToExecutor?.let { analysisHandler.removeCallbacks(it) } }