Is this correct and the most concise way of having...

# coroutines

[spand]

Is this correct and the most concise way of having cleanup in the implement CoroutineScope pattern?

class Activity : CoroutineScope {
    var job = Job()
    override val coroutineContext: CoroutineContext
        get() = Dispatchers.Default + job

    init {
        val workJob = launch(start = CoroutineStart.LAZY) {
            // Doing stuff
            delay(5000)
        }
        launch(start = CoroutineStart.ATOMIC) {
            try {
                workJob.join()
            } finally {
                println("Cleaning up")
            }
        }
    }

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

[gildor]

Actually this should be enough:

class Activity : CoroutineScope {
   override val coroutineContext = Dispatchers.UI + Job()


  override fun onDestroy() {
      coroutineContext.cancel()
  } 
}

[gildor]

Not sure what code in init block should show, doesn’t make a lot of sense for me with all those ATOMIC and additional lazy launch
But maybe I don’t see some important usecase

[spand]

I need to perform an action once the job is done.. either by cancellation or completion.

[spand]

Preferably within the scope of Job so job.join() will return after cleanup

[gildor]

Why not just use invokeOnCompletion?

[spand]

Cant launch new suspendable functions

[gildor]

inside of invokeOnCompletion?

[spand]

yes

[gildor]

I see

[gildor]

in this case you probably need to coroutines yeah

[gildor]

Also curious about clean approach for this
But can you close resources just using onDestroy?
Because if your function that cleanups resources is suspendable it also will be cancelled
Maybe more clear and stable solution would be just runBlocking in onDestroy or even invokeOnCompletion, otherwise you in trap of async closing operations

I of course don’t know everything about your case, so hard to tell will it be better or not

[spand]

The ATOMIC ensures the cleanup operation will run even if the parent is cancelling

[gildor]

yes, but what about suspend functions that you call there

[gildor]

you said that you need suspend function and you cannot use invokeOnComplete

[spand]

Good question. I would assume they inherit that status so they cannot be cancelled either

[gildor]

as I understand ATOMIC will guarantee only that such coroutine will be started, but if you call any suspend function such function will be just cancelled (I may be wrong, I just thought that it works like that)

[gildor]

I don’t think that atomic make coroutine non cancellable

[gildor]

it’s just about way how you start coroutine, atomic cannot be cancelled before actual start

[gildor]

But again, it’s just my understanding

[gildor]

You can use NonCancellable context for some cases, but not sure that it wat you whant here]

[spand]

A quick test seems to confirm that any launch will just be cancelled. Setting the NonCancellable context gives the appropriate behaviour.

[gildor]

Why you just cannot wrap your resource with try/finally in your original “worker” launch, you just shouldn’t acquire resource outside of try, as I understand this should work

[spand]

I can call additional suspend methods which I think is enough in my case albeit a bit fragile to rely on

[spand]

Yeah that would be simpler. I wonder why that got extracted in the first place 😉

[gildor]

Yeah, try to use:

launch {
            try {
                // Doing stuff
               delay(5000)
            } finally {
                println("Cleaning up")
            }
        }

[gildor]

But again, depends on what is “cleaning up”, it cannot be cancellable suspend function, only blocking or something with NonCancellable context

[Dico]

What about launching the cleanup coroutine in global scope?

[Dico]

Or just without the same parent?

[kingsley]

You can use withContext(NonCancellable) { ... } in finally I believe

But yea, Andrey already hit on all the points to consider