JobSupport stylistic improvements:

Add notion of job state finalization
Provide separate package for exception tests

Author: qwwdfsad

common/kotlinx-coroutines-core-common/src/CompletedExceptionally.kt

@@ -27,7 +27,6 @@ open class CompletedExceptionally(
  * A specific subclass of [CompletedExceptionally] for cancelled jobs.
  *
  * **Note: This class cannot be used outside of internal coroutines framework**.
- * TODO rename to CancelledJob?
  *
  * @param job the job that was cancelled.
  * @param cause the exceptional completion cause. If `cause` is null, then a [JobCancellationException] is created.

common/kotlinx-coroutines-core-common/src/JobSupport.kt

@@ -104,7 +104,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
         @Suppress("DEPRECATION")
         val handle = parent.attachChild(this)
         parentHandle = handle
-        // now check our state _after_ registering (see updateState order of actions)
+        // now check our state _after_ registering (see tryFinalizeState order of actions)
         if (isCompleted) {
             handle.dispose()
             parentHandle = NonDisposableHandle // release it just in case, to aid GC
@@ -148,62 +148,39 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
     // ------------ state update ------------
 
     /**
-     * Updates current [state] of this job. Returns `false` if current state is not equal to expected.
-     * @suppress **This is unstable API and it is subject to change.**
+     * Updates current [state] of this job to the final state, invoking all necessary handlers
+     * and/or `on*` methods.
+     *
+     * Returns `false` if current state is not equal to expected.
+     * If this method succeeds, state of this job will never be changed again
      */
-    private fun updateState(expect: Incomplete, proposedUpdate: Any?, mode: Int): Boolean {
+    private fun tryFinalizeState(expect: Incomplete, proposedUpdate: Any?, mode: Int): Boolean {
         val update = coerceProposedUpdate(expect, proposedUpdate)
-        if (!tryUpdateState(expect, update)) return false
-        completeUpdateState(expect, update, mode)
+        if (!tryFinalizeState(expect, update)) return false
+        completeStateFinalization(expect, update, mode)
         return true
     }
 
-    // when Job is in Cancelling state, it can only be promoted to Cancelled state,
-    // so if the proposed Update is not an appropriate Cancelled (preserving the cancellation cause),
-    // then the corresponding Cancelled state is constructed.
-    private fun coerceProposedUpdate(expect: Incomplete, proposedUpdate: Any?): Any? =
-        if (expect is Finishing && expect.cancelled != null && !isCorrespondinglyCancelled(expect.cancelled, proposedUpdate))
-            createCancelled(expect.cancelled, proposedUpdate) else proposedUpdate
-
-    private fun isCorrespondinglyCancelled(cancelled: Cancelled, proposedUpdate: Any?): Boolean {
-        if (proposedUpdate !is Cancelled) return false
-        // NOTE: equality comparison of causes is performed here by design, see equals of JobCancellationException
-        return proposedUpdate.cause == cancelled.cause || proposedUpdate.cause is JobCancellationException
-    }
-
-    private fun createCancelled(cancelled: Cancelled, proposedUpdate: Any?): Cancelled {
-        if (proposedUpdate !is CompletedExceptionally) return cancelled // not exception -- just use original cancelled
-        val exception = proposedUpdate.cause
-        if (cancelled.cause == exception) return cancelled // that is the cancelled we need already!
-        // todo: We need to rework this logic to keep original cancellation cause in the state and suppress other exceptions
-        //       that could have occurred while coroutine is being cancelled.
-        // Do not spam with JCE in suppressed exceptions
-        if (cancelled.cause !is JobCancellationException) {
-            exception.addSuppressedThrowable(cancelled.cause)
-        }
-        return Cancelled(this, exception)
-    }
-
     /**
-     * Tries to initiate update of the current [state] of this job.
-     * @suppress **This is unstable API and it is subject to change.**
+     * Tries to update [_state] of this job to the final state and, if
+     * succeeds, disposes parent handle (de-attaching child from parent)
      */
-    private fun tryUpdateState(expect: Incomplete, update: Any?): Boolean  {
+    private fun tryFinalizeState(expect: Incomplete, update: Any?): Boolean  {
         require(update !is Incomplete) // only incomplete -> completed transition is allowed
         if (!_state.compareAndSet(expect, update)) return false
         // Unregister from parent job
         parentHandle?.let {
             it.dispose() // volatile read parentHandle _after_ state was updated
             parentHandle = NonDisposableHandle // release it just in case, to aid GC
         }
-        return true // continues in completeUpdateState
+        return true // continues in completeStateFinalization
     }
 
     /**
      * Completes update of the current [state] of this job.
      * @suppress **This is unstable API and it is subject to change.**
      */
-    private fun completeUpdateState(expect: Incomplete, update: Any?, mode: Int) {
+    private fun completeStateFinalization(expect: Incomplete, update: Any?, mode: Int) {
         val exceptionally = update as? CompletedExceptionally
         // Do overridable processing before completion handlers
 
@@ -228,11 +205,42 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
         } else {
             expect.list?.notifyCompletion(cause)
         }
-
         onCompletionInternal(update, mode)
     }
 
-    private inline fun <reified T: JobNode<*>> notifyHandlers(list: NodeList, cause: Throwable?) {
+    // when Job is in Cancelling state, it can only be promoted to Cancelled state,
+    // so if the proposed Update is not an appropriate Cancelled (preserving the cancellation cause),
+    // then the corresponding Cancelled state is constructed.
+    private fun coerceProposedUpdate(expect: Incomplete, proposedUpdate: Any?): Any? =
+        if (expect is Finishing && expect.cancelled != null && !isCorrespondinglyCancelled(expect.cancelled, proposedUpdate))
+            createCancelled(expect.cancelled, proposedUpdate) else proposedUpdate
+
+    private fun isCorrespondinglyCancelled(cancelled: Cancelled, proposedUpdate: Any?): Boolean {
+        if (proposedUpdate !is Cancelled) return false
+        // NOTE: equality comparison of causes is performed here by design, see equals of JobCancellationException
+        return proposedUpdate.cause == cancelled.cause || proposedUpdate.cause is JobCancellationException
+    }
+
+    private fun createCancelled(cancelled: Cancelled, proposedUpdate: Any?): Cancelled {
+        if (proposedUpdate !is CompletedExceptionally) return cancelled // not exception -- just use original cancelled
+        val exception = proposedUpdate.cause
+        if (cancelled.cause == exception) return cancelled // that is the cancelled we need already!
+        // todo: We need to rework this logic to keep original cancellation cause in the state and suppress other exceptions
+        //       that could have occurred while coroutine is being cancelled.
+        // Do not spam with JCE in suppressed exceptions
+        if (cancelled.cause !is JobCancellationException) {
+            exception.addSuppressedThrowable(cancelled.cause)
+        }
+        return Cancelled(this, exception)
+    }
+
+    private fun NodeList.notifyCompletion(cause: Throwable?) =
+        notifyHandlers<JobNode<*>>(this, cause)
+
+    private fun notifyCancellation(list: NodeList, cause: Throwable?) =
+        notifyHandlers<JobCancellationNode<*>>(list, cause)
+
+      private inline fun <reified T: JobNode<*>> notifyHandlers(list: NodeList, cause: Throwable?) {
         var exception: Throwable? = null
         list.forEach<T> { node ->
             try {
@@ -246,12 +254,6 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
         exception?.let { handleException(it) }
     }
 
-    private fun NodeList.notifyCompletion(cause: Throwable?) =
-        notifyHandlers<JobNode<*>>(this, cause)
-
-    private fun notifyCancellation(list: NodeList, cause: Throwable?) =
-        notifyHandlers<JobCancellationNode<*>>(list, cause)
-
     public final override fun start(): Boolean {
         loopOnState { state ->
             when (startInternal(state)) {
@@ -472,14 +474,14 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
 
     public override fun cancel(cause: Throwable?): Boolean = when (onCancelMode) {
         ON_CANCEL_MAKE_CANCELLING -> makeCancelling(cause)
-        ON_CANCEL_MAKE_COMPLETING -> makeCompletingOnCancel(cause)
+        ON_CANCEL_MAKE_COMPLETING -> makeCompleting(Cancelled(this, cause))
         else -> error("Invalid onCancelMode $onCancelMode")
     }
 
     // we will be dispatching coroutine to process its cancellation exception, so there is no need for
     // an extra check for Job status in MODE_CANCELLABLE
     private fun updateStateCancelled(state: Incomplete, cause: Throwable?) =
-        updateState(state, Cancelled(this, cause), mode = MODE_ATOMIC_DEFAULT)
+        tryFinalizeState(state, Cancelled(this, cause), mode = MODE_ATOMIC_DEFAULT)
 
     // transitions to Cancelling state
     private fun makeCancelling(cause: Throwable?): Boolean {
@@ -528,9 +530,6 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
         return true
     }
 
-    private fun makeCompletingOnCancel(cause: Throwable?): Boolean =
-        makeCompleting(Cancelled(this, cause))
-
     /**
      * @suppress **This is unstable API and it is subject to change.**
      */
@@ -565,7 +564,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
             val child: ChildJob? = firstChild(state) ?: // or else complete immediately w/o children
             when {
                 state !is Finishing && hasOnFinishingHandler(proposedUpdate) -> null // unless it has onFinishing handler
-                updateState(state, proposedUpdate, mode) -> return COMPLETING_COMPLETED
+                tryFinalizeState(state, proposedUpdate, mode) -> return COMPLETING_COMPLETED
                 else -> return@loopOnState
             }
             val list = state.list ?: // must promote to list to correctly operate on child lists
@@ -590,7 +589,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
                 if (state !is Finishing) onFinishingInternal(proposedUpdate)
                 if (child != null && tryWaitForChild(child, proposedUpdate))
                     return COMPLETING_WAITING_CHILDREN
-                if (updateState(completing, proposedUpdate, mode = MODE_ATOMIC_DEFAULT))
+                if (tryFinalizeState(completing, proposedUpdate, mode = MODE_ATOMIC_DEFAULT))
                     return COMPLETING_COMPLETED
             }
         }
@@ -628,7 +627,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
             // try wait for next child
             if (waitChild != null && tryWaitForChild(waitChild, proposedUpdate)) return // waiting for next child
             // no more children to wait -- try update state
-            if (updateState(state, proposedUpdate, MODE_ATOMIC_DEFAULT)) return
+            if (tryFinalizeState(state, proposedUpdate, MODE_ATOMIC_DEFAULT)) return
         }
     }