Skip to content

Commit eaf9504

Browse files
author
Daniel Rees
authored
Fixed concurrent exception (#66)
1 parent 77ba73c commit eaf9504

File tree

2 files changed

+41
-1
lines changed

2 files changed

+41
-1
lines changed

src/main/kotlin/org/phoenixframework/Socket.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,12 @@ class Socket(
277277
}
278278

279279
fun remove(channel: Channel) {
280-
this.channels.removeAll { it.joinRef == channel.joinRef }
280+
// To avoid a ConcurrentModificationException, filter out the channels to be
281+
// removed instead of calling .remove() on the list, thus returning a new list
282+
// that does not contain the channel that was removed.
283+
this.channels = channels
284+
.filter { it.joinRef != channel.joinRef }
285+
.toMutableList()
281286
}
282287

283288
//------------------------------------------------------------------------------

src/test/kotlin/org/phoenixframework/SocketTest.kt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,41 @@ class SocketTest {
386386
assertThat(socket.channels).contains(channel2)
387387
}
388388

389+
@Test
390+
internal fun `does not throw exception when iterating over channels`() {
391+
val channel1 = socket.channel("topic-1")
392+
val channel2 = socket.channel("topic-2")
393+
394+
channel1.joinPush.ref = "1"
395+
channel2.joinPush.ref = "2"
396+
397+
channel1.join().trigger("ok", emptyMap())
398+
channel2.join().trigger("ok", emptyMap())
399+
400+
401+
var chan1Called = false
402+
channel1.onError { chan1Called = true }
403+
404+
var chan2Called = false
405+
channel2.onError {
406+
chan2Called = true
407+
socket.remove(channel2)
408+
}
409+
410+
// This will trigger an iteration over the socket.channels list which will trigger
411+
// channel2.onError. That callback will attempt to remove channel2 during iteration
412+
// which would throw a ConcurrentModificationException if the socket.remove method
413+
// is implemented incorrectly.
414+
socket.onConnectionError(IllegalStateException(), null)
415+
416+
// Assert that both on all error's got called even when a channel was removed
417+
assertThat(chan1Called).isTrue()
418+
assertThat(chan2Called).isTrue()
419+
420+
assertThat(socket.channels).doesNotContain(channel2)
421+
assertThat(socket.channels).contains(channel1)
422+
}
423+
389424
/* End Remove */
390425
}
391426

0 commit comments

Comments
 (0)