Skip to content

Commit 0d7323a

Browse files
elizarovelizarov
committed
Ensure that launch handles uncaught exception before another coroutine
that uses `join` on it resumes. The launched coroutine handles uncaught exception in _completing_ state. Fixes Kotlin#208
1 parent 9852211 commit 0d7323a

File tree

6 files changed

+78
-29
lines changed
  • common/kotlinx-coroutines-core-common/src
  • core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental
  • js/kotlinx-coroutines-core-js/src/main/kotlin/kotlinx/coroutines/experimental

6 files changed

+78
-29
lines changed

‎common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/CommonJob.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ internal expect open class JobSupport(active: Boolean) : Job {
8787

8888
internal fun initParentJobInternal(parent: Job?)
8989
internal fun makeCompletingOnce(proposedUpdate: Any?, mode: Int): Boolean
90+
internal open fun hasOnFinishingHandler(update: Any?): Boolean
91+
internal open fun onFinishingInternal(update: Any?)
9092
internal open fun onCompletionInternal(state: Any?, mode: Int)
9193
internal open fun onStartInternal()
9294
internal open fun onCancellationInternal(exceptionally: CompletedExceptionally?)

‎common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/CommonWithContextTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ class CommonWithContextTest : TestBase() {
159159
}
160160
} catch (e: Throwable) {
161161
expect(7)
162-
// make sure IOException, not CancellationException is thrown!
163-
assertTrue(e is TestException)
162+
// make sure TestException, not CancellationException is thrown!
163+
assertTrue(e is TestException, "Caught $e")
164164
}
165165
}
166166
expect(2)

‎core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/Builders.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,10 @@ private open class StandaloneCoroutine(
188188
private val parentContext: CoroutineContext,
189189
active: Boolean
190190
) : AbstractCoroutine<Unit>(parentContext, active) {
191-
override fun onCancellation(cause: Throwable?) {
191+
override fun hasOnFinishingHandler(update: Any?) = update is CompletedExceptionally
192+
override fun onFinishingInternal(update: Any?) {
192193
// note the use of the parent's job context below!
193-
if (cause != null) handleCoroutineException(parentContext, cause)
194+
if (update is CompletedExceptionally) handleCoroutineException(parentContext, update.exception)
194195
}
195196
}
196197

‎core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/Job.kt

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,7 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job,
10121012
private fun tryMakeCancelling(expect: Incomplete, list: NodeList, cause: Throwable?): Boolean {
10131013
val cancelled = Cancelled(this, cause)
10141014
if (!_state.compareAndSet(expect, Finishing(list, cancelled, false))) return false
1015+
onFinishingInternal(cancelled)
10151016
onCancellationInternal(cancelled)
10161017
notifyCancellation(list, cause)
10171018
return true
@@ -1051,21 +1052,33 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job,
10511052
return COMPLETING_ALREADY_COMPLETING
10521053
if (state is Finishing && state.completing)
10531054
return COMPLETING_ALREADY_COMPLETING
1054-
val child: Child = firstChild(state) ?: // or else complete immediately w/o children
1055-
if (updateState(state, proposedUpdate, mode)) return COMPLETING_COMPLETED else return@loopOnState
1056-
// must promote to list to correct operate on child lists
1057-
if (state is JobNode<*>) {
1058-
promoteSingleToNodeList(state)
1059-
return@loopOnState // retry
1060-
}
1055+
val child: Child? = firstChild(state) ?: // or else complete immediately w/o children
1056+
when {
1057+
state !is Finishing && hasOnFinishingHandler(proposedUpdate) -> null // unless it has onFinishing handler
1058+
updateState(state, proposedUpdate, mode) -> return COMPLETING_COMPLETED
1059+
else -> return@loopOnState
1060+
}
1061+
val list = state.list ?: // must promote to list to correctly operate on child lists
1062+
when (state) {
1063+
is Empty -> {
1064+
promoteEmptyToNodeList(state)
1065+
return@loopOnState // retry
1066+
}
1067+
is JobNode<*> -> {
1068+
promoteSingleToNodeList(state)
1069+
return@loopOnState // retry
1070+
}
1071+
else -> error("Unexpected state with an empty list: $state")
1072+
}
10611073
// cancel all children in list on exceptional completion
10621074
if (proposedUpdate is CompletedExceptionally)
1063-
child.cancelChildrenInternal(proposedUpdate.exception)
1075+
child?.cancelChildrenInternal(proposedUpdate.exception)
10641076
// switch to completing state
10651077
val cancelled = (state as? Finishing)?.cancelled ?: (proposedUpdate as? Cancelled)
1066-
val completing = Finishing(state.list!!, cancelled, true)
1078+
val completing = Finishing(list, cancelled, true)
10671079
if (_state.compareAndSet(state, completing)) {
1068-
if (tryWaitForChild(child, proposedUpdate))
1080+
if (state !is Finishing) onFinishingInternal(proposedUpdate)
1081+
if (child != null && tryWaitForChild(child, proposedUpdate))
10691082
return COMPLETING_WAITING_CHILDREN
10701083
if (updateState(completing, proposedUpdate, mode = MODE_ATOMIC_DEFAULT))
10711084
return COMPLETING_COMPLETED
@@ -1157,6 +1170,16 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job,
11571170
*/
11581171
internal actual open fun onCancellationInternal(exceptionally: CompletedExceptionally?) {}
11591172

1173+
/**
1174+
* @suppress **This is unstable API and it is subject to change.**
1175+
*/
1176+
internal actual open fun hasOnFinishingHandler(update: Any?) = false
1177+
1178+
/**
1179+
* @suppress **This is unstable API and it is subject to change.**
1180+
*/
1181+
internal actual open fun onFinishingInternal(update: Any?) {}
1182+
11601183
/**
11611184
* Override for post-completion actions that need to do something with the state.
11621185
* @param mode completion mode.

‎js/kotlinx-coroutines-core-js/src/main/kotlin/kotlinx/coroutines/experimental/Builders.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ private open class StandaloneCoroutine(
116116
private val parentContext: CoroutineContext,
117117
active: Boolean
118118
) : AbstractCoroutine<Unit>(parentContext, active) {
119-
override fun onCancellation(cause: Throwable?) {
119+
override fun hasOnFinishingHandler(update: Any?) = update is CompletedExceptionally
120+
override fun onFinishingInternal(update: Any?) {
120121
// note the use of the parent's job context below!
121-
if (cause != null) handleCoroutineException(parentContext, cause)
122+
if (update is CompletedExceptionally) handleCoroutineException(parentContext, update.exception)
122123
}
123124
}
124125

‎js/kotlinx-coroutines-core-js/src/main/kotlin/kotlinx/coroutines/experimental/Job.kt

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,7 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job
724724
private fun makeCancellingList(list: NodeList, cause: Throwable?) {
725725
val cancelled = Cancelled(this, cause)
726726
state = Finishing(list, cancelled, false)
727+
onFinishingInternal(cancelled)
727728
onCancellationInternal(cancelled)
728729
notifyCancellation(list, cause)
729730
}
@@ -761,23 +762,34 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job
761762
return COMPLETING_ALREADY_COMPLETING
762763
if (state is Finishing && state.completing)
763764
return COMPLETING_ALREADY_COMPLETING
764-
val child: Child = firstChild(state) ?: run {
765-
// or else complete immediately w/o children
766-
updateState(proposedUpdate, mode)
767-
return COMPLETING_COMPLETED
768-
}
769-
// must promote to list to correct operate on child lists
770-
if (state is JobNode<*>) {
771-
promoteSingleToNodeList(state)
772-
continue@loop // retry
773-
}
765+
val child: Child? = firstChild(state) ?: // or else complete immediately w/o children
766+
when {
767+
state !is Finishing && hasOnFinishingHandler(proposedUpdate) -> null // unless it has onCompleting handler
768+
else -> {
769+
updateState(proposedUpdate, mode)
770+
return COMPLETING_COMPLETED
771+
}
772+
}
773+
val list = state.list ?: // must promote to list to correctly operate on child lists
774+
when (state) {
775+
is Empty -> {
776+
promoteEmptyToNodeList(state)
777+
continue@loop // retry
778+
}
779+
is JobNode<*> -> {
780+
promoteSingleToNodeList(state)
781+
continue@loop // retry
782+
}
783+
else -> error("Unexpected state with an empty list: $state")
784+
}
774785
// cancel all children in list on exceptional completion
775786
if (proposedUpdate is CompletedExceptionally)
776-
child.cancelChildrenInternal(proposedUpdate.exception)
787+
child?.cancelChildrenInternal(proposedUpdate.exception)
777788
// switch to completing state
778-
val completing = Finishing(state.list!!, (state as? Finishing)?.cancelled, true)
789+
val completing = Finishing(list, (state as? Finishing)?.cancelled, true)
779790
this.state = completing
780-
if (tryWaitForChild(child, proposedUpdate))
791+
if (state !is Finishing) onFinishingInternal(proposedUpdate)
792+
if (child != null && tryWaitForChild(child, proposedUpdate))
781793
return COMPLETING_WAITING_CHILDREN
782794
updateState(proposedUpdate, mode = MODE_ATOMIC_DEFAULT)
783795
return COMPLETING_COMPLETED
@@ -860,6 +872,16 @@ internal actual open class JobSupport actual constructor(active: Boolean) : Job
860872
*/
861873
internal actual open fun onCancellationInternal(exceptionally: CompletedExceptionally?) {}
862874

875+
/**
876+
* @suppress **This is unstable API and it is subject to change.**
877+
*/
878+
internal actual open fun hasOnFinishingHandler(update: Any?) = false
879+
880+
/**
881+
* @suppress **This is unstable API and it is subject to change.**
882+
*/
883+
internal actual open fun onFinishingInternal(update: Any?) {}
884+
863885
/**
864886
* Override for completion actions that need to do something with the state.
865887
* @param mode completion mode.

0 commit comments

Comments
 (0)