Skip to content

Commit

Permalink
Rework initParentJob
Browse files Browse the repository at this point in the history
    * Always establish parent-child relationship when creating a subclass of AbstractCoroutine. That's our own internal class that we have full control of and it never has a chance to leak to the user-code (so cancellation handlers will be installed etc.)
    * As a consequence, get rid of parentContext in all our coroutine classes that are not ScopeCoroutine
    * Remove some dead code

The whole late-binding of initParentJob seems the be the legacy of days when AbstractCoroutine was public and CancellableContinuation implemented a Job
  • Loading branch information
qwwdfsad committed Mar 22, 2021
1 parent 4054910 commit d7a56eb
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 49 deletions.
2 changes: 0 additions & 2 deletions kotlinx-coroutines-core/api/kotlinx-coroutines-core.api
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
public abstract class kotlinx/coroutines/AbstractCoroutine : kotlinx/coroutines/JobSupport, kotlin/coroutines/Continuation, kotlinx/coroutines/CoroutineScope, kotlinx/coroutines/Job {
protected final field parentContext Lkotlin/coroutines/CoroutineContext;
public fun <init> (Lkotlin/coroutines/CoroutineContext;Z)V
public synthetic fun <init> (Lkotlin/coroutines/CoroutineContext;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
protected fun afterResume (Ljava/lang/Object;)V
Expand All @@ -12,7 +11,6 @@ public abstract class kotlinx/coroutines/AbstractCoroutine : kotlinx/coroutines/
protected final fun onCompletionInternal (Ljava/lang/Object;)V
public final fun resumeWith (Ljava/lang/Object;)V
public final fun start (Lkotlinx/coroutines/CoroutineStart;Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)V
public final fun start (Lkotlinx/coroutines/CoroutineStart;Lkotlin/jvm/functions/Function1;)V
}

public final class kotlinx/coroutines/AwaitKt {
Expand Down
45 changes: 9 additions & 36 deletions kotlinx-coroutines-core/common/src/AbstractCoroutine.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package kotlinx.coroutines
import kotlinx.coroutines.CoroutineStart.*
import kotlinx.coroutines.intrinsics.*
import kotlin.coroutines.*
import kotlin.jvm.*

/**
* Abstract base class for implementation of coroutines in coroutine builders.
Expand Down Expand Up @@ -36,8 +35,7 @@ public abstract class AbstractCoroutine<in T>(
/**
* The context of the parent coroutine.
*/
@JvmField
protected val parentContext: CoroutineContext,
parentContext: CoroutineContext,
active: Boolean = true
) : JobSupport(active), Job, Continuation<T>, CoroutineScope {
/**
Expand All @@ -46,25 +44,21 @@ public abstract class AbstractCoroutine<in T>(
@Suppress("LeakingThis")
public final override val context: CoroutineContext = parentContext + this

init {
/*
* Setup parent-child relationship between the parent in the context
* and the current coroutine.
* It may cause this coroutine to become cancelled if the parent is already cancelled
*/
initParentJobInternal(parentContext[Job])
}
/**
* The context of this scope which is the same as the [context] of this coroutine.
*/
public override val coroutineContext: CoroutineContext get() = context

override val isActive: Boolean get() = super.isActive

/**
* Initializes the parent job from the `parentContext` of this coroutine that was passed to it during construction.
* It shall be invoked at most once after construction after all other initialization.
*
* Invocation of this function may cause this coroutine to become cancelled if the parent is already cancelled,
* in which case it synchronously invokes all the corresponding handlers.
* @suppress **This is unstable API and it is subject to change.**
*/
internal fun initParentJob() {
initParentJobInternal(parentContext[Job])
}

/**
* This function is invoked once when the job was completed normally with the specified [value],
* right before all the waiters for the coroutine's completion are notified.
Expand Down Expand Up @@ -117,34 +111,13 @@ public abstract class AbstractCoroutine<in T>(
/**
* Starts this coroutine with the given code [block] and [start] strategy.
* This function shall be invoked at most once on this coroutine.
*
* First, this function initializes parent job from the `parentContext` of this coroutine that was passed to it
* during construction. Second, it starts the coroutine based on [start] parameter:
*
* * [DEFAULT] uses [startCoroutineCancellable].
* * [ATOMIC] uses [startCoroutine].
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
* * [LAZY] does nothing.
*/
public fun start(start: CoroutineStart, block: suspend () -> T) {
initParentJob()
start(block, this)
}

/**
* Starts this coroutine with the given code [block] and [start] strategy.
* This function shall be invoked at most once on this coroutine.
*
* First, this function initializes parent job from the `parentContext` of this coroutine that was passed to it
* during construction. Second, it starts the coroutine based on [start] parameter:
*
* * [DEFAULT] uses [startCoroutineCancellable].
* * [ATOMIC] uses [startCoroutine].
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
* * [LAZY] does nothing.
*/
public fun <R> start(start: CoroutineStart, receiver: R, block: suspend R.() -> T) {
initParentJob()
start(block, receiver, this)
}
}
1 change: 0 additions & 1 deletion kotlinx-coroutines-core/common/src/Builders.common.kt
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ public suspend fun <T> withContext(
}
// SLOW PATH -- use new dispatcher
val coroutine = DispatchedCoroutine(newContext, uCont)
coroutine.initParentJob()
block.startCoroutineCancellable(coroutine, coroutine)
coroutine.getResult()
}
Expand Down
5 changes: 3 additions & 2 deletions kotlinx-coroutines-core/common/src/internal/Scopes.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ internal open class ScopeCoroutine<in T>(
context: CoroutineContext,
@JvmField val uCont: Continuation<T> // unintercepted continuation
) : AbstractCoroutine<T>(context, true), CoroutineStackFrame {

final override val callerFrame: CoroutineStackFrame? get() = uCont as? CoroutineStackFrame
final override fun getStackTraceElement(): StackTraceElement? = null
final override val isScopedCoroutine: Boolean get() = true

internal val parent: Job? get() = parentContext[Job]
final override val isScopedCoroutine: Boolean get() = true
internal val parent: Job? = context[Job]

override fun afterCompletion(state: Any?) {
// Resume in a cancellable way by default when resuming from another context
Expand Down
8 changes: 3 additions & 5 deletions kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,9 @@ private inline fun <T> startDirect(completion: Continuation<T>, block: (Continua
* This function shall be invoked at most once on this coroutine.
* This function checks cancellation of the outer [Job] on fast-path.
*
* First, this function initializes the parent job from the `parentContext` of this coroutine that was passed to it
* during construction. Second, it starts the coroutine using [startCoroutineUninterceptedOrReturn].
* It starts the coroutine using [startCoroutineUninterceptedOrReturn].
*/
internal fun <T, R> ScopeCoroutine<T>.startUndispatchedOrReturn(receiver: R, block: suspend R.() -> T): Any? {
initParentJob()
return undispatchedResult({ true }) {
block.startCoroutineUninterceptedOrReturn(receiver, this)
}
Expand All @@ -96,8 +94,8 @@ internal fun <T, R> ScopeCoroutine<T>.startUndispatchedOrReturn(receiver: R, blo
* Same as [startUndispatchedOrReturn], but ignores [TimeoutCancellationException] on fast-path.
*/
internal fun <T, R> ScopeCoroutine<T>.startUndispatchedOrReturnIgnoreTimeout(
receiver: R, block: suspend R.() -> T): Any? {
initParentJob()
receiver: R, block: suspend R.() -> T
): Any? {
return undispatchedResult({ e -> !(e is TimeoutCancellationException && e.coroutine === this) }) {
block.startCoroutineUninterceptedOrReturn(receiver, this)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
* Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.coroutines.exceptions
Expand All @@ -15,7 +15,7 @@ class SuppressionTests : TestBase() {
@Test
fun testNotificationsWithException() = runTest {
expect(1)
val coroutineContext = kotlin.coroutines.coroutineContext // workaround for KT-22984
val coroutineContext = kotlin.coroutines.coroutineContext + NonCancellable // workaround for KT-22984
val coroutine = object : AbstractCoroutine<String>(coroutineContext, false) {
override fun onStart() {
expect(3)
Expand Down Expand Up @@ -82,4 +82,4 @@ class SuppressionTests : TestBase() {
assertTrue(e.cause!!.suppressed.isEmpty())
}
}
}
}

0 comments on commit d7a56eb

Please sign in to comment.