Cleaned up connection handling code so that it deals more robustly and

gracefully with failures; also no longer attempt to send messages to a
connection that has already been closed.


git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@1565 542714f4-19e9-0310-aa3c-eee0fc999fb1

src/java/com/threerings/presents/server/net/Connection.java

@@ -1,5 +1,5 @@
 //
-// $Id: Connection.java,v 1.6 2001/10/11 04:07:53 mdb Exp $
+// $Id: Connection.java,v 1.7 2002/07/10 01:23:45 mdb Exp $
 
 package com.threerings.presents.server.net;
 
@@ -81,6 +81,14 @@ public abstract class Connection implements NetEventHandler
         return _socket;
     }
 
+    /**
+     * Returns true if this connection is closed.
+     */
+    public boolean isClosed ()
+    {
+        return (_socket == null);
+    }
+
     /**
      * Closes this connection and unregisters it from the connection
      * manager. This should only be called from the conmgr thread.
@@ -88,8 +96,9 @@ public abstract class Connection implements NetEventHandler
     public void close ()
     {
         // we shouldn't be closed twice
-        if (_socket == null) {
+        if (isClosed()) {
-            Log.warning("Attempted to re-close connection.");
+            Log.warning("Attempted to re-close connection " + this + ".");
+            Thread.dumpStack();
             return;
         }
 
@@ -97,15 +106,48 @@ public abstract class Connection implements NetEventHandler
         _cmgr.connectionClosed(this);
 
         // close our socket
-        try {
+        closeSocket();
-            _socket.close();
+    }
-        } catch (IOException ioe) {
+
-            Log.warning("Error closing connection [conn=" + this +
+    /**
-                        ", error=" + ioe + "].");
+     * Called when there is a failure reading or writing on this
+     * connection. We notify the connection manager and close ourselves
+     * down.
+     */
+    public void handleFailure (IOException ioe)
+    {
+        // if we're already closed, then something is seriously funny
+        if (isClosed()) {
+            Log.warning("Failure reported on closed connection " + this + ".");
+            Thread.dumpStack();
+            return;
         }
 
-        // clear out our socket reference to prevent repeat closings
+        // let the connection manager know we're hosed
-        _socket = null;
+        _cmgr.connectionFailed(this, ioe);
+
+        // and close our socket
+        closeSocket();
+    }
+
+    /**
+     * Closes the socket associated with this connection. This happens
+     * when we receive EOF, are requested to close down or when our
+     * connection fails.
+     */
+    protected void closeSocket ()
+    {
+        if (_socket != null) {
+            try {
+                _socket.close();
+            } catch (IOException ioe) {
+                Log.warning("Error closing connection [conn=" + this +
+                            ", error=" + ioe + "].");
+            }
+
+            // clear out our socket reference to prevent repeat closings
+            _socket = null;
+        }
     }
 
     /**
@@ -122,15 +164,14 @@ public abstract class Connection implements NetEventHandler
             }
 
         } catch (EOFException eofe) {
-            // let the connection manager know that we done went away
+            // close down the socket gracefully
-            _cmgr.connectionClosed(this);
+            close();
 
         } catch (IOException ioe) {
             Log.warning("Error reading message from socket " +
                         "[socket=" + _socket + ", error=" + ioe + "].");
-            Log.logStackTrace(ioe);
+            // deal with the failure
-            // let the connection manager know that something when awry
+            handleFailure(ioe);
-            _cmgr.connectionFailed(this, ioe);
         }
     }
 

src/java/com/threerings/presents/server/net/ConnectionManager.java

@@ -1,5 +1,5 @@
 //
-// $Id: ConnectionManager.java,v 1.18 2002/03/28 22:32:32 mdb Exp $
+// $Id: ConnectionManager.java,v 1.19 2002/07/10 01:23:45 mdb Exp $
 
 package com.threerings.presents.server.net;
 
@@ -173,13 +173,27 @@ public class ConnectionManager extends LoopingThread
         // close any connections that have been queued up to die
         Connection dconn;
         while ((dconn = (Connection)_deathq.getNonBlocking()) != null) {
-            dconn.close();
+            // it's possible that we caught an EOF trying to read from
+            // this connection even after it was queued up for death, so
+            // let's avoid trying to close it twice
+            if (!dconn.isClosed()) {
+                dconn.close();
+            }
         }
 
         // send any messages that are waiting on the outgoing queue
         Tuple tup;
         while ((tup = (Tuple)_outq.getNonBlocking()) != null) {
             Connection conn = (Connection)tup.left;
+            // if the connection to which this message is destined is
+            // closed, drop the message and move along quietly; this is
+            // perfectly legal, a user can logoff whenever they like, even
+            // if we still have things to tell them; such is life in a
+            // fully asynchronous distributed system
+            if (conn.isClosed()) {
+                continue;
+            }
+
             DownstreamMessage outmsg = (DownstreamMessage)tup.right;
             try {
                 // first flatten the message (and frame it)
@@ -188,7 +202,8 @@ public class ConnectionManager extends LoopingThread
                 _framer.writeFrameAndReset(conn.getOutputStream());
 
             } catch (IOException ioe) {
-                connectionFailed(conn, ioe);
+                // instruct the connection to deal with its failure
+                conn.handleFailure(ioe);
             }
         }