From 10d56dd3939eb550523a5f485ec1b0430594a6a9 Mon Sep 17 00:00:00 2001 From: Matt Mullins Date: Wed, 8 Apr 2020 22:18:03 -0700 Subject: [PATCH] User: use a stably-ordered queue for long-polling If the AJAX request for LongPollServlet happens with enough of a delay, there may be two stateful messages in the queue -- I have observed two GAME_STATE_CHANGE messages enqueued simultaneously when the Card Czar does not long-poll while the other players progress the game through PLAYING to JUDGING. The JS code overwrites the state as it iterates the list, so the "last state wins". But PriorityQueue explicitly does not guarantee any particular ordering among elements that compare "equal", causing the client to think the game is PLAYING. --- .../socialgamer/cah/data/ConnectedUsers.java | 3 +-- .../java/net/socialgamer/cah/data/Game.java | 3 +-- .../socialgamer/cah/data/QueuedMessage.java | 18 ++---------------- .../java/net/socialgamer/cah/data/User.java | 7 ++++--- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/main/java/net/socialgamer/cah/data/ConnectedUsers.java b/src/main/java/net/socialgamer/cah/data/ConnectedUsers.java index 01ed78d..e693791 100644 --- a/src/main/java/net/socialgamer/cah/data/ConnectedUsers.java +++ b/src/main/java/net/socialgamer/cah/data/ConnectedUsers.java @@ -269,8 +269,7 @@ public class ConnectedUsers { * @param broadcastTo * List of users to broadcast the message to. * @param type - * Type of message to broadcast. This determines the order the messages are returned by - * priority. + * Type of message to broadcast. * @param masterData * Message data to broadcast. */ diff --git a/src/main/java/net/socialgamer/cah/data/Game.java b/src/main/java/net/socialgamer/cah/data/Game.java index 4924204..3018c1a 100644 --- a/src/main/java/net/socialgamer/cah/data/Game.java +++ b/src/main/java/net/socialgamer/cah/data/Game.java @@ -463,8 +463,7 @@ public class Game { * Broadcast a message to all players in this game. * * @param type - * Type of message to broadcast. This determines the order the messages are returned by - * priority. + * Type of message to broadcast. * @param masterData * Message data to broadcast. */ diff --git a/src/main/java/net/socialgamer/cah/data/QueuedMessage.java b/src/main/java/net/socialgamer/cah/data/QueuedMessage.java index 92e9f40..5c91cf4 100644 --- a/src/main/java/net/socialgamer/cah/data/QueuedMessage.java +++ b/src/main/java/net/socialgamer/cah/data/QueuedMessage.java @@ -33,7 +33,7 @@ import net.socialgamer.cah.Constants.ReturnableData; * * @author Andy Janata (ajanata@socialgamer.net) */ -public class QueuedMessage implements Comparable { +public class QueuedMessage { private final MessageType messageType; private final Map data; @@ -42,8 +42,7 @@ public class QueuedMessage implements Comparable { * Create a new queued message. * * @param messageType - * Type of message to be queued. The type influences the priority in returning messages - * to the client. + * Type of message to be queued. * @param data * The data of the message to be queued. */ @@ -66,15 +65,6 @@ public class QueuedMessage implements Comparable { return data; } - /** - * This is not guaranteed to be consistent with .equals() since we do not care about the data for - * ordering. - */ - @Override - public int compareTo(final QueuedMessage qm) { - return this.messageType.getWeight() - qm.messageType.getWeight(); - } - @Override public String toString() { return messageType.toString() + "_" + data.toString(); @@ -92,9 +82,5 @@ public class QueuedMessage implements Comparable { MessageType(final int weight) { this.weight = weight; } - - public int getWeight() { - return weight; - } } } diff --git a/src/main/java/net/socialgamer/cah/data/User.java b/src/main/java/net/socialgamer/cah/data/User.java index fccb9f0..c360c93 100644 --- a/src/main/java/net/socialgamer/cah/data/User.java +++ b/src/main/java/net/socialgamer/cah/data/User.java @@ -26,7 +26,8 @@ package net.socialgamer.cah.data; import java.util.ArrayList; import java.util.Collection; import java.util.Date; -import java.util.concurrent.PriorityBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import javax.annotation.Nullable; @@ -54,7 +55,7 @@ public class User { private final String idCode; - private final PriorityBlockingQueue queuedMessages; + private final BlockingQueue queuedMessages; private final Object queuedMessageSynchronization = new Object(); @@ -121,7 +122,7 @@ public class User { this.sessionId = sessionId; this.clientLanguage = clientLanguage == null ? "" : clientLanguage; agent = UADetectorServiceFactory.getResourceModuleParser().parse(clientAgent); - queuedMessages = new PriorityBlockingQueue(); + queuedMessages = new LinkedBlockingQueue(); } public interface Factory {