From f847424b9cea423ace5661d1efb6e4f01483c655 Mon Sep 17 00:00:00 2001 From: Janis Date: Thu, 4 Dec 2025 02:29:19 +0100 Subject: [PATCH] fix: BAC-25 Race Condition: Websocket Promises (#99) Reviewed-on: https://git.janis-eccarius.de/KnockOutWhist/KnockOutWhist-Web/pulls/99 Reviewed-by: lq64 Co-authored-by: Janis Co-committed-by: Janis --- .../app/model/sessions/UserSession.scala | 79 +++++++++---------- .../model/sessions/UserWebsocketActor.scala | 30 +++++++ 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/knockoutwhistweb/app/model/sessions/UserSession.scala b/knockoutwhistweb/app/model/sessions/UserSession.scala index 7e295e7..565fa76 100644 --- a/knockoutwhistweb/app/model/sessions/UserSession.scala +++ b/knockoutwhistweb/app/model/sessions/UserSession.scala @@ -26,6 +26,7 @@ class UserSession(val user: User, val host: Boolean, val gameLobby: GameLobby) e else canInteract = Some(InteractionType.Card) case _ => } + websocketActor.foreach(_.solveRequests()) websocketActor.foreach(_.transmitEventToClient(event)) } @@ -38,49 +39,41 @@ class UserSession(val user: User, val host: Boolean, val gameLobby: GameLobby) e } def handleWebResponse(eventType: String, data: JsObject): Unit = { - lock.lock() - val result = Try { - eventType match { - case "ping" => - // No action needed for Ping - () - case "StartGame" => - gameLobby.startGame(user) - case "PlayCard" => - val maybeCardIndex: Option[String] = (data \ "cardindex").asOpt[String] - maybeCardIndex match { - case Some(index) => - val session = gameLobby.getUserSession(user.id) - gameLobby.playCard(session, index.toInt) - case None => - println("Card Index not found or is not a number.") - } - case "PickTrumpsuit" => - val maybeSuitIndex: Option[Int] = (data \ "suitIndex").asOpt[Int] - maybeSuitIndex match { - case Some(index) => - val session = gameLobby.getUserSession(user.id) - gameLobby.selectTrump(session, index) - case None => - println("Card Index not found or is not a number.") - } - case "KickPlayer" => - val maybePlayerId: Option[String] = (data \ "playerId").asOpt[String] - maybePlayerId match { - case Some(id) => - val playerUUID = UUID.fromString(id) - gameLobby.leaveGame(playerUUID, true) - case None => - println("Player ID not found or is not a valid UUID.") - } - case "ReturnToLobby" => - gameLobby.returnToLobby(this) - } - } - lock.unlock() - if (result.isFailure) { - val throwable = result.failed.get - throw throwable + eventType match { + case "ping" => + // No action needed for Ping + () + case "StartGame" => + gameLobby.startGame(user) + case "PlayCard" => + val maybeCardIndex: Option[String] = (data \ "cardindex").asOpt[String] + maybeCardIndex match { + case Some(index) => + val session = gameLobby.getUserSession(user.id) + gameLobby.playCard(session, index.toInt) + case None => + println("Card Index not found or is not a number.") + } + case "PickTrumpsuit" => + val maybeSuitIndex: Option[Int] = (data \ "suitIndex").asOpt[Int] + maybeSuitIndex match { + case Some(index) => + val session = gameLobby.getUserSession(user.id) + gameLobby.selectTrump(session, index) + case None => + println("Card Index not found or is not a number.") + } + case "KickPlayer" => + val maybePlayerId: Option[String] = (data \ "playerId").asOpt[String] + maybePlayerId match { + case Some(id) => + val playerUUID = UUID.fromString(id) + gameLobby.leaveGame(playerUUID, true) + case None => + println("Player ID not found or is not a valid UUID.") + } + case "ReturnToLobby" => + gameLobby.returnToLobby(this) } } diff --git a/knockoutwhistweb/app/model/sessions/UserWebsocketActor.scala b/knockoutwhistweb/app/model/sessions/UserWebsocketActor.scala index b2b59c7..782954c 100644 --- a/knockoutwhistweb/app/model/sessions/UserWebsocketActor.scala +++ b/knockoutwhistweb/app/model/sessions/UserWebsocketActor.scala @@ -5,6 +5,7 @@ import org.apache.pekko.actor.{Actor, ActorRef} import play.api.libs.json.{JsObject, JsValue, Json} import util.WebsocketEventMapper +import scala.collection.mutable import scala.util.{Failure, Success, Try} class UserWebsocketActor( @@ -12,6 +13,8 @@ class UserWebsocketActor( session: UserSession ) extends Actor { + private val requests: mutable.Map[String, String] = mutable.Map() + { session.lock.lock() if (session.websocketActor.isDefined) { @@ -48,12 +51,14 @@ class UserWebsocketActor( } private def handle(json: JsValue): Unit = { + session.lock.lock() val idOpt = (json \ "id").asOpt[String] if (idOpt.isEmpty) { transmitJsonToClient(Json.obj( "status" -> "error", "error" -> "Missing 'id' field" )) + session.lock.unlock() return } val id = idOpt.get @@ -65,17 +70,25 @@ class UserWebsocketActor( "status" -> "error", "error" -> "Missing 'event' field" )) + session.lock.unlock() return } val statusOpt = (json \ "status").asOpt[String] if (statusOpt.isDefined) { + session.lock.unlock() return } val event = eventOpt.get val data = (json \ "data").asOpt[JsObject].getOrElse(Json.obj()) + requests += (id -> event) val result = Try { session.handleWebResponse(event, data) } + if (!requests.contains(id)) { + session.lock.unlock() + return + } + requests -= id if (result.isSuccess) { transmitJsonToClient(Json.obj( "id" -> id, @@ -90,6 +103,7 @@ class UserWebsocketActor( "error" -> result.failed.get.getMessage )) } + session.lock.unlock() } def transmitJsonToClient(jsonObj: JsValue): Unit = { @@ -100,4 +114,20 @@ class UserWebsocketActor( transmitJsonToClient(WebsocketEventMapper.toJson(event, session)) } + def solveRequests(): Unit = { + if (!session.lock.isHeldByCurrentThread) + return; + if (requests.isEmpty) + return; + val pendingRequests = requests.toMap + requests.clear() + pendingRequests.foreach { case (id, event) => + transmitJsonToClient(Json.obj( + "id" -> id, + "event" -> event, + "status" -> "success" + )) + } + } + }