Skip to content

Commit 8b6473d

Browse files
Comply with Subscriber rule 2.7 in the await* impl (#3360)
There is a possibility of a race between Subscription.request and Subscription.cancel methods since cancellation handler could be executed in a separate thread. Rule [2.7](https://github.com/reactive-streams/reactive-streams-jvm/blob/v1.0.3/README.md#2-subscriber-code) requires Subscription methods to be executed serially.
1 parent 10261a7 commit 8b6473d

File tree

2 files changed

+68
-5
lines changed

2 files changed

+68
-5
lines changed

reactive/kotlinx-coroutines-reactive/src/Await.kt

+25-5
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,20 @@ private suspend fun <T> Publisher<T>.awaitOne(
198198
/** cancelling the new subscription due to rule 2.5, though the publisher would either have to
199199
* subscribe more than once, which would break 2.12, or leak this [Subscriber]. */
200200
if (subscription != null) {
201-
sub.cancel()
201+
withSubscriptionLock {
202+
sub.cancel()
203+
}
202204
return
203205
}
204206
subscription = sub
205-
cont.invokeOnCancellation { sub.cancel() }
206-
sub.request(if (mode == Mode.FIRST || mode == Mode.FIRST_OR_DEFAULT) 1 else Long.MAX_VALUE)
207+
cont.invokeOnCancellation {
208+
withSubscriptionLock {
209+
sub.cancel()
210+
}
211+
}
212+
withSubscriptionLock {
213+
sub.request(if (mode == Mode.FIRST || mode == Mode.FIRST_OR_DEFAULT) 1 else Long.MAX_VALUE)
214+
}
207215
}
208216

209217
override fun onNext(t: T) {
@@ -228,12 +236,16 @@ private suspend fun <T> Publisher<T>.awaitOne(
228236
return
229237
}
230238
seenValue = true
231-
sub.cancel()
239+
withSubscriptionLock {
240+
sub.cancel()
241+
}
232242
cont.resume(t)
233243
}
234244
Mode.LAST, Mode.SINGLE, Mode.SINGLE_OR_DEFAULT -> {
235245
if ((mode == Mode.SINGLE || mode == Mode.SINGLE_OR_DEFAULT) && seenValue) {
236-
sub.cancel()
246+
withSubscriptionLock {
247+
sub.cancel()
248+
}
237249
/* the check for `cont.isActive` is needed in case `sub.cancel() above calls `onComplete` or
238250
`onError` on its own. */
239251
if (cont.isActive) {
@@ -289,6 +301,14 @@ private suspend fun <T> Publisher<T>.awaitOne(
289301
inTerminalState = true
290302
return true
291303
}
304+
305+
/**
306+
* Enforce rule 2.7: [Subscription.request] and [Subscription.cancel] must be executed serially
307+
*/
308+
@Synchronized
309+
private fun withSubscriptionLock(block: () -> Unit) {
310+
block()
311+
}
292312
})
293313
}
294314

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.coroutines.reactive
6+
7+
import kotlinx.coroutines.*
8+
import org.junit.*
9+
import org.reactivestreams.*
10+
import java.util.concurrent.locks.*
11+
12+
/**
13+
* This test checks implementation of rule 2.7 for await methods - serial execution of subscription methods
14+
*/
15+
class AwaitCancellationStressTest : TestBase() {
16+
private val iterations = 10_000 * stressTestMultiplier
17+
18+
@Test
19+
fun testAwaitCancellationOrder() = runTest {
20+
repeat(iterations) {
21+
val job = launch(Dispatchers.Default) {
22+
testPublisher().awaitFirst()
23+
}
24+
job.cancelAndJoin()
25+
}
26+
}
27+
28+
private fun testPublisher() = Publisher<Int> { s ->
29+
val lock = ReentrantLock()
30+
s.onSubscribe(object : Subscription {
31+
override fun request(n: Long) {
32+
check(lock.tryLock())
33+
s.onNext(42)
34+
lock.unlock()
35+
}
36+
37+
override fun cancel() {
38+
check(lock.tryLock())
39+
lock.unlock()
40+
}
41+
})
42+
}
43+
}

0 commit comments

Comments
 (0)