Fix memory leak in ComplicationDataSourceInfoRetriever
For reasons unknown the continuation object from
suspendCancellableCoroutine was not getting GCd and this was a
gc root that retained the activity via the compiler generated
coroutine continuations.
This patch 'fixes' this by introducing a helper class where
the callback can be explicitly nulled after we're done with it.
This is enough to prevent the activity being retained.
We also change the coroutine scope to a lifetime observer one
which automatically cancels at the appropriate times.
Test: Manual testing with android studio profiler
Bug: 249121838
Change-Id: Id3a47b91e5cca8ea4ba0c13b90d4f2dcecd4a591
diff --git a/wear/watchface/watchface-complications/src/main/java/androidx/wear/watchface/complications/ComplicationDataSourceInfoRetriever.kt b/wear/watchface/watchface-complications/src/main/java/androidx/wear/watchface/complications/ComplicationDataSourceInfoRetriever.kt
index 035c75f..99d37bb 100644
--- a/wear/watchface/watchface-complications/src/main/java/androidx/wear/watchface/complications/ComplicationDataSourceInfoRetriever.kt
+++ b/wear/watchface/watchface-complications/src/main/java/androidx/wear/watchface/complications/ComplicationDataSourceInfoRetriever.kt
@@ -51,6 +51,7 @@
import java.lang.IllegalArgumentException
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
+import kotlinx.coroutines.CancellableContinuation
private typealias WireComplicationProviderInfo =
android.support.wearable.complications.ComplicationProviderInfo
@@ -209,44 +210,53 @@
}
return suspendCancellableCoroutine { continuation ->
- val deathObserver = IBinder.DeathRecipient {
- continuation.resumeWithException(ServiceDisconnectedException())
- }
- service.asBinder().linkToDeath(deathObserver, 0)
-
- // Not a huge deal but we might as well unlink the deathObserver.
- continuation.invokeOnCancellation {
- safeUnlinkToDeath(service, deathObserver)
- }
-
+ val callback = PreviewComplicationDataCallback(service, continuation)
if (!service.requestPreviewComplicationData(
complicationDataSourceComponent,
complicationType.toWireComplicationType(),
- object : IPreviewComplicationDataCallback.Stub() {
- override fun updateComplicationData(
- data: android.support.wearable.complications.ComplicationData?
- ) {
- safeUnlinkToDeath(service, deathObserver)
- continuation.resume(data?.toApiComplicationData())
- }
- }
+ callback
)
) {
- safeUnlinkToDeath(service, deathObserver)
+ callback.safeUnlinkToDeath()
continuation.resume(null)
}
}
}
- internal fun safeUnlinkToDeath(
- service: IProviderInfoService,
- deathObserver: IBinder.DeathRecipient
- ) {
- try {
- service.asBinder().unlinkToDeath(deathObserver, 0)
- } catch (e: NoSuchElementException) {
- // This really shouldn't happen.
- Log.w(TAG, "retrievePreviewComplicationData encountered", e)
+ private class PreviewComplicationDataCallback(
+ val service: IProviderInfoService,
+ var continuation: CancellableContinuation<ComplicationData?>?
+ ) : IPreviewComplicationDataCallback.Stub() {
+ val deathObserver: IBinder.DeathRecipient = IBinder.DeathRecipient {
+ continuation?.resumeWithException(ServiceDisconnectedException())
+ }
+
+ init {
+ service.asBinder().linkToDeath(deathObserver, 0)
+
+ // Not a huge deal but we might as well unlink the deathObserver.
+ continuation?.invokeOnCancellation {
+ safeUnlinkToDeath()
+ }
+ }
+
+ override fun updateComplicationData(
+ data: android.support.wearable.complications.ComplicationData?
+ ) {
+ safeUnlinkToDeath()
+ continuation!!.resume(data?.toApiComplicationData())
+
+ // Re http://b/249121838 this is important, it prevents a memory leak.
+ continuation = null
+ }
+
+ internal fun safeUnlinkToDeath() {
+ try {
+ service.asBinder().unlinkToDeath(deathObserver, 0)
+ } catch (e: NoSuchElementException) {
+ // This really shouldn't happen.
+ Log.w(TAG, "retrievePreviewComplicationData encountered", e)
+ }
}
}
diff --git a/wear/watchface/watchface-editor/build.gradle b/wear/watchface/watchface-editor/build.gradle
index eb5993b..fcd5e21 100644
--- a/wear/watchface/watchface-editor/build.gradle
+++ b/wear/watchface/watchface-editor/build.gradle
@@ -33,6 +33,7 @@
api(libs.kotlinCoroutinesAndroid)
implementation("androidx.core:core:1.1.0")
+ implementation("androidx.lifecycle:lifecycle-runtime-ktx:2.5.1")
implementation("androidx.lifecycle:lifecycle-common-java8:2.3.1")
androidTestImplementation(project(":wear:watchface:watchface-complications-rendering"))
diff --git a/wear/watchface/watchface-editor/src/main/java/androidx/wear/watchface/editor/EditorSession.kt b/wear/watchface/watchface-editor/src/main/java/androidx/wear/watchface/editor/EditorSession.kt
index cde44f8..da41032 100644
--- a/wear/watchface/watchface-editor/src/main/java/androidx/wear/watchface/editor/EditorSession.kt
+++ b/wear/watchface/watchface-editor/src/main/java/androidx/wear/watchface/editor/EditorSession.kt
@@ -37,6 +37,7 @@
import androidx.annotation.UiThread
import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.lifecycleScope
import androidx.wear.watchface.complications.ComplicationDataSourceInfo
import androidx.wear.watchface.complications.ComplicationDataSourceInfoRetriever
import androidx.wear.watchface.complications.data.ComplicationData
@@ -66,7 +67,6 @@
import androidx.wear.watchface.style.UserStyleSchema
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.TimeoutCancellationException
import kotlinx.coroutines.android.asCoroutineDispatcher
@@ -289,7 +289,6 @@
"EditorSession.createOnWatchEditorSessionAsyncImpl"
).use {
try {
- val coroutineScope = CoroutineScope(Dispatchers.Main.immediate)
val editorRequest = editIntent.getParcelableExtra<ComponentName>(
Constants.EXTRA_WATCH_FACE_COMPONENT
)?.let {
@@ -307,13 +306,13 @@
editorRequest.watchFaceId,
editorRequest.initialUserStyle,
complicationDataSourceInfoRetrieverProvider,
- coroutineScope,
+ activity.lifecycleScope,
editorRequest.previewScreenshotParams
)
// But full initialization has to be deferred because
// [WatchFace.getOrCreateEditorDelegate] is async.
// Resolve only after init has been completed.
- withContext(coroutineScope.coroutineContext) {
+ withContext(activity.lifecycleScope.coroutineContext) {
withTimeout(EDITING_SESSION_TIMEOUT.toMillis()) {
session.setEditorDelegate(
// Either create a delegate for a new headless client or await an
@@ -744,7 +743,6 @@
releaseResources()
closed = true
editorSessionTraceEvent.close()
- coroutineScope.cancel()
activity = null
complicationDataSourceInfoRetrieverProvider = null
chooseComplicationDataSource = null
@@ -760,7 +758,6 @@
releaseResources()
EditorService.globalEditorService.removeCloseCallback(closeCallback)
editorSessionTraceEvent.close()
- coroutineScope.cancel()
activity?.finish()
activity = null
complicationDataSourceInfoRetrieverProvider = null