Introduce deprecated coroutine builder overloads accepting a Job

[dkhalanskyjb]

This is a small step towards deprecating the practice of breaking
structured concurrency by passing a Job to a coroutine builder.

Thanks to @LouisCAD for the idea!

See #3670

[LouisCAD]

Awesome stuff!

[lukellmann]

Why do an explicit join here? supervisorScope should wait for its children, right?

[dkhalanskyjb]

Yes, it will. The idea was to highlight how joinAll will not fail in this scenario, but this piece of documentation may not be a good place for that.

[lukellmann]

Does that mean you can loose the value, even if there is no code after the withContext block?

scope.async(start = ATOMIC) {
    withContext(NonCancellable) {
        dontWantToLooseThisResource()
    }
    // no code here, or only code without suspension points
}

The documentation for withContext says there will be no cancellation on entry/exit with NonCancellable.

[dkhalanskyjb]

My comment is misleading, but yes, the value can be lost, just not because of withContext. The problem is that if the Deferred is cancelled before returning a value, it will no longer be able to finish with a value, even if the computation succeeds.

[lukellmann]

Ok, that's a bit surprising to me. I guess it's because it could have children that haven't completed yet?

[dkhalanskyjb]

Nope, even without any children: https://pl.kotl.in/-ZYf_PR9z
The reason can be seen in the diagram in https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-job/ : once a Job gets cancelled, there is no way back to being Active or Completing or returning a value.

[lukellmann]

Makes sense, good to know 👍

[lukellmann]

Should it be pointed out that just calling cancel on deferred won't prevent the current coroutine from completing before the now cancelled coroutine in the other scope completes?

[LouisCAD]

With these changes, someScope.launch(NonCancellable) { ... } will no longer compile. I've yet to see a tangible use case for that, where the someScope is useful (for anything but Job, that'd be), but wanted to point it out.

Another construct could be thought out, or an overload taking the NonCancellable type could be introduced, if that actually needs to be addressed.

[dkhalanskyjb]

@LouisCAD, what is the compilation error? I've just checked, and launch(NonCancellable) compiles for me (with a warning, as intended). I consider launch(NonCancellable) to be incorrect, with launch(start = CoroutineStart.ATOMIC) { withContext(NonCancellable) { /* computation */ } } being a strictly better version of the same.