https://kotlinlang.org logo
Join Slack
Powered by
When dealing with a lifecycle-aware component, suc...
# coroutines
m
When dealing with a lifecycle-aware component, such as a Presenter, where you may have repeated `onStart`/`onStop` toggling, is it okay to use 
cancelChildren()
while never explicitly cancelling the parent 
Job
itself? This way when 
onStart()
is called, you can continue reusing the 
Job
for launching more coroutines?
In Rx land, typically a 
CompositeDisposable
is used where instead of invoking 
dispose()
, you can use 
clear()
so that future 
Disposables
can be added should the lifecycle begin again for the same instance of the lifecycle-aware component.
Let's assume that this lifecycle-aware Presenter does not know when it is being destroyed and therefore does not have an 
onDestroy
.
w
seems like a micro optimization to me
Have you looked at having your activity/fragment implement 
CoroutineScope
and the examples on github?
m
I have. But typically we move the logic away from the Activity. New code is using Android's 
ViewModel
so it's pretty straightforward to override 
onCleared()
and cancel the entire parent 
Job
. But we have a lot of older code that is following MVP with our own definition of Presenters. These presenters only have an 
onTake
and 
onDrop
callback which are tied to the 
onStart()
and 
onStop()
events of the Activity.
Ah, okay. Will take a look. Thank you!
👍 1
Looking at the structured concurrency example it seems that for each invocation of 
coroutineContext
, a new 
SupervisorJob
instance will be created.
Essentially the 
onPause()
will cancel a different instance of 
SupervisorJob
which is not the same instance that is being used in 
onResume
to launch the coroutine.
w
Actually, it is a 
val
property, so it get’s set once, unless the activity is destroyed anyways
The only reason I have it that way is because the way presented https://github.com/Kotlin/kotlinx.coroutines/blob/master/ui/coroutines-guide-ui.md#structured-concurrency-lifecycle-and-coroutine-parent-child-hierarchy, using 
by MainScope()
was saying that it is deprecated
Your question has me thinking, I’m going to look into the 
cancelChildren
call 😄
m
Just ran the following test:
Copy code
fun main(args: Array<String>) {
    val person = Person()
	println(person.name)
    println(person.name)
    println(person.name)
}

class Person {
    var counter = 0
    val name: String get() = "User id ${counter++}"
}
And the results were:
Copy code
User id 0
User id 1
User id 2
Using 
get()
for a 
val
property causes the expression to be evaluated every time you access the property. It's like having a 
getter
function that does some work before returning.
w
Woops, read that wrong
you’re right
m
Which is why in your post, I believe 
onPause
is creating a new 
SupervisorJob
instance and then immediately invoking 
cancel
on it, essentially leaking the previous one.
💯 1
w
Yep, need to fix that
m
That's most likely why pausing and then resuming the Activity still seems to be working as far as successfully launching coroutines.
If you were to cache the 
SupervisorJob
and continue to invoke 
cancel
, then 
onResume
will fail to launch the coroutine the second time around.
Which leads me to my original question 🙂. Is it okay to use 
cancelChildren
instead and allow the Activity to be destroyed without ever officially invoking 
cancel()
on the parent 
Job
?
w
So I did a quick test with the following
Copy code
email_sign_in_button.onClick(coroutineContext) {
//            clickEventChannel.send(SignInClickEvent.SignInButton)
            val ctx1 = coroutineContext
            val ctx2 = coroutineContext

            println(ctx1)
            println("=======")
            println(ctx2)

        }
result:
Copy code
[StandaloneCoroutine{Active}@93c7f7b, Main]
=======
[StandaloneCoroutine{Active}@93c7f7b, Main]
As well as testing for the same job with :
Copy code
email_sign_in_button.onClick(coroutineContext) {
//            clickEventChannel.send(SignInClickEvent.SignInButton)
            val ctx1 = coroutineContext
            val ctx2 = coroutineContext

            println(ctx1[Job])
            println("=======")
            println(ctx2[Job])

        }
with similar results
m
Hmm... 🤔
w
I think I’ve made myself even more confused tbqh
m
😆
w
For your actual question, I imagine that if all the children are already canceled then destroying the 
Job
is fine
Sorry for the derail
m
No problem. Essentially, the Activity will be destroyed where the parent 
Job
is still 
active
, however all of it's children will be cancelled.
💯 1
w
Nevermind, I did some more experimenting and every call to 
coroutineContext
, using the correct property, returns a new context
m
Ah ok.
w
I did a bunch more testing and your solution of 
cancelChildren
seems a good way to go, I need to update my examples to use it
👍 1
m
I'm still not 100% sure though if it is okay to keep the parent 
Job
as active if we never call 
cancel
on it though. I'd like to think it is okay. 🤔
w
I think we could also cancel it in onDestroy
m
Typically, our presenters don't expose an 
onDestroy
. As mentioned before, with RxJava, we use 
compositeDisposable.clear()
instead of 
compositeDisposable.dispose()
and leave the 
CompositeDisposable
as undisposed.
w
Oh sorry, I didn't realize you're cancelling the jobs in the presenter itself.
m
Yeah. Trying to integrate Coroutines into legacy MVP code that sorta missed the Jetpack train when it was written 😞
For new code, we're using MVVM and leverage Android's ViewModel.
g
Looking at the structured concurrency example it seems that for each invocation of 
coroutineContext
, a new 
SupervisorJob
instance will be created.
No, it shouldn’t work like that, you should keep Job until scope cancellation and recreate your job if some onStart method may be called a few times
using 
by MainScope()
was saying that it is deprecated
No, it’s not deprecated, it’s experimental, it means that this API might be changed in the future, but it doesn’t mean that it deprecated
💯 1
with RxJava, we use 
compositeDisposable.clear()
instead of 
compositeDisposable.dispose()
and leave the 
CompositeDisposable
as undisposed.
You can do this with coroutines scope, just use cancelChildren()
m
Because I've been uncertain as to whether it is okay to only 
cancelChildren()
, I've been using approach similar to the following:
Copy code
private var scope by Delegates.notNull<CoroutineScope>()

fun onStart() {
    scope = CoroutineScope(SupervisorJob() + Dispatchers.Main)
    haveFunAndLaunchSomeCoroutines()
}

fun onStop() {
    scope.cancel()
}
g
yes, you can use this too
👍 1
just depends on your case
m
I personally prefer the 
cancelChildren()
approach since you get to use a 
val
and avoid the initialization in the 
onStart
.
g
possible problem with cancelChildren() and with CompositeDisposable.clean() is that there is a chance to leak some operation that started after onStop()
of course it some sort of programming error, you shouldn’t start new operation if state is not supported it, but it would be hard to find such error
m
Good point! Fortunately, we avoid performing asynchronous tasks like this after 
onStop()
.
g
Yes, everyone tries to avoid it, but I saw a bunch of error related to this, for example even some actions like click on view are asyncronous so you can get callback after onStop
you can write code that prevents such bugs, but if you cancel scope you don’t have this problem, because started coroutine will be immediately cancelled
👍 2
w
Yep you're right @gildor I misspoke
3 Views
Open in Slack
PreviousNext
Powered by