I'm a bit confused... since when does `cancel()` t...

# coroutines

[Stefan Oltmann]

I'm a bit confused... since when does cancel() throw a JobCancellationException to the calling thread?

kotlinx.coroutines.JobCancellationException: StandaloneCoroutine was cancelled; job="coroutine#2444":StandaloneCoroutine{Cancelling}@6b6879d8
	at kotlinx.coroutines.JobSupport.cancel(JobSupport.kt:1605)
	at kotlinx.coroutines.Job$DefaultImpls.cancel$default(Job.kt:189)

[Joffrey]

Are you cancelling that job from a coroutine running in that job? This behaviour would not surprise me in that situation

[Stefan Oltmann]

This code used to not throw back an exception before Kotlin 1.6.0 ...

fun sync(photoSourceId: PhotoSourceId): Flow<PhotoAction> = channelFlow {
    
    val existingJob = jobMap[photoSourceId]
    
    if (existingJob != null && existingJob.isActive)
        return@channelFlow

    val job = backgroundCoroutineScope.launch {

        // the work
    }

    jobMap[photoSourceId] = job

    job.join()
}

fun cancelSync(photoSourceId: PhotoSourceId) {


    val job = jobMap[photoSourceId]

    job?.let {

        it.cancel()

        jobMap.remove(photoSourceId)
    }
}

[Stefan Oltmann]

Both methods are called from my main thread

[Stefan Oltmann]

Maybe I got something wrong elsewhere, not sure

[Joffrey]

Are you calling cancelSync from inside the coroutine launched in sync?

[Stefan Oltmann]

No, I call both from the outside - separate launch on the main dispatcher

[Joffrey]

How did you determine that this exception is launched on the thread that calls cancel? Do you have a reproducible example?

[Stefan Oltmann]

Because the stacktrace above starts in my cancelSync() (I snipped that)

[Joffrey]

This might only show where the exception is built, but not which thread crashes as a result

[Joffrey]

Here: https://pl.kotl.in/d7FDktnkr

You can see that the stacktrace shows cancel() as the first line, and yet if you remove this catch, the main thread doesn't throw. I think you might have a try-catch-all somewhere, misleading you into thinking your main thread crashes

[Stefan Oltmann]

Oh, you are right. Thanks for that hint. I added a try catch block around that and indeed it does not come from cancel() ... that's confusing.

My internal sync() logic is wrapped with a try-catch like this:

} catch (ignore: CancellationException) {


    send(PhotoAction.SyncCanceled(photoSourceId))

}

I still land in there, but the exception is not ignored but now printed to my command line and even recognized by Sentry as uncaught. That's a new behaviour.

[Stefan Oltmann]

Maybe that's something Sentry changed. Don't know. I updated a lot of dependencies.

[Stefan Oltmann]

Thanks for your help. 🙏
I will find a way around this.

[Joffrey]

It is usually a bad idea to try and catch CancellationException. It's meant for the coroutines framework. If you catch it, you should at least rethrow it so the machinery can work properly.

Also, if the coroutine is cancelled, you likely won't be able so send anything on the channel because send is a suspend function and respects cancellation. You might work around this by using a finally block and withContext(NonCancellable), but I don't know your use case enough to know whether that would be a good idea

[Stefan Oltmann]

Yeah, I was thinking it would be good to have that mechanic, call yield() sometimes and ignore the cancellelationException

[Stefan Oltmann]

But maybe I should just use an atomatic boolean as a latch to signal the user wants to cancel the sync

[Joffrey]

You don't need yield if you already call some suspend functions (which you likely do in order to send items through the channelFlow

[Stefan Oltmann]

I noticed that without calling yield() at certain points I can stop the sync never cancels

[Stefan Oltmann]

because it's cooperative cancellation

[Joffrey]

Yes, what I mean is that calling suspend functions has the same effect as yield (built-in cooperation on cancellation, and opportunity to dispatch other coroutines). If you have loops without any suspend function calls, indeed it's better to add yield()

[Stefan Oltmann]

Yes, I use it in a for loop

[Stefan Oltmann]

But now as I can't ignore the exception because some mechanic still picks it up (I suspect the Sentry update) I need to change it anyway to prevent my logs from being spammed

[Joffrey]

It would be interesting to see the body of the // work here

[Stefan Oltmann]

private suspend fun downloadMissingThumbnailsForCloudService(
    photoRepository: PhotoRepository,
    cloudClient: CloudClient,
    photoSourceId: PhotoSourceId,
) {

    val photoIdsWithoutThumbnail =
        photoRepository.findAllPhotoIdsWithoutThumbnail(photoSourceId)

    for (photoId in photoIdsWithoutThumbnail) {

        /*
         * Give a chance to cancel.
         */
        yield()

        try {
            
            val thumbnailBytes = cloudClient.getThumbnailImageItemBytes(photoId)

            photoRepository.saveThumbnailBytes(
                photoId = photoId,
                bytes = thumbnailBytes
            )
            
        } catch (ex: TimeoutCancellationException) {
            Log.error("Receiving thumbnail for $photoId timed out.", ex)
        }
    }
}

[Stefan Oltmann]

Downloading 1000+ thumbnails from a cloud takes a while 😉

[Stefan Oltmann]

I could surely remove the yield() by making the called methods suspend if I understood you correctly 🤔

[Joffrey]

Yes that would be an option, but if the methods are not suspending then yes you need yield.

But wait where do you emit things to the channelFlow? I don't see anything about it

[Stefan Oltmann]

Yeah, I removed that here for better visibility. I have lots of those all over the place.

send(
    PhotoAction.ThumbnailStatusUpdate(
        photoIds = setOf(photoId),
        thumbnailStatus = ThumbnailStatus.LOADED
    )
)

[Stefan Oltmann]

I use a Redux pattern like the official kmm-production-sample does and fire back status updates for the UI to update trough this channel

[Joffrey]

The whole point I was making was that those send calls were suspending. If they are present in your loop, you don't need yield. But I don't see how they could be present in downloadMissingThumbnailsForCloudService since you don't have the producer scope of the channel flow there.
Sorry I guess the whole thing might be too entangled to share the idea without dumping the whole source

[Stefan Oltmann]

Oh wow, thank you a lot. Absolutely. I might no longer need the yield()

[Stefan Oltmann]

I added it as there were no send() calls in place and never gave that a second thought 🙈

[louiscad]

BTW, in place of yield(), you can also call currentCoroutineContext().ensureActive() if starving the dispatcher while no cancellation happened is not a concern (often, in infinite loops, it is though).

[Stefan Oltmann]

It's a bit annoying to replace that mechanic, because together with Kotlin Native I don't know how to do it... how to have a thing like Javas AtomicBoolean and communicate cancellation between different threads.