Skip to content

Commit 8dceb17

Browse files
committed
Default delay improvements:
* Fixed memory leak when coroutine was cancelled before delay() invocation * synchronized(this) is replaced with @synchronized to reduce TSH footprint
1 parent 1d48a7c commit 8dceb17

File tree

2 files changed

+37
-23
lines changed

2 files changed

+37
-23
lines changed

core/kotlinx-coroutines-core/src/EventLoop.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ public fun EventLoop(thread: Thread = Thread.currentThread(), parentJob: Job? =
6767
public fun EventLoop_Deprecated(thread: Thread = Thread.currentThread(), parentJob: Job? = null): CoroutineDispatcher =
6868
EventLoop(thread, parentJob) as CoroutineDispatcher
6969

70-
private const val DELAYED = 0
71-
private const val REMOVED = 1
72-
private const val RESCHEDULED = 2
70+
internal const val DELAYED = 0
71+
internal const val REMOVED = 1
72+
internal const val RESCHEDULED = 2
7373

7474
@Suppress("PrivatePropertyName")
7575
private val CLOSED_EMPTY = Symbol("CLOSED_EMPTY")
@@ -226,10 +226,10 @@ internal abstract class EventLoopBase: CoroutineDispatcher(), Delay, EventLoop {
226226

227227
internal fun schedule(delayedTask: DelayedTask) {
228228
if (scheduleImpl(delayedTask)) {
229-
// todo: we should unpark only when this delayed task became first in the queue
230229
unpark()
231-
} else
230+
} else {
232231
DefaultExecutor.schedule(delayedTask)
232+
}
233233
}
234234

235235
private fun scheduleImpl(delayedTask: DelayedTask): Boolean {

core/kotlinx-coroutines-core/src/internal/ThreadSafeHeap.kt

+32-18
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
package kotlinx.coroutines.experimental.internal
66

7+
import kotlinx.coroutines.experimental.*
78
import java.util.*
89

910
/**
@@ -26,42 +27,50 @@ public class ThreadSafeHeap<T> : SynchronizedObject() where T: ThreadSafeHeapNod
2627

2728
public val isEmpty: Boolean get() = size == 0
2829

29-
public fun clear() = synchronized(this) {
30+
@Synchronized
31+
public fun clear() {
3032
Arrays.fill(a, 0, size, null)
3133
size = 0
3234
}
3335

34-
public fun peek(): T? = synchronized(this) { firstImpl() }
36+
@Synchronized
37+
public fun peek(): T? = firstImpl()
3538

36-
public fun removeFirstOrNull(): T? = synchronized(this) {
37-
if (size > 0) {
39+
@Synchronized
40+
public fun removeFirstOrNull(): T? {
41+
return if (size > 0) {
3842
removeAtImpl(0)
39-
} else
43+
} else {
4044
null
45+
}
4146
}
4247

43-
public inline fun removeFirstIf(predicate: (T) -> Boolean): T? = synchronized(this) {
44-
val first = firstImpl() ?: return@synchronized null
45-
if (predicate(first)) {
48+
@Synchronized
49+
public inline fun removeFirstIf(predicate: (T) -> Boolean): T? {
50+
val first = firstImpl() ?: return null
51+
return if (predicate(first)) {
4652
removeAtImpl(0)
47-
} else
53+
} else {
4854
null
55+
}
4956
}
5057

51-
public fun addLast(node: T) = synchronized(this) {
52-
addImpl(node)
53-
}
58+
@Synchronized
59+
public fun addLast(node: T) = addImpl(node)
5460

55-
public fun addLastIf(node: T, cond: () -> Boolean): Boolean = synchronized(this) {
56-
if (cond()) {
61+
@Synchronized
62+
public fun addLastIf(node: T, cond: () -> Boolean): Boolean {
63+
return if (cond()) {
5764
addImpl(node)
5865
true
59-
} else
66+
} else {
6067
false
68+
}
6169
}
6270

63-
public fun remove(node: T): Boolean = synchronized(this) {
64-
if (node.index < 0) {
71+
@Synchronized
72+
public fun remove(node: T): Boolean {
73+
return if (node.index < 0) {
6574
false
6675
} else {
6776
removeAtImpl(node.index)
@@ -95,6 +104,11 @@ public class ThreadSafeHeap<T> : SynchronizedObject() where T: ThreadSafeHeapNod
95104

96105
@PublishedApi
97106
internal fun addImpl(node: T) {
107+
// TODO remove this after #541 when ThreadSafeHeapNode is gone
108+
if (node is EventLoopBase.DelayedTask && node.state == REMOVED) {
109+
return
110+
}
111+
98112
val a = realloc()
99113
val i = size++
100114
a[i] = node
@@ -140,4 +154,4 @@ public class ThreadSafeHeap<T> : SynchronizedObject() where T: ThreadSafeHeapNod
140154
ni.index = i
141155
nj.index = j
142156
}
143-
}
157+
}

0 commit comments

Comments
 (0)