https://kotlinlang.org logo
Title
s

Scott Kruse

03/15/2021, 4:03 PM
So i have a coroutine that consumes a
ReceiveChannel
.
MainScope
is a basic scope tied to
Dispatchers.Main
-- For some reason, despite calling
cancel()
from the calling viewmodel's
onCleared
function, This job continues to run after back pressing and reentering the app. If i change the scope to
viewmodelScope
the coroutine is cancelled appropriately. Can anyone shed some light on this?
fun run(params: Params, onEachResult: (any: Any) -> Unit) {   

      mainScope.job = mainScope.launch {
            run(params).consumeEach(onEachResult)
      }

   }

   fun cancel() {
        mainScope.job.cancel()
   }

    internal class MainScope(private val mainContext: CoroutineContext) : CoroutineScope {

        var job: Job = Job()
        private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
            showError(throwable)
        }

        override val coroutineContext: CoroutineContext = mainContext + job + exceptionHandler

        fun showError(t: Throwable) {
            Timber.e(t)
            throw t
        }
    }
z

Zach Klippenstein (he/him) [MOD]

03/15/2021, 4:19 PM
Is
run
getting called multiple times? If it is, by the time
cancel
is called,
mainScope.job
will be the leaf job in a tree of jobs, and since you have no reference to the scope’s root job, there’s no way to cancel any of the other jobs. The implementation of
MainScope
is very surprising – it’s highly unusual for a `CoroutineScope`’s job to change after it’s created. This makes the code very weird to reason about – I would make
job
immutable. It makes the scope effectively disable structured concurrency, which is pretty dangerous (and I’m guessing somehow the cause for your bug).
👍 1
s

Scott Kruse

03/15/2021, 4:27 PM
ty for responding. Yes
run
has the potential to be called multiple times
var job: Job = Job()

val coroutineContext: CoroutineContext = mainContext + job + exceptionHandler
Would this not imply the scope's root job?
z

Zach Klippenstein (he/him) [MOD]

03/15/2021, 4:35 PM
Yes, sorry. But that’s not the job you’re cancelling in
cancel
.
You’re using
mainScope.job
, not
mainScope.coroutineContext.job
.
:mind-blown: 1
But since
MainScope
is a
CoroutineScope
, you can also just call
mainScope.cancel()
, which calls
coroutineScope.job.cancel()
under the hood. This confusion is part of the reason why
MainScope
is smelly.
s

Scott Kruse

03/15/2021, 4:45 PM
Okay. so even though im assigning job reference to
mainScope.job
and calling
mainScope.job.cancel()
this is not sufficiently canceling since the job has become a leaf job potentially if
run
is called twice? If
run
was only to be called once it would behave ok since it would be the only job
mainScope
has launched
mainScope.job = mainScope.launch {
            run(params).consumeEach(onEachResult)
      }
z

Zach Klippenstein (he/him) [MOD]

03/15/2021, 4:48 PM
it would still be sketchy, since any other jobs that were started from that scope before you re-assigned the job would also be leaked. Maybe you’re looking for
cancelChildren
?
s

Scott Kruse

03/15/2021, 4:51 PM
Yeah definietly would be sketchy. Thanks for your help on this man, truly cleared up some confusion around the concepts. I'll be removing the
MainScope
implementation as it's literally only used for this one job so imo there's really not a need to have a dedicated scope that's not lifecycle aware.
z

Zach Klippenstein (he/him) [MOD]

03/15/2021, 4:53 PM
Yea, that sounds like a much cleaner (and more idiomatic) approach
1