diff --git a/src/java/com/threerings/presents/Log.java b/src/java/com/threerings/presents/Log.java index f610d42e3..909895ade 100644 --- a/src/java/com/threerings/presents/Log.java +++ b/src/java/com/threerings/presents/Log.java @@ -30,8 +30,7 @@ import java.util.logging.Logger; public class Log { /** We dispatch our log messages through this logger. */ - public static Logger log = - Logger.getLogger("com.threerings.narya.presents"); + public static Logger log = Logger.getLogger("com.threerings.narya.presents"); /** Convenience function. */ public static void debug (String message) diff --git a/src/java/com/threerings/presents/server/ClientManager.java b/src/java/com/threerings/presents/server/ClientManager.java index 4181f60d2..ff6c369c7 100644 --- a/src/java/com/threerings/presents/server/ClientManager.java +++ b/src/java/com/threerings/presents/server/ClientManager.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.logging.Level; import com.samskivert.util.Interval; import com.samskivert.util.ObserverList; @@ -33,13 +34,14 @@ import com.samskivert.util.StringUtil; import com.threerings.util.Name; -import com.threerings.presents.Log; import com.threerings.presents.data.ClientObject; import com.threerings.presents.net.AuthRequest; import com.threerings.presents.net.AuthResponse; import com.threerings.presents.net.Credentials; import com.threerings.presents.server.net.*; +import static com.threerings.presents.Log.log; + /** * The client manager is responsible for managing the clients (surprise, surprise) which are * slightly more than just connections. Clients persist in the absence of connections in case a @@ -110,17 +112,17 @@ public class ClientManager // documentation inherited from interface public void shutdown () { - Log.info("Client manager shutting down [ccount=" + _usermap.size() + "]."); + log.info("Client manager shutting down [ccount=" + _usermap.size() + "]."); // inform all of our clients that they are being shut down - for (Iterator iter = _usermap.values().iterator(); iter.hasNext(); ) { - PresentsClient pc = (PresentsClient)iter.next(); - try { - pc.shutdown(); - } catch (Exception e) { - Log.warning( - "Client choked in shutdown() [client=" + StringUtil.safeToString(pc) + "]."); - Log.logStackTrace(e); + synchronized (_usermap) { + for (PresentsClient pc : _usermap.values()) { + try { + pc.shutdown(); + } catch (Exception e) { + log.log(Level.WARNING, "Client choked in shutdown() [client=" + + StringUtil.safeToString(pc) + "].", e); + } } } } @@ -155,7 +157,9 @@ public class ClientManager */ public int getClientCount () { - return _usermap.size(); + synchronized (_usermap) { + return _usermap.size(); + } } /** @@ -196,7 +200,9 @@ public class ClientManager */ public PresentsClient getClient (Name authUsername) { - return _usermap.get(authUsername); + synchronized (_usermap) { + return _usermap.get(authUsername); + } } /** @@ -269,7 +275,7 @@ public class ClientManager { ClientObject clobj = _objmap.get(username); if (clobj == null) { - Log.warning("Requested to release unmapped client object [username=" + username + "]."); + log.warning("Requested to release unmapped client object [username=" + username + "]."); Thread.dumpStack(); return; } @@ -279,7 +285,7 @@ public class ClientManager return; } - Log.debug("Destroying client " + clobj.who() + "."); + log.fine("Destroying client " + clobj.who() + "."); // we're all clear to go; remove the mapping _objmap.remove(username); @@ -311,7 +317,7 @@ public class ClientManager _penders.remove(username); } - // documentation inherited + // from interface ConnectionObserver public synchronized void connectionEstablished ( Connection conn, AuthRequest req, AuthResponse rsp) { @@ -322,30 +328,32 @@ public class ClientManager PresentsClient client = getClient(username); if (client != null) { - Log.info("Resuming session [username=" + username + ", conn=" + conn + "]."); + log.info("Resuming session [username=" + username + ", conn=" + conn + "]."); client.resumeSession(req, conn); } else { - Log.info("Session initiated [username=" + username + ", conn=" + conn + "]."); + log.info("Session initiated [username=" + username + ", conn=" + conn + "]."); // create a new client and stick'em in the table client = _factory.createClient(req); client.startSession(this, req, conn, rsp.authdata); // map their client instance - _usermap.put(username, client); + synchronized (_usermap) { + _usermap.put(username, client); + } } // map this connection to this client _conmap.put(conn, client); } - // documentation inherited + // from interface ConnectionObserver public synchronized void connectionFailed (Connection conn, IOException fault) { // remove the client from the connection map PresentsClient client = _conmap.remove(conn); if (client != null) { - Log.info("Unmapped failed client [client=" + client + ", conn=" + conn + + log.info("Unmapped failed client [client=" + client + ", conn=" + conn + ", fault=" + fault + "]."); // let the client know the connection went away client.wasUnmapped(); @@ -353,23 +361,23 @@ public class ClientManager client.connectionFailed(fault); } else if (!(conn instanceof AuthingConnection)) { - Log.info("Unmapped connection failed? [conn=" + conn + ", fault=" + fault + "]."); + log.info("Unmapped connection failed? [conn=" + conn + ", fault=" + fault + "]."); Thread.dumpStack(); } } - // documentation inherited + // from interface ConnectionObserver public synchronized void connectionClosed (Connection conn) { // remove the client from the connection map PresentsClient client = _conmap.remove(conn); if (client != null) { - Log.debug("Unmapped client [client=" + client + ", conn=" + conn + "]."); + log.fine("Unmapped client [client=" + client + ", conn=" + conn + "]."); // let the client know the connection went away client.wasUnmapped(); } else { - Log.info("Closed unmapped connection '" + conn + "'. " + + log.info("Closed unmapped connection '" + conn + "'. " + "Client probably not yet authenticated."); } } @@ -379,7 +387,9 @@ public class ClientManager { report.append("* presents.ClientManager:\n"); report.append("- Sessions: "); - report.append(_usermap.size()).append(" total, "); + synchronized (_usermap) { + report.append(_usermap.size()).append(" total, "); + } report.append(_conmap.size()).append(" connected, "); report.append(_penders.size()).append(" pending\n"); report.append("- Mapped users: ").append(_objmap.size()).append("\n"); @@ -407,17 +417,20 @@ public class ClientManager protected void clientSessionDidEnd (final PresentsClient client) { // remove the client from the username map - PresentsClient rc = _usermap.remove(client.getCredentials().getUsername()); + PresentsClient rc; + synchronized (_usermap) { + rc = _usermap.remove(client.getCredentials().getUsername()); + } // sanity check just because we can if (rc == null) { - Log.warning("Unregistered client ended session " + client + "."); + log.warning("Unregistered client ended session " + client + "."); Thread.dumpStack(); } else if (rc != client) { - Log.warning("Different clients with same username!? " + + log.warning("Different clients with same username!? " + "[c1=" + rc + ", c2=" + client + "]."); } else { - Log.info("Ending session " + client + "."); + log.info("Ending session " + client + "."); } // notify the observers that the session is ended @@ -435,30 +448,26 @@ public class ClientManager */ protected void flushClients () { - ArrayList victims = null; + ArrayList victims = new ArrayList(); long now = System.currentTimeMillis(); - // first build a list of our victims (we can't flush clients directly while iterating due - // to risk of a ConcurrentModificationException) - for (PresentsClient client : _usermap.values()) { - if (client.checkExpired(now)) { - if (victims == null) { - victims = new ArrayList(); + // first build a list of our victims + synchronized (_usermap) { + for (PresentsClient client : _usermap.values()) { + if (client.checkExpired(now)) { + victims.add(client); } - victims.add(client); } } - if (victims != null) { - for (PresentsClient client : victims) { - try { - Log.info("Client expired, ending session [client=" + client + - ", dtime=" + (now-client.getNetworkStamp()) + "ms]."); - client.endSession(); - } catch (Exception e) { - Log.warning("Choke while flushing client [victim=" + client + "]."); - Log.logStackTrace(e); - } + // now end their sessions + for (PresentsClient client : victims) { + try { + log.info("Client expired, ending session [client=" + client + + ", dtime=" + (now-client.getNetworkStamp()) + "ms]."); + client.endSession(); + } catch (Exception e) { + log.log(Level.WARNING, "Choke while flushing client [victim=" + client + "].", e); } } } @@ -476,8 +485,8 @@ public class ClientManager _clop.apply(clobj); } catch (Exception e) { - Log.warning("Client op failed [username=" + username + ", clop=" + _clop + "]."); - Log.logStackTrace(e); + log.log(Level.WARNING, "Client op failed [username=" + username + + ", clop=" + _clop + "].", e); } finally { releaseClientObject(username); diff --git a/src/java/com/threerings/presents/server/net/Connection.java b/src/java/com/threerings/presents/server/net/Connection.java index 2eb2b5f12..c2953ac6d 100644 --- a/src/java/com/threerings/presents/server/net/Connection.java +++ b/src/java/com/threerings/presents/server/net/Connection.java @@ -28,6 +28,8 @@ import java.net.InetAddress; import java.nio.channels.SelectionKey; import java.nio.channels.SocketChannel; +import com.samskivert.util.StringUtil; + import com.threerings.io.FramedInputStream; import com.threerings.io.FramingOutputStream; import com.threerings.io.ObjectInputStream; @@ -39,27 +41,22 @@ import com.threerings.presents.net.PingRequest; import com.threerings.presents.net.UpstreamMessage; /** - * The base connection class implements the net event handler interface - * and processes raw incoming network data into a stream of parsed - * UpstreamMessage objects. It also provides the means to - * send messages to the client and facilities for checking delinquency. + * The base connection class implements the net event handler interface and processes raw incoming + * network data into a stream of parsed UpstreamMessage objects. It also provides the + * means to send messages to the client and facilities for checking delinquency. */ public abstract class Connection implements NetEventHandler { /** - * Constructs a connection object that is associated with the supplied - * socket. + * Constructs a connection object that is associated with the supplied socket. * - * @param cmgr The connection manager with which this connection is - * associated. - * @param selkey the key used by the NIO code to track this - * connection. - * @param channel The socket channel from which we'll be reading - * messages. + * @param cmgr The connection manager with which this connection is associated. + * @param selkey the key used by the NIO code to track this connection. + * @param channel The socket channel from which we'll be reading messages. * @param createStamp The time at which this connection was created. */ - public Connection (ConnectionManager cmgr, SelectionKey selkey, - SocketChannel channel, long createStamp) + public Connection (ConnectionManager cmgr, SelectionKey selkey, SocketChannel channel, + long createStamp) throws IOException { _cmgr = cmgr; @@ -69,9 +66,8 @@ public abstract class Connection implements NetEventHandler } /** - * Instructs the connection to pass parsed messages on to this handler - * for processing. This should be done before the connection is turned - * loose to process messages. + * Instructs the connection to pass parsed messages on to this handler for processing. This + * should be done before the connection is turned loose to process messages. */ public void setMessageHandler (MessageHandler handler) { @@ -98,8 +94,7 @@ public abstract class Connection implements NetEventHandler } /** - * Returns the non-blocking socket object used to construct this - * connection. + * Returns the non-blocking socket object used to construct this connection. */ public SocketChannel getChannel () { @@ -107,8 +102,8 @@ public abstract class Connection implements NetEventHandler } /** - * Returns the address associated with this connection or null if it - * has no underlying socket channel. + * Returns the address associated with this connection or null if it has no underlying socket + * channel. */ public InetAddress getInetAddress () { @@ -124,8 +119,8 @@ public abstract class Connection implements NetEventHandler } /** - * Closes this connection and unregisters it from the connection - * manager. This should only be called from the conmgr thread. + * Closes this connection and unregisters it from the connection manager. This should only be + * called from the conmgr thread. */ public void close () { @@ -144,9 +139,8 @@ public abstract class Connection implements NetEventHandler } /** - * Called when there is a failure reading or writing on this - * connection. We notify the connection manager and close ourselves - * down. + * 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) { @@ -165,8 +159,8 @@ public abstract class Connection implements NetEventHandler } /** - * Returns the object input stream associated with this connection. - * This should only be used by the connection manager. + * Returns the object input stream associated with this connection. This should only be used + * by the connection manager. */ protected ObjectInputStream getObjectInputStream () { @@ -174,10 +168,9 @@ public abstract class Connection implements NetEventHandler } /** - * Instructs this connection to inherit its streams from the supplied - * connection object. This is called by the connection manager when - * the time comes to pass streams from the authing connection to the - * running connection. + * Instructs this connection to inherit its streams from the supplied connection object. This + * is called by the connection manager when the time comes to pass streams from the authing + * connection to the running connection. */ protected void inheritStreams (Connection other) { @@ -190,16 +183,13 @@ public abstract class Connection implements NetEventHandler } /** - * Returns the object output stream associated with this connection - * (creating it if necessary). This should only be used by the - * connection manager. + * Returns the object output stream associated with this connection (creating it if + * necessary). This should only be used by the connection manager. */ - protected ObjectOutputStream getObjectOutputStream ( - FramingOutputStream fout) + protected ObjectOutputStream getObjectOutputStream (FramingOutputStream fout) { - // we're lazy about creating our output stream because we may be - // inheriting it from our authing connection and we don't want to - // unnecessarily create it in that case + // we're lazy about creating our output stream because we may be inheriting it from our + // authing connection and we don't want to unnecessarily create it in that case if (_oout == null) { _oout = new ObjectOutputStream(fout); } @@ -207,8 +197,8 @@ public abstract class Connection implements NetEventHandler } /** - * Sets the object output stream used by this connection. This should - * only be called by the connection manager. + * Sets the object output stream used by this connection. This should only be called by the + * connection manager. */ protected void setObjectOutputStream (ObjectOutputStream oout) { @@ -216,26 +206,25 @@ public abstract class Connection implements NetEventHandler } /** - * Closes the socket associated with this connection. This happens - * when we receive EOF, are requested to close down or when our - * connection fails. + * 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 (_channel != null) { - Log.debug("Closing channel " + this + "."); - - try { - _channel.close(); - } catch (IOException ioe) { - Log.warning("Error closing connection [conn=" + this + - ", error=" + ioe + "]."); - } - - // clear out our references to prevent repeat closings - _selkey = null; - _channel = null; + if (_channel == null) { + return; } + + Log.debug("Closing channel " + this + "."); + try { + _channel.close(); + } catch (IOException ioe) { + Log.warning("Error closing connection [conn=" + this + ", error=" + ioe + "]."); + } + + // clear out our references to prevent repeat closings + _selkey = null; + _channel = null; } /** @@ -248,9 +237,8 @@ public abstract class Connection implements NetEventHandler int bytesIn = 0; try { - // we're lazy about creating our input streams because we may - // be inheriting them from our authing connection and we don't - // want to unnecessarily create them in that case + // we're lazy about creating our input streams because we may be inheriting them from + // our authing connection and we don't want to unnecessarily create them in that case if (_fin == null) { _fin = new FramedInputStream(); _oin = new ObjectInputStream(_fin); @@ -259,12 +247,11 @@ public abstract class Connection implements NetEventHandler } } - // there may be more than one frame in the buffer, so we keep - // reading them until we run out of data + // there may be more than one frame in the buffer, so we keep reading them until we run + // out of data while (_fin.readFrame(_channel)) { - // make a note of how many bytes are in this frame - // (including the frame length bytes which aren't reported - // in available()) + // make a note of how many bytes are in this frame (including the frame length + // bytes which aren't reported in available()) bytesIn = _fin.available() + 4; // parse the message and pass it on UpstreamMessage msg = (UpstreamMessage)_oin.readObject(); @@ -278,30 +265,18 @@ public abstract class Connection implements NetEventHandler close(); } catch (ClassNotFoundException cnfe) { - try { - Log.warning("Error reading message from socket " + - "[channel=" + _channel + ", error=" + cnfe + "]."); - } catch (Error err) { - Log.warning("Error reading message from socket " + - "and error printing channel [error=" + cnfe + "]."); - } + Log.warning("Error reading message from socket [channel=" + + StringUtil.safeToString(_channel) + ", error=" + cnfe + "]."); // deal with the failure - handleFailure((IOException) new IOException( - "Unable to decode incoming message.").initCause(cnfe)); + String errmsg = "Unable to decode incoming message."; + handleFailure((IOException) new IOException(errmsg).initCause(cnfe)); } catch (IOException ioe) { - // don't log a warning for the ever-popular "the client - // dropped the connection" failure + // don't log a warning for the ever-popular "the client dropped the connection" failure String msg = ioe.getMessage(); if (msg == null || msg.indexOf("reset by peer") == -1) { - try { - Log.warning("Error reading message from socket " + - "[channel=" + _channel + - ", error=" + ioe + "]."); - } catch (Error err) { - Log.warning("Error reading message from socket " + - "and error printing channel [error=" + ioe + "]."); - } + Log.warning("Error reading message from socket [channel=" + + StringUtil.safeToString(_channel) + ", error=" + ioe + "]."); } // deal with the failure handleFailure(ioe); @@ -326,9 +301,8 @@ public abstract class Connection implements NetEventHandler } /** - * Posts a downstream message for delivery to this connection. The - * message will be delivered by the conmgr thread as soon as it gets - * to it. + * Posts a downstream message for delivery to this connection. The message will be delivered by + * the conmgr thread as soon as it gets to it. */ public void postMessage (DownstreamMessage msg) { @@ -349,8 +323,7 @@ public abstract class Connection implements NetEventHandler protected MessageHandler _handler; protected ClassLoader _loader; - /** The number of milliseconds beyond the ping interval that we allow - * a client's network connection to be idle before we forcibly - * disconnect them. */ + /** The number of milliseconds beyond the ping interval that we allow a client's network + * connection to be idle before we forcibly disconnect them. */ protected static final long LATENCY_GRACE = 30 * 1000L; }