From 0276911b88d99501b5ceb05d44a9938d08b81400 Mon Sep 17 00:00:00 2001 From: Calum Date: Thu, 17 Aug 2017 14:02:18 +1200 Subject: [PATCH] Fixed disconnection issues. Fix is only temporary until a consistent interface for disconnections exists. #bug #implement --- .../seng302/gameServer/MainServerThread.java | 17 +++--- .../gameServer/ServerListenThread.java | 2 +- .../gameServer/ServerToClientThread.java | 54 ++++++++++++------- .../visualiser/ClientToServerThread.java | 23 ++++---- .../java/seng302/visualiser/GameClient.java | 15 +++--- .../DisconnectionTest.java | 20 +++++++ .../RegularPacketsTest.java | 2 +- src/test/java/steps/ToggleSailSteps.java | 2 +- 8 files changed, 92 insertions(+), 43 deletions(-) create mode 100644 src/test/java/seng302/visualiser/ClientToServerTests/DisconnectionTest.java diff --git a/src/main/java/seng302/gameServer/MainServerThread.java b/src/main/java/seng302/gameServer/MainServerThread.java index f4dc387d..9cbcb8ca 100644 --- a/src/main/java/seng302/gameServer/MainServerThread.java +++ b/src/main/java/seng302/gameServer/MainServerThread.java @@ -93,6 +93,9 @@ public class MainServerThread implements Runnable, ClientConnectionDelegate { // TODO: 14/07/17 wmu16 - Send out disconnect packet to clients try { + for (ServerToClientThread serverToClientThread : serverToClientThreads) { + serverToClientThread.terminate(); + } serverSocket.close(); return; } catch (IOException e) { @@ -173,6 +176,7 @@ public class MainServerThread implements Runnable, ClientConnectionDelegate { thread.sendSetupMessages(); } }); + serverToClientThread.addDisconnectListener(this::clientDisconnected); } /** @@ -182,11 +186,11 @@ public class MainServerThread implements Runnable, ClientConnectionDelegate { */ @Override public void clientDisconnected(Player player) { - try { - player.getSocket().close(); - } catch (Exception e) { - serverLog("Cannot disconnect the socket for the disconnected player.", 0); - } +// try { +// player.getSocket().close(); +// } catch (Exception e) { +// serverLog("Cannot disconnect the socket for the disconnected player.", 0); +// } serverLog("Player " + player.getYacht().getSourceId() + "'s socket disconnected", 0); GameState.removeYacht(player.getYacht().getSourceId()); GameState.removePlayer(player); @@ -194,11 +198,12 @@ public class MainServerThread implements Runnable, ClientConnectionDelegate { for (ServerToClientThread serverToClientThread : serverToClientThreads) { if (serverToClientThread.getSocket() == player.getSocket()) { closedConnection = serverToClientThread; - } else { + } else if (GameState.getCurrentStage() != GameStages.RACING){ serverToClientThread.sendSetupMessages(); } } serverToClientThreads.remove(closedConnection); + closedConnection.terminate(); } public void startGame() { diff --git a/src/main/java/seng302/gameServer/ServerListenThread.java b/src/main/java/seng302/gameServer/ServerListenThread.java index e05b76a9..2c220ca2 100644 --- a/src/main/java/seng302/gameServer/ServerListenThread.java +++ b/src/main/java/seng302/gameServer/ServerListenThread.java @@ -38,7 +38,7 @@ public class ServerListenThread implements Runnable { } public void run(){ - while (true){ + while (serverSocket != null && !serverSocket.isClosed()){ acceptConnection(); } } diff --git a/src/main/java/seng302/gameServer/ServerToClientThread.java b/src/main/java/seng302/gameServer/ServerToClientThread.java index 479333bd..088fbd14 100644 --- a/src/main/java/seng302/gameServer/ServerToClientThread.java +++ b/src/main/java/seng302/gameServer/ServerToClientThread.java @@ -19,26 +19,22 @@ import java.util.zip.CRC32; import java.util.zip.Checksum; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import seng302.gameServer.messages.BoatAction; +import seng302.gameServer.messages.BoatLocationMessage; +import seng302.gameServer.messages.ClientType; +import seng302.gameServer.messages.Message; +import seng302.gameServer.messages.RegistrationResponseMessage; +import seng302.gameServer.messages.RegistrationResponseStatus; +import seng302.gameServer.messages.XMLMessage; +import seng302.gameServer.messages.XMLMessageSubType; import seng302.gameServer.messages.YachtEventCodeMessage; import seng302.model.Player; +import seng302.model.ServerYacht; import seng302.model.stream.packets.PacketType; import seng302.model.stream.packets.StreamPacket; import seng302.model.stream.xml.generator.Race; import seng302.model.stream.xml.generator.Regatta; import seng302.utilities.XMLGenerator; -import seng302.gameServer.messages.BoatAction; -import seng302.gameServer.messages.BoatLocationMessage; -import seng302.gameServer.messages.BoatSubMessage; -import seng302.gameServer.messages.ClientType; -import seng302.gameServer.messages.Message; -import seng302.gameServer.messages.RaceStatus; -import seng302.gameServer.messages.RaceStatusMessage; -import seng302.gameServer.messages.RaceType; -import seng302.gameServer.messages.RegistrationResponseMessage; -import seng302.gameServer.messages.RegistrationResponseStatus; -import seng302.gameServer.messages.XMLMessage; -import seng302.gameServer.messages.XMLMessageSubType; -import seng302.model.ServerYacht; /** * A class describing a single connection to a Client for the purposes of sending and receiving on @@ -55,6 +51,12 @@ public class ServerToClientThread implements Runnable, Observer { void notifyConnection (); } + // TODO: 17/08/17 this is only temporary disconnects should be handled consistently + @FunctionalInterface + interface DisconnectListener { + void notifyDisconnect (Player player); + } + private Logger logger = LoggerFactory.getLogger(ServerToClientThread.class); private Thread thread; @@ -74,8 +76,10 @@ public class ServerToClientThread implements Runnable, Observer { private XMLGenerator xml; private List connectionListeners = new ArrayList<>(); + private DisconnectListener disconnectListener; private ServerYacht yacht; + private Player player; public ServerToClientThread(Socket socket) { this.socket = socket; @@ -122,8 +126,9 @@ public class ServerToClientThread implements Runnable, Observer { ); yacht.addObserver(this); // TODO: yacht can notify mark rounding message hyi25 13/8/17 + player = new Player(socket, yacht); GameState.addYacht(sourceId, yacht); - GameState.addPlayer(new Player(socket, yacht)); + GameState.addPlayer(player); } @Override @@ -169,8 +174,7 @@ public class ServerToClientThread implements Runnable, Observer { int sync2; // TODO: 14/07/17 wmu16 - Work out how to fix this while loop - while (socket.isConnected()) { - + while (socket.isConnected() && !socket.isClosed()) { try { crcBuffer = new ByteArrayOutputStream(); sync1 = readByte(); @@ -213,6 +217,7 @@ public class ServerToClientThread implements Runnable, Observer { return; } } + logger.warn("Closed serverToClientThread" + thread, 1); } public void sendSetupMessages() { @@ -252,11 +257,12 @@ public class ServerToClientThread implements Runnable, Observer { private int readByte() throws Exception { int currentByte = -1; try { - // @TODO @FIX ConnectionReset Exception when a client disconnects before it is garbage collected currentByte = is.read(); crcBuffer.write(currentByte); + } catch (SocketException se) { + disconnectListener.notifyDisconnect(this.player); } catch (IOException e) { - e.printStackTrace(); + disconnectListener.notifyDisconnect(this.player); logger.warn("Socket read failed", 1); } if (currentByte == -1) { @@ -335,4 +341,16 @@ public class ServerToClientThread implements Runnable, Observer { public void removeConnectionListener(ConnectionListener listener) { connectionListeners.remove(listener); } + + public void terminate () { + try { + socket.close(); + } catch (IOException ioe) { + logger.warn("IOException attempting to terminate serverToClientThread " + this.thread); + } + } + + public void addDisconnectListener(DisconnectListener disconnectListener) { + this.disconnectListener = disconnectListener; + } } diff --git a/src/main/java/seng302/visualiser/ClientToServerThread.java b/src/main/java/seng302/visualiser/ClientToServerThread.java index 10001869..3655f080 100644 --- a/src/main/java/seng302/visualiser/ClientToServerThread.java +++ b/src/main/java/seng302/visualiser/ClientToServerThread.java @@ -142,18 +142,20 @@ public class ClientToServerThread implements Runnable { } } catch (ByteReadException e) { logger.warn("Byte read exception on ClientToServerThread", 1); - closeSocket(); notifyDisconnectListeners("Connection to server was interrupted"); + closeSocket(); } } logger.warn("Closed connection to server", 1); - closeSocket(); notifyDisconnectListeners("Connection to server was terminated"); + closeSocket(); } private void notifyDisconnectListeners (String message) { - for (DisconnectedFromHostListener listener : disconnectionListeners) { - listener.notifYDisconnection(message); + if (socketOpen) { + for (DisconnectedFromHostListener listener : disconnectionListeners) { + listener.notifYDisconnection(message); + } } } @@ -167,8 +169,8 @@ public class ClientToServerThread implements Runnable { os.write(requestMessage.getBuffer()); } catch (IOException e) { logger.error("Could not send registration request. Exiting"); - closeSocket(); notifyDisconnectListeners("Failed to register with server"); + closeSocket(); } } @@ -197,8 +199,8 @@ public class ClientToServerThread implements Runnable { else{ alertErrorText = "Could not connect to server"; } - closeSocket(); notifyDisconnectListeners(alertErrorText); + closeSocket(); } /** @@ -208,7 +210,7 @@ public class ClientToServerThread implements Runnable { * - MAINTAIN_HEADING = DOWNWIND and UPWIND packets stop being sent. * @param actionType The boat action that will dictate packets sent. */ - public void sendBoatActionMessage(BoatAction actionType) { + public void sendBoatAction(BoatAction actionType) { switch (actionType) { case MAINTAIN_HEADING: if (upwindTimerFlag) { @@ -272,9 +274,9 @@ public class ClientToServerThread implements Runnable { try { os.write(message.getBuffer()); } catch (IOException e) { - logger.warn("IOException on attempting to sendBoatActionMessage from Client"); - closeSocket(); + logger.warn("IOException on attempting to sendBoatAction from Client"); notifyDisconnectListeners("Cannot communicate with server"); + closeSocket(); } } } @@ -282,6 +284,7 @@ public class ClientToServerThread implements Runnable { private void closeSocket() { try { socket.close(); + socketOpen = false; } catch (IOException e) { logger.warn("IOException on attempting to close ClientToServerSocket"); } @@ -318,8 +321,8 @@ public class ClientToServerThread implements Runnable { crcBuffer.write(currentByte); } catch (IOException e) { logger.warn("IOException on readByte Client side", 1); - closeSocket(); notifyDisconnectListeners("Cannot read from server."); + closeSocket(); } if (currentByte == -1) { throw new ByteReadException("InputStream reach end of stream"); diff --git a/src/main/java/seng302/visualiser/GameClient.java b/src/main/java/seng302/visualiser/GameClient.java index 68cd120c..e55fa180 100644 --- a/src/main/java/seng302/visualiser/GameClient.java +++ b/src/main/java/seng302/visualiser/GameClient.java @@ -104,6 +104,7 @@ public class GameClient { * @param portNumber Port to connect to. */ public void runAsHost(String ipAddress, Integer portNumber) { + server = new MainServerThread(); try { startClientToServerThread(ipAddress, portNumber); socketThread.addDisconnectionListener((cause) -> { @@ -126,6 +127,8 @@ public class GameClient { lobbyController.disableReadyButton(); server.startGame(); } else if (exitCause == CloseStatus.LEAVE) { + server.terminate(); + server = null; loadStartScreen(); } }); @@ -370,13 +373,13 @@ public class GameClient { private void keyPressed(KeyEvent e) { switch (e.getCode()) { case SPACE: // align with vmg - socketThread.sendBoatActionMessage(BoatAction.VMG); break; + socketThread.sendBoatAction(BoatAction.VMG); break; case PAGE_UP: // upwind - socketThread.sendBoatActionMessage(BoatAction.UPWIND); break; + socketThread.sendBoatAction(BoatAction.UPWIND); break; case PAGE_DOWN: // downwind - socketThread.sendBoatActionMessage(BoatAction.DOWNWIND); break; + socketThread.sendBoatAction(BoatAction.DOWNWIND); break; case ENTER: // tack/gybe - socketThread.sendBoatActionMessage(BoatAction.TACK_GYBE); break; + socketThread.sendBoatAction(BoatAction.TACK_GYBE); break; } } @@ -385,12 +388,12 @@ public class GameClient { switch (e.getCode()) { //TODO 12/07/17 Determine the sail state and send the appropriate packet (eg. if sails are in, send a sail out packet) case SHIFT: // sails in/sails out - socketThread.sendBoatActionMessage(BoatAction.SAILS_IN); + socketThread.sendBoatAction(BoatAction.SAILS_IN); allBoatsMap.get(socketThread.getClientId()).toggleSail(); break; case PAGE_UP: case PAGE_DOWN: - socketThread.sendBoatActionMessage(BoatAction.MAINTAIN_HEADING); break; + socketThread.sendBoatAction(BoatAction.MAINTAIN_HEADING); break; } } diff --git a/src/test/java/seng302/visualiser/ClientToServerTests/DisconnectionTest.java b/src/test/java/seng302/visualiser/ClientToServerTests/DisconnectionTest.java new file mode 100644 index 00000000..6bc3e9ad --- /dev/null +++ b/src/test/java/seng302/visualiser/ClientToServerTests/DisconnectionTest.java @@ -0,0 +1,20 @@ +package seng302.visualiser.ClientToServerTests; + +import org.junit.Assert; +import org.junit.Test; +import seng302.gameServer.MainServerThread; +import seng302.visualiser.ClientToServerThread; + +/** + * Created by cir27 on 17/08/17. + */ +public class DisconnectionTest { + @Test + public void testServerDisconnection () throws Exception { + MainServerThread serverThread = new MainServerThread(); + ClientToServerThread clientThread = new ClientToServerThread("localhost", 4942); + Thread.sleep(1000); + clientThread.addDisconnectionListener(message -> Assert.assertTrue(message != null)); + serverThread.terminate(); + } +} diff --git a/src/test/java/seng302/visualiser/ClientToServerTests/RegularPacketsTest.java b/src/test/java/seng302/visualiser/ClientToServerTests/RegularPacketsTest.java index 70e47e79..f3293606 100644 --- a/src/test/java/seng302/visualiser/ClientToServerTests/RegularPacketsTest.java +++ b/src/test/java/seng302/visualiser/ClientToServerTests/RegularPacketsTest.java @@ -61,7 +61,7 @@ public class RegularPacketsTest { // SleepThreadMaxDelay(); // ServerYacht yacht = new ArrayList<>(GameState.getYachts().values()).get(0); // boolean startState = yacht.getSailIn(); -// clientThread.sendBoatActionMessage(BoatAction.SAILS_IN); +// clientThread.sendBoatAction(BoatAction.SAILS_IN); // SleepThreadMaxDelay(); // Assert.assertEquals(startState, !yacht.getSailIn()); // } diff --git a/src/test/java/steps/ToggleSailSteps.java b/src/test/java/steps/ToggleSailSteps.java index cd4caeea..e4c6abed 100644 --- a/src/test/java/steps/ToggleSailSteps.java +++ b/src/test/java/steps/ToggleSailSteps.java @@ -36,7 +36,7 @@ public class ToggleSailSteps { public void the_user_has_pressed(String arg1) throws Throwable { startTime = System.currentTimeMillis(); if (arg1 == "shift") { - client.sendBoatActionMessage(BoatAction.SAILS_IN); + client.sendBoatAction(BoatAction.SAILS_IN); } }