Skip to content

Commit 5a570e1

Browse files
authored
Work around KT-58685 (#3881)
Implement `withLock` and `withPermit` without try/finally, as the idiomatic implementation leads to incorrect code being produced by the compiler.
1 parent 2b5d93f commit 5a570e1

File tree

4 files changed

+51
-7
lines changed

4 files changed

+51
-7
lines changed

kotlinx-coroutines-core/common/src/sync/Mutex.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,18 @@ public suspend inline fun <T> Mutex.withLock(owner: Any? = null, action: () -> T
122122
callsInPlace(action, InvocationKind.EXACTLY_ONCE)
123123
}
124124

125+
// Cannot use 'finally' in this function because of KT-58685
126+
// See kotlinx.coroutines.sync.MutexTest.testWithLockJsMiscompilation
127+
125128
lock(owner)
126-
try {
127-
return action()
128-
} finally {
129+
val result = try {
130+
action()
131+
} catch (e: Throwable) {
129132
unlock(owner)
133+
throw e
130134
}
135+
unlock(owner)
136+
return result
131137
}
132138

133139

kotlinx-coroutines-core/common/src/sync/Semaphore.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,18 @@ public suspend inline fun <T> Semaphore.withPermit(action: () -> T): T {
8383
callsInPlace(action, InvocationKind.EXACTLY_ONCE)
8484
}
8585

86+
// Cannot use 'finally' in this function because of KT-58685
87+
// See kotlinx.coroutines.sync.SemaphoreTest.testWithPermitJsMiscompilation
88+
8689
acquire()
87-
try {
88-
return action()
89-
} finally {
90+
val result = try {
91+
action()
92+
} catch (e: Throwable) {
9093
release()
94+
throw e
9195
}
96+
release()
97+
return result
9298
}
9399

94100
@Suppress("UNCHECKED_CAST")

kotlinx-coroutines-core/common/test/sync/MutexTest.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,20 @@ class MutexTest : TestBase() {
148148
assertFailsWith<IllegalStateException> { mutex.lock(owner) }
149149
assertFailsWith<IllegalStateException> { select { mutex.onLock(owner) {} } }
150150
}
151+
152+
@Test
153+
fun testWithLockJsMiscompilation() = runTest {
154+
// This is a reproducer for KT-58685
155+
// On Kotlin/JS IR, the compiler miscompiles calls to 'unlock' in an inlined finally
156+
// This is visible on the withLock function
157+
// Until the compiler bug is fixed, this test case checks that we do not suffer from it
158+
val mutex = Mutex()
159+
assertFailsWith<IndexOutOfBoundsException> {
160+
try {
161+
mutex.withLock { null } ?: throw IndexOutOfBoundsException() // should throw…
162+
} catch (e: Exception) {
163+
throw e // …but instead fails here
164+
}
165+
}
166+
}
151167
}

kotlinx-coroutines-core/common/test/sync/SemaphoreTest.kt

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,20 @@ class SemaphoreTest : TestBase() {
168168
assertFailsWith<IllegalArgumentException> { Semaphore(1, -1) }
169169
assertFailsWith<IllegalArgumentException> { Semaphore(1, 2) }
170170
}
171-
}
171+
172+
@Test
173+
fun testWithPermitJsMiscompilation() = runTest {
174+
// This is a reproducer for KT-58685
175+
// On Kotlin/JS IR, the compiler miscompiles calls to 'release' in an inlined finally
176+
// This is visible on the withPermit function
177+
// Until the compiler bug is fixed, this test case checks that we do not suffer from it
178+
val semaphore = Semaphore(1)
179+
assertFailsWith<IndexOutOfBoundsException> {
180+
try {
181+
semaphore.withPermit { null } ?: throw IndexOutOfBoundsException() // should throw…
182+
} catch (e: Exception) {
183+
throw e // …but instead fails here
184+
}
185+
}
186+
}
187+
}

0 commit comments

Comments
 (0)