Shouldn't join resume after onCompletion?

[jcornaz]

Hello,

please consider the following code:

fun foo() {
  Thread.sleep(1000)
  println("alright this have been executed")
}

fun main(args: Array<String>) {
  val job = Job()

  job.cancel()

  GlobalScope.launch(job) {
    try {
      println("do stuff")
    } finally {
      foo()
    }
  }

  runBlocking {
    println("before join")
    job.cancelAndJoin()
    println("afterJoin")
  }
}

foo() is not invoked at all, because the job is cancelled before being scheduled.

So what are my options, if I want to make sure foo() is always executed?

One may try to use onCompletion like this:

fun foo() {
  Thread.sleep(1000)
  println("alright this have been executed")
}

fun main(args: Array<String>) {
  val job = Job()

  job.cancel()

  GlobalScope.launch(job, onCompletion = { foo() }) { println("do stuff") }

  runBlocking {
    println("before join")
    job.cancelAndJoin()
    println("afterJoin")
  }
}

But it doesn't help, because join doesn't seem to care if onCompletion have been executed or not.

And here comes my question: Shouldn't join resume only after the execution onCompletion?

[elizarov]

We have a feature for that — atomic start mode. launch(start = CoroutineStart.ATOMIC) { ... } seems to be doing what you are looking for, isn't it?

[jcornaz]

Thanks @elizarov. I can, indeed make sure that my example always execute foo.

But I don't really care if println("do stuff") is executed or not. And in fact I'd prefer not execute it if the job has been cancelled.

What I'm saying is that despite that the goal of onCompletion (#279) was to add a way to make sure a resource is closed/cancel even without the need to use CoroutineStart.ATOMIC, this goal is not completely achieved, as onCompletion may not be executed in the (extreme) condition shown in my example.

I'd like to quote yourself (in #279) about the solution of CoroutineStart.ATOMIC:

We can use produce(context, start = CoroutineStart.ATOMIC) { ... } in implementation of all the operators. It makes sure that coroutine is not cancellable until it starts to execute. This change provides an immediate relief for this problem, but it has a slight unintended side-effect that this "non-cancellable" period extends until the first suspension in the coroutine, while might be too long. It also feels extremely fragile. It is quite easy to forget that start parameter.

[jcornaz]

And still CoroutineStart.ATOMIC or not, I'd expect foo to be able print in console with the following code:

fun foo() {
  Thread.sleep(1000)
  println("alright this have been executed")
}

fun main(args: Array<String>) {
  val job = Job()

  job.cancel()

  GlobalScope.launch(job, start = CoroutineStart.ATOMIC, onCompletion = { foo() }) { println("do stuff") }

  runBlocking {
    println("before join")
    job.cancelAndJoin()
    println("after join")
  }
}

But the result is:

do stuff
before join
after join

And I'd like to get:

before join
alright this have been executed
after join

[elizarov]

We've deprecated onCompletion (and one of the reasons is the unpredictable order). However, your first example (with try/finally{ foo() }) should work as expected with start = ATOMIC.

[jcornaz]

However, your first example (with try/finally{ foo() }) should work as expected with start = ATOMIC

Yes indeed.

We've deprecated onCompletion (and one of the reasons is the unpredictable order)

Ok. thanks.

Before I close this issue, may I ask what will be the preferred solution to #279? Shall we use CoroutineStart.ATOMIC ?

[elizarov]

For now, start = ATOMIC with try / finally { ... } is the only guaranteed way to make sure that something is done before other coroutines that .join resume. In the future we might add back support or something similar to onCompletion, but it has to be composable (don't want to hack every coroutine builder again).

Btw, if you are only interested in exceptions, you can install CorouinteExceptionHandler. It is guaranteed to be invoked before all joining coroutines can resume.

[elizarov]

Ops. We need to promote CoroutineStart.ATOMIC from internal to experimental.