Skip to content

Introduce Job.parent API #3384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kotlinx-coroutines-core/api/kotlinx-coroutines-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ public abstract interface class kotlinx/coroutines/Job : kotlin/coroutines/Corou
public abstract fun getCancellationException ()Ljava/util/concurrent/CancellationException;
public abstract fun getChildren ()Lkotlin/sequences/Sequence;
public abstract fun getOnJoin ()Lkotlinx/coroutines/selects/SelectClause0;
public abstract fun getParent ()Lkotlinx/coroutines/Job;
public abstract fun invokeOnCompletion (Lkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/DisposableHandle;
public abstract fun invokeOnCompletion (ZZLkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/DisposableHandle;
public abstract fun isActive ()Z
Expand Down Expand Up @@ -443,6 +444,7 @@ public class kotlinx/coroutines/JobSupport : kotlinx/coroutines/ChildJob, kotlin
public final fun getCompletionExceptionOrNull ()Ljava/lang/Throwable;
public final fun getKey ()Lkotlin/coroutines/CoroutineContext$Key;
public final fun getOnJoin ()Lkotlinx/coroutines/selects/SelectClause0;
public fun getParent ()Lkotlinx/coroutines/Job;
protected fun handleJobException (Ljava/lang/Throwable;)Z
protected final fun initParentJob (Lkotlinx/coroutines/Job;)V
public final fun invokeOnCompletion (Lkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/DisposableHandle;
Expand Down Expand Up @@ -485,6 +487,7 @@ public final class kotlinx/coroutines/NonCancellable : kotlin/coroutines/Abstrac
public fun getCancellationException ()Ljava/util/concurrent/CancellationException;
public fun getChildren ()Lkotlin/sequences/Sequence;
public fun getOnJoin ()Lkotlinx/coroutines/selects/SelectClause0;
public fun getParent ()Lkotlinx/coroutines/Job;
public fun invokeOnCompletion (Lkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/DisposableHandle;
public fun invokeOnCompletion (ZZLkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/DisposableHandle;
public fun isActive ()Z
Expand Down
18 changes: 17 additions & 1 deletion kotlinx-coroutines-core/common/src/Job.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import kotlin.jvm.*
* It is completed by calling [CompletableJob.complete].
*
* Conceptually, an execution of a job does not produce a result value. Jobs are launched solely for their
* side-effects. See [Deferred] interface for a job that produces a result.
* side effects. See [Deferred] interface for a job that produces a result.
*
* ### Job states
*
Expand Down Expand Up @@ -117,6 +117,22 @@ public interface Job : CoroutineContext.Element {

// ------------ state query ------------

/**
* Returns a parent of the current job if the parent-child relationship
* is established or `null` if the job has no parent or was successfully completed.
*
* Accesses to this property are not idempotent, the property becomes `null` as soon
* as the job is transitioned to its final state, whether it is cancelled or completed,
* and all job children are completed.
Comment on lines +124 to +126
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Accesses to this property are not idempotent, the property becomes `null` as soon
* as the job is transitioned to its final state, whether it is cancelled or completed,
* and all job children are completed.
* Accesses to this property are not idempotent: the property becomes `null` as soon
* as the job is transitioned to its final statewhether it is cancelled or completed
* and all job children are completed.

Do we allow Unicode in comments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to avoid it, we never know whether Dokka and kotlinlang support it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument cuts both ways: we may treat this as a good chance to find out. I don't care that much though.

*
* For coroutines, its corresponding job completes as soon as the coroutine itself
* and all its children are complete.
*
* @see [Job] state transitions for additional details.
*/
@ExperimentalCoroutinesApi
public val parent: Job?

/**
* Returns `true` when this job is active -- it was already started and has not completed nor was cancelled yet.
* The job that is waiting for its [children] to complete is still considered to be active if it
Expand Down
3 changes: 3 additions & 0 deletions kotlinx-coroutines-core/common/src/JobSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
get() = _parentHandle.value
set(value) { _parentHandle.value = value }

override val parent: Job?
get() = parentHandle?.parent

// ------------ initialization ------------

/**
Expand Down
8 changes: 8 additions & 0 deletions kotlinx-coroutines-core/common/src/NonCancellable.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ public object NonCancellable : AbstractCoroutineContextElement(Job), Job {

private const val message = "NonCancellable can be used only as an argument for 'withContext', direct usages of its API are prohibited"

/**
* Always returns `null`.
* @suppress **This an internal API and should not be used from general code.**
*/
@Deprecated(level = DeprecationLevel.WARNING, message = message)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add ReplaceWith(null) to allow for automatic replacements in bulk? I don't see the downsides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, it rather means that the whole usage of this API is incorrect and it's time to rethink what is written :)

override val parent: Job?
get() = null

/**
* Always returns `true`.
* @suppress **This an internal API and should not be used from general code.**
Expand Down
1 change: 0 additions & 1 deletion kotlinx-coroutines-core/common/src/internal/Scopes.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ internal open class ScopeCoroutine<in T>(
final override fun getStackTraceElement(): StackTraceElement? = null

final override val isScopedCoroutine: Boolean get() = true
internal val parent: Job? get() = parentHandle?.parent

override fun afterCompletion(state: Any?) {
// Resume in a cancellable way by default when resuming from another context
Expand Down
7 changes: 6 additions & 1 deletion kotlinx-coroutines-core/common/test/JobStatesTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class JobStatesTest : TestBase() {
@Test
public fun testNormalCompletion() = runTest {
expect(1)
val parent = coroutineContext.job
val job = launch(start = CoroutineStart.LAZY) {
expect(2)
// launches child
Expand All @@ -28,23 +29,27 @@ class JobStatesTest : TestBase() {
assertFalse(job.isActive)
assertFalse(job.isCompleted)
assertFalse(job.isCancelled)
assertSame(parent, job.parent)
// New -> Active
job.start()
assertTrue(job.isActive)
assertFalse(job.isCompleted)
assertFalse(job.isCancelled)
assertSame(parent, job.parent)
// Active -> Completing
yield() // scheduled & starts child
expect(3)
assertTrue(job.isActive)
assertFalse(job.isCompleted)
assertFalse(job.isCancelled)
assertSame(parent, job.parent)
// Completing -> Completed
yield()
finish(5)
assertFalse(job.isActive)
assertTrue(job.isCompleted)
assertFalse(job.isCancelled)
assertNull(job.parent)
}

@Test
Expand Down Expand Up @@ -159,4 +164,4 @@ class JobStatesTest : TestBase() {
assertTrue(job.isCompleted)
assertTrue(job.isCancelled)
}
}
}
5 changes: 4 additions & 1 deletion kotlinx-coroutines-core/common/test/JobTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class JobTest : TestBase() {
@Test
fun testState() {
val job = Job()
assertNull(job.parent)
assertTrue(job.isActive)
job.cancel()
assertTrue(!job.isActive)
Expand Down Expand Up @@ -210,11 +211,13 @@ class JobTest : TestBase() {

@Test
fun testIncompleteJobState() = runTest {
val parent = coroutineContext.job
val job = launch {
coroutineContext[Job]!!.invokeOnCompletion { }
}

assertSame(parent, job.parent)
job.join()
assertNull(job.parent)
assertTrue(job.isCompleted)
assertFalse(job.isActive)
assertFalse(job.isCancelled)
Expand Down