We started migration of our Android apps to Structured Concurrency implementation from kotlionx.coroutines (before we used own solution for that) and now I have question about CoroutineScope. Looks that for any UI component SupervisorJob is the best choice (it’s even mentioned on Coroutines guide, see https://github.com/Kotlin/kotlinx.coroutines/blob/master/docs/exception-handling.md#supervision), because you usually don’t want to cancel all the jobs is one of them failed on your UI component, this is especially important for

async

builder, because you handle result only when call

await

and don’t want to cancel all other coroutines and especially CoroutineScope of this UI component if one of our async jobs is failed, we can actually handle error when call await() And you need non-supervisor Job and scope only in case if you want to run a few operations in parallel and want to cancel them if one of them failed, but it’s specific and explicit case, at least in usual UI development So I have 2 points here: 1. Should coroutine guide recommend to use SupervisorJob for all UI components and not Job (what is now standard in documentation, for example check

CoroutineScope

docs which use Job in Android Activity). Otherwise it can cause a lot of very subtle bugs when you use CoroutineScope for UI development. 2. We need some easy way to start non-supervisor scope. Now the only way to do that is

CoroutineScope(coroutineContext + Job())

which is quite wordy way. maybe make sense to change

coroutineScope

default implementation and create new Job instance if outer context contains SupervisorJob If it make sense for someone, I would happy to create an issue and contribute, but maybe I missed some important use cases there

async should be multiple background calls. If it just one, you can use withContext()

What do you mean? Of course, I understand where to use async It can be used for multiple calls or as suspendable alternative for lazy But it means nothing for my concerns. You still can have multiple async operations, you just don’t want to cancel whole scope just because some of operations fail in some particular case

I don't think failing

async { … }

coroutine cancels the scope, but I think uncaught exceptions from the

await()

call do.

It does, check for example this code:

import kotlinx.coroutines.*

suspend fun main(args: Array<String>) {
    coroutineScope {
        val res1 = async {
            delay(100)
            throw IllegalStateException()
        }
        val res2 = async {
            delay(200)
            "job 2 result"
        }

        try {
            res1.await()
        } catch (e: Exception) {
            println("Job 1: failed $e")
        }

        try {
            val result = res2.await()
            println("Job 2: $result")
        } catch (e: Exception) {
            println("Job 2: failed $e")
        }
    }
}

@elizarov Why would

async { ... }

cancel its scope and not just rely on

await()

call to throw? That sounds unnatural to me...

Because it covers important use case: you run multiple

async

to do something in parallel and if one of them cancelled you don’t want to leak those coroutines, you just want to get error message and cancel other see this for reference https://github.com/Kotlin/kotlinx.coroutines/issues/552

1. Maybe it does make sense to recommend

SupervisorJob

. I’m not sure what would be most common error handling practice in android development, though.

2. I thought about something like that:

class SomeActivity : Activity, CoroutineScope {
    override val coroutineContext = Dispatchers.Main + SupervisorJob()
    
   suspend fun doSomeTaskInParallel() = coroutineScope {
         (0..10).map {
               // I want Job() semantics here, cancel all tasks in case of error, but will get SupervisorJob behaviour
               launch/async { }
         }
      }
   } 
}

But now I see that probably for a real case I would just use

awaitAll()

Can we achieve 2. with

launch(Job())

?

yes, this also should work, but coroutineScope may be more efficient, because starts undispatched coroutine, but not sure that it visible in real life. I just think that

coroutineScope

is more idiomatic for this case. But It can be done using another approaches, such as coroutines builders or just using awaitAll

I think

async

cancelling scope is risky. Does this happen also on normal cancellation (

CancellationException

or subclass)?

if you not cancelling scope you will not solve problem with hanging coroutines that not required anymore (upd: actually awaitAll solves this problem explicitly), so we have 2 approaches: Default and Supervisor

Isn't it inconsistent that

supervisorScope

has a

SupervisorJob

behavior, and ignores its parent's type, whereas

coroutineScope

depends on its parent to decide if it's a

Supervisor

or regular

Job

... or am I not understanding something?

@gildor stated the contrary in the beginning of this thread,

coroutineScope

ignores parent job type

coroutineScope uses any job that available in coroutine context, if no jobs are available new Job will be created. I propose to do that more explicit and create new Job if only SupervisorJob available and do not inherit supervisor behavior

So what does this mean?

maybe make sense to change

coroutineScope

default implementation and create new Job instance if outer context contains SupervisorJob

It means something like:

val existingJob = coroutineContext[Job].takeIf { it !is SupervisorJob }
CoroutineScope(existingJob ?: Job())

Until you raised this issue, I assumed

coroutineScope

ignored the outer context too 🙈

It means something like:

But it doesn’t. E.g.

GlobalScope.async {
        try {
            coroutineScope {
                async {
                    try {
                        delay(Long.MAX_VALUE)
                    } catch (e: Exception) {
                        println("Scoped async is cancelled")
                    }
                }
                throw Exception()
            }
        } catch (e: Exception) {
            println("Caught exception from coroutineScope")
        }

        delay(1000)
        println("Outer async is not cancelled")
    }

Yes, sure, You right

A right solution should also listen competition of parent job, probably add invokeOnComplete

Not sure what do you mean.

coroutineScope

will be cancelled, if outer job is cancelled.

Now yes

But you still would want the new Job to have the outer scope as its parent, to handle if the outer scope is cancelled? That doesn't seem to be what that snippet does...

Thread is too big and mixing multiple things at once, so I’m slightly lost. @gildor let’s focus on semantics of what we have and what you are missing.

coroutineScope

creates isolated coroutine scope, which is cancelled either when its body throws an exception, its child throws an exception or caller (if caller has a job) is cancelled. Cancellation is not propagated upwards to the caller, instead

coroutineScope

throws its exception. For example, it’s safe to write:

coroutineScope {
  async() { throw E() }.await()
}

Caller will not be cancelled,

coroutineScope

will throw

E

.

supervisorScope

has all the properties

coroutineScope

has except one: it is not cancelled if its child throws an exception. Having that in mind, could you please clarify which mechanism you are missing right now?