Skip to content

Commit a5b6a33

Browse files
committed
Improve unboxing mechanics
1 parent ecfd227 commit a5b6a33

File tree

5 files changed

+97
-32
lines changed

5 files changed

+97
-32
lines changed

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

+16-13
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
10341034
private val job: JobSupport
10351035
) : CancellableContinuationImpl<T>(delegate, MODE_CANCELLABLE) {
10361036
override fun getContinuationCancellationCause(parent: Job): Throwable {
1037-
val state = job.state.unboxState()
1037+
val state = job.state
10381038
/*
10391039
* When the job we are waiting for had already completely completed exceptionally or
10401040
* is failing, we shall use its root/completion cause for await's result.
@@ -1059,7 +1059,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
10591059
public val isCompletedExceptionally: Boolean get() = state is CompletedExceptionally
10601060

10611061
public fun getCompletionExceptionOrNull(): Throwable? {
1062-
val state = this.state.unboxState()
1062+
val state = this.state
10631063
check(state !is Incomplete) { "This job has not completed yet" }
10641064
return state.exceptionOrNull
10651065
}
@@ -1068,10 +1068,10 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
10681068
* @suppress **This is unstable API and it is subject to change.**
10691069
*/
10701070
internal fun getCompletedInternal(): Any? {
1071-
val state = this.state.unboxState()
1071+
val state = this.state
10721072
check(state !is Incomplete) { "This job has not completed yet" }
10731073
if (state is CompletedExceptionally) throw state.cause
1074-
return state
1074+
return state.unboxState()
10751075
}
10761076

10771077
/**
@@ -1080,11 +1080,11 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
10801080
internal suspend fun awaitInternal(): Any? {
10811081
// fast-path -- check state (avoid extra object creation)
10821082
while (true) { // lock-free loop on state
1083-
val state = this.state.unboxState()
1083+
val state = this.state
10841084
if (state !is Incomplete) {
10851085
// already complete -- just return result
10861086
if (state is CompletedExceptionally) throw state.cause
1087-
return state
1087+
return state.unboxState()
10881088

10891089
}
10901090
if (startInternal(state) >= 0) break // break unless needs to retry
@@ -1116,10 +1116,12 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
11161116
if (state !is Incomplete) {
11171117
// already complete -- select result
11181118
if (select.trySelect(null)) {
1119-
if (state is CompletedExceptionally)
1119+
if (state is CompletedExceptionally) {
11201120
select.resumeSelectCancellableWithException(state.cause)
1121-
else
1122-
block.startCoroutineUnintercepted(state as T, select.completion)
1121+
}
1122+
else {
1123+
block.startCoroutineUnintercepted(state.unboxState() as T, select.completion)
1124+
}
11231125
}
11241126
return
11251127
}
@@ -1136,12 +1138,12 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
11361138
*/
11371139
@Suppress("UNCHECKED_CAST")
11381140
internal fun <T, R> selectAwaitCompletion(select: SelectInstance<R>, block: suspend (T) -> R) {
1139-
val state = this.state.unboxState()
1141+
val state = this.state
11401142
// Note: await is non-atomic (can be cancelled while dispatched)
11411143
if (state is CompletedExceptionally)
11421144
select.resumeSelectCancellableWithException(state.cause)
11431145
else
1144-
block.startCoroutineCancellable(state as T, select.completion)
1146+
block.startCoroutineCancellable(state.unboxState() as T, select.completion)
11451147
}
11461148
}
11471149

@@ -1244,14 +1246,15 @@ private class ResumeAwaitOnCompletion<T>(
12441246
private val continuation: AbstractContinuation<T>
12451247
) : JobNode<JobSupport>(job) {
12461248
override fun invoke(cause: Throwable?) {
1247-
val state = job.state.unboxState()
1249+
val state = job.state
1250+
check(state !is Incomplete)
12481251
if (state is CompletedExceptionally) {
12491252
// Resume with exception in atomic way to preserve exception
12501253
continuation.resumeWithExceptionMode(state.cause, MODE_ATOMIC_DEFAULT)
12511254
} else {
12521255
// Resuming with value in a cancellable way (AwaitContinuation is configured for this mode).
12531256
@Suppress("UNCHECKED_CAST")
1254-
continuation.resume(state as T)
1257+
continuation.resume(state.unboxState() as T)
12551258
}
12561259
}
12571260
override fun toString() = "ResumeAwaitOnCompletion[$continuation]"

common/kotlinx-coroutines-core-common/src/intrinsics/Undispatched.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,15 @@ private inline fun <T> AbstractCoroutine<T>.undispatchedResult(
123123
return when {
124124
result === COROUTINE_SUSPENDED -> COROUTINE_SUSPENDED
125125
makeCompletingOnce(result, MODE_IGNORE) -> {
126-
val state = state.unboxState()
126+
val state = state
127127
if (state is CompletedExceptionally) {
128128
when {
129129
shouldThrow(state.cause) -> throw state.cause
130130
result is CompletedExceptionally -> throw result.cause
131131
else -> result
132132
}
133133
} else {
134-
state
134+
state.unboxState()
135135
}
136136
}
137137
else -> COROUTINE_SUSPENDED

common/kotlinx-coroutines-core-common/test/AsyncTest.kt

+23-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
33
*/
44

5-
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED", "UNREACHABLE_CODE") // KT-21913
5+
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED", "UNREACHABLE_CODE", "USELESS_IS_CHECK") // KT-21913
66

77
package kotlinx.coroutines
88

@@ -242,13 +242,30 @@ class AsyncTest : TestBase() {
242242

243243
@Test
244244
fun testIncompleteAsyncState() = runTest {
245-
val job = async {
245+
val deferred = async {
246246
coroutineContext[Job]!!.invokeOnCompletion { }
247247
}
248248

249-
job.await().dispose()
250-
assertTrue(job.isCompleted)
251-
assertFalse(job.isActive)
252-
assertFalse(job.isCancelled)
249+
deferred.await().dispose()
250+
assertTrue(deferred.getCompleted() is DisposableHandle)
251+
assertNull(deferred.getCompletionExceptionOrNull())
252+
assertTrue(deferred.isCompleted)
253+
assertFalse(deferred.isActive)
254+
assertFalse(deferred.isCancelled)
253255
}
256+
257+
@Test
258+
fun testIncompleteAsyncFastPath() = runTest {
259+
val deferred = async(Dispatchers.Unconfined) {
260+
coroutineContext[Job]!!.invokeOnCompletion { }
261+
}
262+
263+
deferred.await().dispose()
264+
assertTrue(deferred.getCompleted() is DisposableHandle)
265+
assertNull(deferred.getCompletionExceptionOrNull())
266+
assertTrue(deferred.isCompleted)
267+
assertFalse(deferred.isActive)
268+
assertFalse(deferred.isCancelled)
269+
}
270+
254271
}

common/kotlinx-coroutines-core-common/test/CompletableDeferredTest.kt

+22-11
Original file line numberDiff line numberDiff line change
@@ -15,30 +15,41 @@ class CompletableDeferredTest : TestBase() {
1515
checkFresh(c)
1616
}
1717

18-
private fun checkFresh(c: CompletableDeferred<String>) {
19-
assertEquals(true, c.isActive)
20-
assertEquals(false, c.isCancelled)
21-
assertEquals(false, c.isCompleted)
22-
assertThrows<IllegalStateException> { c.getCancellationException() }
23-
assertThrows<IllegalStateException> { c.getCompleted() }
24-
assertThrows<IllegalStateException> { c.getCompletionExceptionOrNull() }
25-
}
26-
2718
@Test
2819
fun testComplete() {
2920
val c = CompletableDeferred<String>()
3021
assertEquals(true, c.complete("OK"))
3122
checkCompleteOk(c)
23+
assertEquals("OK", c.getCompleted())
3224
assertEquals(false, c.complete("OK"))
3325
checkCompleteOk(c)
26+
assertEquals("OK", c.getCompleted())
27+
}
28+
29+
@Test
30+
fun testCompleteWithIncompleteResult() {
31+
val c = CompletableDeferred<DisposableHandle>()
32+
assertEquals(true, c.complete(c.invokeOnCompletion { }))
33+
checkCompleteOk(c)
34+
assertEquals(false, c.complete(c.invokeOnCompletion { }))
35+
checkCompleteOk(c)
36+
assertTrue(c.getCompleted() is Incomplete)
37+
}
38+
39+
private fun checkFresh(c: CompletableDeferred<*>) {
40+
assertEquals(true, c.isActive)
41+
assertEquals(false, c.isCancelled)
42+
assertEquals(false, c.isCompleted)
43+
assertThrows<IllegalStateException> { c.getCancellationException() }
44+
assertThrows<IllegalStateException> { c.getCompleted() }
45+
assertThrows<IllegalStateException> { c.getCompletionExceptionOrNull() }
3446
}
3547

36-
private fun checkCompleteOk(c: CompletableDeferred<String>) {
48+
private fun checkCompleteOk(c: CompletableDeferred<*>) {
3749
assertEquals(false, c.isActive)
3850
assertEquals(false, c.isCancelled)
3951
assertEquals(true, c.isCompleted)
4052
assertTrue(c.getCancellationException() is JobCancellationException)
41-
assertEquals("OK", c.getCompleted())
4253
assertEquals(null, c.getCompletionExceptionOrNull())
4354
}
4455

common/kotlinx-coroutines-core-common/test/selects/SelectDeferredTest.kt

+34
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,38 @@ class SelectDeferredTest : TestBase() {
136136
expectUnreached()
137137
}
138138

139+
@Test
140+
fun testSelectIncomplete() = runTest {
141+
val deferred = async { Wrapper("OK") }
142+
val result = select<Wrapper> {
143+
assertFalse(deferred.isCompleted)
144+
assertTrue(deferred.isActive)
145+
deferred.onAwait {
146+
it
147+
}
148+
}
149+
150+
assertEquals("OK", result.value)
151+
}
152+
153+
@Test
154+
fun testSelectIncompleteFastPath() = runTest {
155+
val deferred = async(Dispatchers.Unconfined) { Wrapper("OK") }
156+
val result = select<Wrapper> {
157+
assertTrue(deferred.isCompleted)
158+
assertFalse(deferred.isActive)
159+
deferred.onAwait {
160+
it
161+
}
162+
}
163+
164+
assertEquals("OK", result.value)
165+
}
166+
167+
private class Wrapper(val value: String) : Incomplete {
168+
override val isActive: Boolean
169+
get() = error("")
170+
override val list: NodeList?
171+
get() = error("")
172+
}
139173
}

0 commit comments

Comments
 (0)