Skip to content

Commit 733b8e1

Browse files
e5lclaude
andauthored
KTOR-9373 Make ConcurrentMap iteration safe on Native (#5407)
* KTOR-9373 Add test for CIOEngine.close() with active endpoints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * KTOR-9373 Make ConcurrentMap iteration safe on Native Return snapshot copies from entries/keys/values to match JVM's ConcurrentHashMap weakly-consistent iteration semantics. Also synchronize size and isEmpty() which were reading without the lock. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up CIOEngine close test: remove issue prefix and comments Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback (iteration 1) - Fix ConcurrentModificationException in ConcurrentMap.entries on Native: use LinkedHashMap(delegate).entries to create entries that don't hold references to the original backing map's EntryRef objects --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1f83f21 commit 733b8e1

2 files changed

Lines changed: 19 additions & 5 deletions

File tree

ktor-client/ktor-client-cio/common/test/CIOEngineTest.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,20 @@ class CIOEngineTest : ClientEngineTest<CIOEngineConfig>(CIO) {
171171
}
172172
}
173173

174+
@Test
175+
fun `close engine with active endpoints does not throw ConcurrentModificationException`() = testClient {
176+
test { client ->
177+
val jobs = (1..5).map { i ->
178+
client.async {
179+
runCatching {
180+
client.get("$TEST_SERVER/content/hello?i=$i")
181+
}
182+
}
183+
}
184+
jobs.forEach { it.await() }
185+
}
186+
}
187+
174188
@Test
175189
fun testErrorMessageWhenServerDontRespondWithUpgrade() = testClient {
176190
config {

ktor-utils/posix/src/io/ktor/util/collections/ConcurrentMapNative.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,24 +32,24 @@ public actual class ConcurrentMap<Key, Value> public actual constructor(
3232
}
3333

3434
actual override val size: Int
35-
get() = delegate.size
35+
get() = synchronized(lock) { delegate.size }
3636

3737
actual override fun containsKey(key: Key): Boolean = synchronized(lock) { delegate.containsKey(key) }
3838

3939
actual override fun containsValue(value: Value): Boolean = synchronized(lock) { delegate.containsValue(value) }
4040

4141
actual override fun get(key: Key): Value? = synchronized(lock) { delegate[key] }
4242

43-
actual override fun isEmpty(): Boolean = delegate.isEmpty()
43+
actual override fun isEmpty(): Boolean = synchronized(lock) { delegate.isEmpty() }
4444

4545
actual override val entries: MutableSet<MutableMap.MutableEntry<Key, Value>>
46-
get() = synchronized(lock) { delegate.entries }
46+
get() = synchronized(lock) { LinkedHashMap(delegate).entries }
4747

4848
actual override val keys: MutableSet<Key>
49-
get() = synchronized(lock) { delegate.keys }
49+
get() = synchronized(lock) { LinkedHashSet(delegate.keys) }
5050

5151
actual override val values: MutableCollection<Value>
52-
get() = synchronized(lock) { delegate.values }
52+
get() = synchronized(lock) { ArrayList(delegate.values) }
5353

5454
actual override fun clear() {
5555
synchronized(lock) {

0 commit comments

Comments
 (0)