Skip to content

Commit 0aa6db0

Browse files
committed
Fixed JobSupport single->list upgrade procedure.
It was hanging at LockFreeLinkedList.helpDelete.
1 parent db0d4fc commit 0aa6db0

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/internal/LockFreeLinkedList.kt

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,12 +528,45 @@ public open class LockFreeLinkedListNode {
528528
private fun markPrev(): Node {
529529
_prev.loop { prev ->
530530
if (prev is Removed) return prev.ref
531-
check(prev !== this) { "Cannot remove node that was not fully added" }
532-
val removedPrev = (prev as Node).removed()
531+
// See detailed comment in findHead on why `prev === this` is a special case for which we know that
532+
// the prev should have being pointing to the head of list but finishAdd that was supposed
533+
// to do that is not complete yet.
534+
val removedPrev = (if (prev === this) findHead() else (prev as Node)).removed()
533535
if (_prev.compareAndSet(prev, removedPrev)) return prev
534536
}
535537
}
536538

539+
/**
540+
* Finds the head of the list (implementing [LockFreeLinkedListHead]) by following [next] pointers.
541+
*
542+
* The code in [kotlinx.coroutines.experimental.JobSupport] performs upgrade of a single node to a list.
543+
* It uses [addOneIfEmpty] to add the list head to "empty list of a single node" once.
544+
* During upgrade a transient state of the list looks like this:
545+
*
546+
* ```
547+
* +-----------------+
548+
* | |
549+
* node V head |
550+
* +---+---+ +---+---+ |
551+
* +-> | P | N | --> | P | N |-+
552+
* | +---+---+ +---+---+
553+
* | | ^ |
554+
* +---- + +---------+
555+
* ```
556+
*
557+
* The [prev] pointer in `node` still points to itself when [finishAdd] (invoked inside [addOneIfEmpty])
558+
* has not completed yet. If this state is observed, then we know that [prev] should have been pointing
559+
* to the list head. This function is looking up the head by following consistent chain of [next] pointers.
560+
*/
561+
private fun findHead(): Node {
562+
var cur = this
563+
while (true) {
564+
if (cur is LockFreeLinkedListHead) return cur
565+
cur = cur.next.unwrap()
566+
check(cur !== this) { "Cannot loop to this while looking for list head" }
567+
}
568+
}
569+
537570
// fixes next links to the left of this node
538571
@PublishedApi
539572
internal fun helpDelete() {

core/kotlinx-coroutines-core/src/test/kotlin/kotlinx/coroutines/experimental/JobHandlersUpgradeStressTest.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class JobHandlersUpgradeStressTest : TestBase() {
5555
cyclicBarrier.await()
5656
val job = job ?: break
5757
// burn some time
58-
repeat(rnd.nextInt(30)) { sink.incrementAndGet() }
58+
repeat(rnd.nextInt(3000)) { sink.incrementAndGet() }
5959
// cancel job
6060
job.cancel()
6161
cyclicBarrier.await()
@@ -72,14 +72,14 @@ class JobHandlersUpgradeStressTest : TestBase() {
7272
val job = job ?: break
7373
val state = State()
7474
// burn some time
75-
repeat(rnd.nextInt(10)) { sink.incrementAndGet() }
75+
repeat(rnd.nextInt(1000)) { sink.incrementAndGet() }
7676
val handle =
7777
job.invokeOnCompletion(onCancelling = onCancelling, invokeImmediately = invokeImmediately) {
7878
if (!state.state.compareAndSet(0, 1))
7979
error("Fired more than once or too late: state=${state.state.value}")
8080
}
8181
// burn some time
82-
repeat(rnd.nextInt(10)) { sink.incrementAndGet() }
82+
repeat(rnd.nextInt(1000)) { sink.incrementAndGet() }
8383
// dispose
8484
handle.dispose()
8585
cyclicBarrier.await()

0 commit comments

Comments
 (0)