Oya Canli
Oya Canli

Reputation: 2516

Cancelling a callback inside a suspendCancellableCoroutine

I have wrapped a callback in suspendCancellableCoroutine to convert it to a suspend function:

suspend fun TextToSpeech.speakAndWait(text: String) : Boolean {
    val uniqueUtteranceId = getUniqueUtteranceId(text)
    speak(text, TextToSpeech.QUEUE_FLUSH, null, uniqueUtteranceId)
    return suspendCancellableCoroutine { continuation ->
        this.setOnUtteranceProgressListener(object : JeLisUtteranceProgressListener() {
            override fun onDone(utteranceId: String?) {
                if(utteranceId == uniqueUtteranceId) {
                    Timber.d("word is read, resuming with the next word")
                    continuation.resume(true)
                }
            }
        })
    }
}

I'm calling this function with the lifecycleScope coroutine scope of the fragment and I was assuming that it was cancelled when fragment is destroyed. However, LeakCanary reported that my fragment was leaking because of this listener and I verified with logs that the callback was called even after the coroutine is cancelled.

So it seems that wrapping with suspendCancellableCoroutine instead of suspendCoroutine does not suffice to cancel the callback. I guess I should actively check whether the job is active, but how? I tried coroutineContext.ensureActive() and checking coroutineContext.isActive inside the callback but IDE gives an error saying that "suspension functions can be called only within coroutine body" What else can I do to ensure that it doesn't resume if the job is cancelled?

Upvotes: 5

Views: 12705

Answers (3)

Pawel
Pawel

Reputation: 17258

If you want to remove your JeLisUtteranceProgressListener regardless of result (success, cancellation or other errors) you can instead use a classic try/finally block:

suspend fun TextToSpeech.speakAndWait(text: String) : Boolean {
    val uniqueUtteranceId = getUniqueUtteranceId(text)
    speak(text, TextToSpeech.QUEUE_FLUSH, null, uniqueUtteranceId)
    return try { 
        suspendCancellableCoroutine { continuation ->
            this.setOnUtteranceProgressListener(object : JeLisUtteranceProgressListener() {
                override fun onDone(utteranceId: String?) {
                    if(utteranceId == uniqueUtteranceId) {
                        Timber.d("word is read, resuming with the next word")
                        continuation.resume(true)
                    }
                }
        })
    } finally {
        this.setOnUtteranceProgressListener(null)
    }
}

Upvotes: 6

Oya Canli
Oya Canli

Reputation: 2516

In addition to the accepted answer, I recognized that continuation object has an isActive property as well. So alternatively we can check whether coroutine is still active inside the callback before resuming:

    return suspendCancellableCoroutine { continuation ->
        this.setOnUtteranceProgressListener(object : JeLisUtteranceProgressListener() 
        {
            override fun onDone(utteranceId: String?) {
                if(utteranceId == uniqueUtteranceId) {
                    if (continuation.isActive) {
                        continuation.resume(true)
                    }
                }
            }
        })
        continuation.invokeOnCancellation {
            this.setOnUtteranceProgressListener(null)
        }
    }

Upvotes: 4

Marko Topolnik
Marko Topolnik

Reputation: 200158

LeakCanary reported that my fragment was leaking because of this listener and I verified with logs that the callback was called even after the coroutine is cancelled.

Yes, the underlying async API is unaware of Kotlin coroutines and you have to work with it to explicitly propagate cancellation. Kotlin provides the invokeOnCancellation callback specifically for this purpose:

return suspendCancellableCoroutine { continuation ->
    this.setOnUtteranceProgressListener(object : JeLisUtteranceProgressListener() { 
        /* continuation.resume() */ 
    })
    continuation.invokeOnCancellation {
        this.setOnUtteranceProgressListener(null)
    }
}

Upvotes: 10

Related Questions