Skip to content

Commit

Permalink
Fix spring websocket adapter's deadlock
Browse files Browse the repository at this point in the history
Related:
#72
  • Loading branch information
Joffrey Bion committed Sep 26, 2020
1 parent 446b7b2 commit fc61d17
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 3 deletions.
4 changes: 4 additions & 0 deletions krossbow-websocket-spring/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ dependencies {
// JSR 356 - Java API for WebSocket (reference implementation)
// Low-level implementation required by Spring's client
implementation("org.glassfish.tyrus.bundles:tyrus-standalone-client-jdk:1.15")

implementation(kotlin("test"))
implementation(kotlin("test-junit"))
testImplementation(project(":krossbow-websocket-test"))
}

val dokkaJar by tasks.creating(Jar::class) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.hildan.krossbow.websocket.spring

import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.channels.ReceiveChannel
import kotlinx.coroutines.future.await
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.hildan.krossbow.websocket.WebSocketFrame
Expand Down Expand Up @@ -66,7 +68,12 @@ private class KrossbowToSpringHandlerAdapter : WebSocketHandler {
}

override fun afterConnectionClosed(session: SpringWebSocketSession, closeStatus: CloseStatus) {
runBlocking {
// afterConnectionClosed is synchronously called by Tyrus implementation during a session.close() call.
// It is not called when receiving the server close frame when the closure is initiated on the client side.
// Source: org.glassfish.tyrus.core.ProtocolHandler.close()
// This means that if no receiver is listening on the incoming frames channel, onClose() here may suspend
// forever. That's why we need an asynchronous dispatch of onClose() here.
GlobalScope.launch {
channelListener.onClose(closeStatus.code, closeStatus.reason)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.hildan.krossbow.websocket.spring

import org.hildan.krossbow.websocket.WebSocketClient
import org.hildan.krossbow.websocket.test.WebSocketClientTestSuite

class SpringWebSocketClientTest : WebSocketClientTestSuite() {

override fun provideClient(): WebSocketClient = SpringDefaultWebSocketClient
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package org.hildan.krossbow.websocket.test

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.yield
import kotlinx.coroutines.delay
import org.hildan.krossbow.websocket.WebSocketClient
import org.hildan.krossbow.websocket.WebSocketCloseCodes
import org.hildan.krossbow.websocket.WebSocketFrame
Expand Down Expand Up @@ -55,6 +55,6 @@ suspend fun testEchoWs(websocketClient: WebSocketClient, url: String) {
assertTrue(closeFrame is WebSocketFrame.Close, "Last frame should be a close frame")
assertEquals(WebSocketCloseCodes.NORMAL_CLOSURE, closeFrame.code)

yield() // somehow isClosedForReceive needs some time (or at least to regain control)
delay(20) // somehow isClosedForReceive needs some time to become true
assertTrue(session.incomingFrames.isClosedForReceive, "The incoming frames channel should be closed")
}

0 comments on commit fc61d17

Please sign in to comment.