fix(network): never send CloseConnection manually

Author: xeruf

helpers/test-client/src/sc/TestClient.java

@@ -9,7 +9,6 @@
 import sc.api.plugins.IGamePlugin;
 import sc.framework.plugins.Player;
 import sc.networking.clients.XStreamClient;
-import sc.protocol.CloseConnection;
 import sc.protocol.ProtocolPacket;
 import sc.protocol.room.RoomPacket;
 import sc.protocol.requests.*;
@@ -338,8 +337,7 @@ private void prepareNewClients() {
 
   private static void exit(int status) {
     if (testclient != null) {
-      if (!testclient.isClosed())
-        testclient.send(new CloseConnection());
+      testclient.stop();
       testclient.waiter.shutdownNow();
     }
 

sdk/src/server-api/sc/networking/clients/XStreamClient.java

@@ -91,7 +91,7 @@ public void run() {
 
   /** Used by the receiving thread. All exceptions should be handled. */
   public void receiveThread() {
-    try(ObjectInputStream in = xStream.createObjectInputStream(networkInterface.getInputStream())) {
+    try (ObjectInputStream in = xStream.createObjectInputStream(networkInterface.getInputStream())) {
       synchronized(readyLock) {
         while (!isReady()) {
           readyLock.wait();
@@ -109,7 +109,7 @@ public void receiveThread() {
 
           if (response instanceof CloseConnection) {
             handleDisconnect(DisconnectCause.RECEIVED_DISCONNECT);
-            // handleDisconnect takes care of stopping the thread
+            break;
           } else {
             onObject(response);
           }
@@ -133,13 +133,13 @@ public void receiveThread() {
         if (exceptionCause instanceof SocketException) {
           // If the thread was interrupted, we have a regular disconnect.
           // Unfortunately, OIS.readObject() doesn't react to interruptions directly.
-          if(!Thread.interrupted())
+          if (!Thread.interrupted())
             handleDisconnect(DisconnectCause.LOST_CONNECTION, e);
         } else if (exceptionCause instanceof EOFException) {
           handleDisconnect(DisconnectCause.LOST_CONNECTION, e);
         } else if (exceptionCause instanceof IOException
-                && exceptionCause.getCause() != null && exceptionCause
-                .getCause() instanceof InterruptedException) {
+            && exceptionCause.getCause() != null && exceptionCause
+            .getCause() instanceof InterruptedException) {
           handleDisconnect(DisconnectCause.LOST_CONNECTION, e);
         } else {
           handleDisconnect(DisconnectCause.PROTOCOL_ERROR, e);
@@ -164,7 +164,11 @@ public void sendCustomData(byte[] data) throws IOException {
     networkInterface.getOutputStream().flush();
   }
 
-  public synchronized void send(ProtocolPacket packet) {
+  public void send(ProtocolPacket packet) {
+    sendObject(packet);
+  }
+
+  protected synchronized void sendObject(Object packet) {
     if (!isReady())
       throw new IllegalStateException(
           String.format("Trying to write packet on %s which wasn't started: %s", shortString(), packet));
@@ -176,7 +180,7 @@ public synchronized void send(ProtocolPacket packet) {
       return;
     }
 
-    logger.debug("{}: Sending {} via {} from {}", shortString(), packet, networkInterface, toString());
+    logger.debug("{}: Sending {} via {} from {}", shortString(), packet, networkInterface, this);
     if (logger.isTraceEnabled())
       logger.trace("Dumping {}:\n{}", packet, xStream.toXML(packet));
 
@@ -231,7 +235,8 @@ public DisconnectCause getDisconnectCause() {
    */
   public void stop() {
     // this side caused disconnect, notify other side
-    send(new CloseConnection());
+    if (!isClosed())
+      send(new CloseConnection());
     handleDisconnect(DisconnectCause.DISCONNECTED);
   }
 

sdk/src/server-api/sc/protocol/CloseConnection.kt

@@ -5,6 +5,9 @@ import com.thoughtworks.xstream.annotations.XStreamAlias
 /**
  * Is sent by one party immediately before this party closes the communication
  * connection and should make the receiving party also close the connection.
+ *
+ * This should not be sent manually, the XStreamClient will automatically send
+ * it when stopped.
  */
 @XStreamAlias(value = "close")
 class CloseConnection: ProtocolPacket {