From 6d4d5bbeb27cf13ee6caf5cf21a8f9a16678feb4 Mon Sep 17 00:00:00 2001 From: Abhay Sood Date: Thu, 28 Nov 2024 21:32:11 +0530 Subject: [PATCH 1/2] fix(android): revert usage of reentrant lock for heartbeat and SDK initialization --- .../sh/measure/android/MeasureInternal.kt | 12 +++--- .../sh/measure/android/exporter/Heartbeat.kt | 42 ++++++++----------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/android/measure/src/main/java/sh/measure/android/MeasureInternal.kt b/android/measure/src/main/java/sh/measure/android/MeasureInternal.kt index 22b78d78c..9c49e7e6f 100644 --- a/android/measure/src/main/java/sh/measure/android/MeasureInternal.kt +++ b/android/measure/src/main/java/sh/measure/android/MeasureInternal.kt @@ -3,8 +3,6 @@ package sh.measure.android import android.os.Build import sh.measure.android.lifecycle.AppLifecycleListener import sh.measure.android.logger.LogLevel -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock /** * Initializes the Measure SDK and hides the internal dependencies from public API. @@ -43,7 +41,7 @@ internal class MeasureInternal(measureInitializer: MeasureInitializer) : AppLife private val dataCleanupService by lazy { measureInitializer.dataCleanupService } private val powerStateProvider by lazy { measureInitializer.powerStateProvider } private var isStarted: Boolean = false - private var startLock = ReentrantLock() + private var startLock = Any() fun init() { logger.log(LogLevel.Debug, "Starting Measure SDK") @@ -77,7 +75,7 @@ internal class MeasureInternal(measureInitializer: MeasureInitializer) : AppLife } fun start() { - startLock.withLock { + synchronized(startLock) { if (!isStarted) { registerCollectors() isStarted = true @@ -86,7 +84,7 @@ internal class MeasureInternal(measureInitializer: MeasureInitializer) : AppLife } fun stop() { - startLock.withLock { + synchronized(startLock) { if (isStarted) { unregisterCollectors() isStarted = false @@ -124,7 +122,7 @@ internal class MeasureInternal(measureInitializer: MeasureInitializer) : AppLife // session manager must be the first to be notified about app foreground to ensure that // new session ID (if created) is reflected in all events collected after the launch. sessionManager.onAppForeground() - startLock.withLock { + synchronized(startLock) { if (isStarted) { powerStateProvider.register() networkChangesCollector.register() @@ -137,7 +135,7 @@ internal class MeasureInternal(measureInitializer: MeasureInitializer) : AppLife override fun onAppBackground() { sessionManager.onAppBackground() - startLock.withLock { + synchronized(startLock) { if (isStarted) { cpuUsageCollector.pause() memoryUsageCollector.pause() diff --git a/android/measure/src/main/java/sh/measure/android/exporter/Heartbeat.kt b/android/measure/src/main/java/sh/measure/android/exporter/Heartbeat.kt index eae387315..68bfde0ae 100644 --- a/android/measure/src/main/java/sh/measure/android/exporter/Heartbeat.kt +++ b/android/measure/src/main/java/sh/measure/android/exporter/Heartbeat.kt @@ -8,8 +8,6 @@ import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.Future import java.util.concurrent.RejectedExecutionException import java.util.concurrent.TimeUnit -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock internal interface HeartbeatListener { fun pulse() @@ -28,8 +26,8 @@ internal class HeartbeatImpl( private val logger: Logger, private val scheduler: MeasureExecutorService, ) : Heartbeat { + @Volatile private var future: Future<*>? = null - private val lock = ReentrantLock() @VisibleForTesting internal val listeners = CopyOnWriteArrayList() @@ -39,30 +37,26 @@ internal class HeartbeatImpl( } override fun start(intervalMs: Long, initialDelayMs: Long) { - lock.withLock { - if (future != null) { - return - } - try { - future = scheduler.scheduleAtFixedRate( - { - listeners.forEach(HeartbeatListener::pulse) - }, - initialDelayMs, - intervalMs, - TimeUnit.MILLISECONDS, - ) - } catch (e: RejectedExecutionException) { - logger.log(LogLevel.Error, "Failed to start ExportHeartbeat", e) - return - } + if (future != null) { + return + } + try { + future = scheduler.scheduleAtFixedRate( + { + listeners.forEach(HeartbeatListener::pulse) + }, + initialDelayMs, + intervalMs, + TimeUnit.MILLISECONDS, + ) + } catch (e: RejectedExecutionException) { + logger.log(LogLevel.Error, "Failed to start ExportHeartbeat", e) + return } } override fun stop() { - lock.withLock { - future?.cancel(false) - future = null - } + future?.cancel(false) + future = null } } From 27e4078ca226c7e0fd71a511d2c7da3a0c4c1f9b Mon Sep 17 00:00:00 2001 From: Abhay Sood Date: Thu, 28 Nov 2024 22:32:11 +0530 Subject: [PATCH 2/2] fix(android): add keep rule for AndroidComposeView --- android/measure/consumer-rules.pro | 3 +++ 1 file changed, 3 insertions(+) diff --git a/android/measure/consumer-rules.pro b/android/measure/consumer-rules.pro index b9b2bd472..7c339d129 100644 --- a/android/measure/consumer-rules.pro +++ b/android/measure/consumer-rules.pro @@ -7,3 +7,6 @@ # Required to check if androidx-fragment is present in runtime classpath -keepnames class androidx.fragment.app.FragmentActivity + +# Required to find gesture targe for composables +-keepnames class androidx.compose.ui.platform.AndroidComposeView \ No newline at end of file