One problem we have on our team when onboarding developers to Coroutines is the complex access to the Job in a CoroutineScope - specifically scope.coroutineContext[Job]!!. Access to methods like .invokeOnCompletion, cancel, etc are unwieldy to use because of the Job nullability and violation of the Law of Demeter.
// easy to use/understand
scope.invokeOnCompletion { }
scope.cancel { }
// difficult to understand
scope.coroutineContext[Job]!!.invokeOnCompletion { }
scope.coroutineContext[Job]!!..cancel { }
The latter is more difficult because you have to understand that a CoroutineScope is just a wrapper around a CoroutineContext that will always have a Job (even though the API says a Job can be null). And once it is undestood, it's a pretty verbose API.
I see two possible solutions going forward (though both could be implemented).
.cancel(), invokeOnCompletion{ }, etc as extension functions on CoroutineScope.scope.job that returns an non-null Job - because a Job should never be null for a CoroutineScope.note: scope.cancel() was added in https://github.com/Kotlin/kotlinx.coroutines/pull/866
We have been thinking about this for a while.
The problem with a lot of extensions of CoroutinesScope is its mental model for users that a new to kotlinx.coroutines. When scope API surface duplicates Job one, it becomes a source of confusion "why should I use both" and unclear semantics. While standalone .job extension may be useful for advanced usages, we are still not sure. Could you please elaborate what Job API are you using apart from invokeOnCompletion?
This issue is more about design of the mental model and clear understanding of Job and CoroutineScope concepts in the library, implementation is straightforward and trivial, so if you need it right now you can introduce an extension property/method in your project.
what Job API are you using apart from invokeOnCompletion?
We are using:
job.cancel()
job.cancelAllChildren()
job.invokeOnCompletion()
job.isCompleted // rare
job.isActive // rare
job.cancelAndJoin() // rare
Could you please elaborate what Job API are you using apart from invokeOnCompletion
I'm currently using it to signal a ViewModel.onCleared() event, instead of relying on extending the android arch components ViewModel
While standalone .job extension may be useful for advanced usages, we are still not sure.
Ultimately, I think it's important to understand that the handle on coroutines created from a CoroutineScope is a Job. Is it safe to assume that all CoroutineScope instances should have a Job associated with this?
I don't think it's very important (for beginners) to understand that a Job lives inside a CoroutineContext. In most common usage - the interesting bits of using a CoroutineScope are what dispatcher is used, and being able to control the scope's lifecycle via Job. I think this is an argument in favor of a standalone .job extension.
val scope = CoroutineScope(Dispatchers.Main) // specify the dispatcher
// control lifecycle
scope.job.invokeOnCompletion { }
The above is simple to onboard beginner devs to. The following is not:
val scope = CoroutineScope(Dispatchers.Main)
scope.coroutineContext[Job]!!.invokeOnCompletion { }
It requires a dev to understand that:
Job is a special part of a CoroutineContextJob.Companion as a Key<Job> for accessing the Job instance of a CoroutineContextJob should never be null in a CoroutineScope, so it is safe to force unwrap it (!!)Job will automatically be created for you if you don't specify itNot requiring a dev to understand those things at the very beginning for the very basics would be a big win
Even if we ignore the mental model and discoverability issues, I am afraid that introducing job property will be a source-breaking change.
E.g.
class Activity() : CoroutineScope by CoroutineScope(EmptyCoroutineContext) {
val job = Job()
suspend fun foo() {
withContext(Dispatchers.IO) {
println("Cancelling $job")
job.cancel()
}
}
}
If we introduce job extension, foo semantics will silently change.
We can workaround it with LowPriorityInOverloadResolution, but it's still a subtle topic
I'd use this opportunity to take a better look at Job use-cases and cover the missing ones with CoroutineScope. I see two main missing pieces that require a Job here:
cancelChildren() -- this is easy to add. The only problem with it is that, in general, this function is extremely hard to use right in a race-free manner. Can you elaborate what are use-cases for this particular function?
invokeOnCancellation { ... } -- this is quite an advanced low-level function, because, for one thing, it does not specify which context it is invoked in, so it is easy to use incorrectly in a non-thread-safe manner. Can you give a bit more of example of how you use it, please. We might be able to design a safer alternative that would be a better fit for CoroutineScope extension.
invokeOnCancellation { ... }-- this is quite an advanced low-level function
Note: I'm using job.invokeOnCompletion { }, not invokeOnCancellation { }
Can you give a bit more of example of how you use it, please.
Sure. Consider how the Android ViewModel has the onCleared() callback. This is equivalent to the view model's "scope" being cancelled. Therefore, I have a custom ViewModel implementation that does not have an onCleared() callback. Instead, I leverage the scope it's injected with to handle additional cancellation that isn't already covered by a CoroutineScope being cancelled.
class MyViewModel(private val scope: CoroutineScope) {
init {
scope.coroutineContext[Job]!!.invokeOnCompletion {
// clean up anything that would need to be cleaned up in response to `onCleared()`
}
}
cancelChildren()-- this is easy to add. The only problem with it is that, in general, this function is extremely hard to use right in a race-free manner. Can you elaborate what are use-cases for this particular function?
I actually don't use that function at all for the race condition reasons. Some other members of my team like it because it gives them cleanup at a low level of effort. They use it to cancel network requests that are running at a "global" scope. (think like a CoroutineScope that is scoped to an Android Application lifecycle, which is effectively a singleton)
There's also this example
Sure. Consider how the Android
ViewModelhas theonCleared()callback. This is equivalent to the view model's "scope" being cancelled. Therefore, I have a customViewModelimplementation that does not have anonCleared()callback. Instead, I leverage the scope it's injected with to handle additional cancellation that isn't already covered by aCoroutineScopebeing cancelled.
What kind of cleanup you'd typically do in a ViewModel using invokeOnCompletion this way? Does it require manipulating UI?
As a side note, the original design was based on architecture in which ViewModel _is a scope_ or _owns a scope_ instead of _gets injected with scope_. It makes a big different with respect to cleanup, because when view model _owns a scope_ it has its own clear/close/cancel function that does all cleanup, like this:
class MyViewModel {
// I own the scope, as opposed to somebody else owning the scope and inject it to me
private val scope = MainScope()
fun clear() {
scope.cancel()
// clean up anything that would need to be cleaned up in response to `onCleared()`
}
}
So my other question would be if you have to inject a scope into the view model and why.
What kind of cleanup you'd typically do in a ViewModel using invokeOnCompletion this way?
Because I use the injected CoroutineScope to do all my cleanup, it happens automatically 馃憤 . The only reason to use scope.job.invokeOnCompletion { .. } would be to do non-coroutine cleanup (.clear() a CompositeDisposable or something).
Does it require manipulating UI?
No. They just expose a ReceiveChannel<State> (soon to be Flow<State> 馃槃 )
when view model owns a scope it has its own
clear/close/cancelfunction that does all cleanup
onCleared() is a cancellation mechanism, but so is CoroutineScope. I don't see the reason to have both. This has some nice benefits when it comes to testing. For example, take the injected scope view model below:
class MyViewModel(scope: CoroutineScope) : ViewModel, CoroutineScope by scope { .. }
My BaseUnitTest class exposes a CoroutineScope that is cancelled when @After fun tearDown() is called. That is the CoroutineScope that MyViewModel is injected with during testing, so I get the automatic cleanup for free (though it should get GC'd anyway). There becomes no need to manually call viewModel.onCleared(), because it's injected with a scope that gets cancelled at the appropriate time.
// I own the scope, as opposed to somebody else owning the scope and inject it to me
I would actually disagree - it's whoever calls onCleared() who owns the scope. This is effectively the same as injected a CoroutineScope that gets cancelled appropriately.
Most helpful comment
We are using:
I'm currently using it to signal a
ViewModel.onCleared()event, instead of relying on extending the android arch componentsViewModelUltimately, I think it's important to understand that the handle on coroutines created from a
CoroutineScopeis aJob. Is it safe to assume that allCoroutineScopeinstances should have aJobassociated with this?I don't think it's very important (for beginners) to understand that a
Joblives inside aCoroutineContext. In most common usage - the interesting bits of using aCoroutineScopeare what dispatcher is used, and being able to control the scope's lifecycle viaJob. I think this is an argument in favor of a standalone.jobextension.The above is simple to onboard beginner devs to. The following is not:
It requires a dev to understand that:
Jobis a special part of aCoroutineContextJob.Companionas aKey<Job>for accessing theJobinstance of aCoroutineContextJobshould never benullin aCoroutineScope, so it is safe to force unwrap it (!!)Jobwill automatically be created for you if you don't specify itNot requiring a dev to understand those things at the very beginning for the very basics would be a big win