From 7d54db4a95cc2b5b8279d45f68e466c2a2699018 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Sat, 1 Jul 2006 03:29:19 +0000 Subject: [PATCH] Cleaned things up, fixed a years old bug. Now we will not get hosed if client resolution fails for whatever reason and we will properly track our pending resolutions (not leaving them dangling if resolution failed) and do so more elegantly (by simply acting as a ClientResolutionObserver ourselves). git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@4239 542714f4-19e9-0310-aa3c-eee0fc999fb1 --- .../presents/server/ClientManager.java | 58 ++++++++----------- .../presents/server/ClientResolver.java | 31 +++++----- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/java/com/threerings/presents/server/ClientManager.java b/src/java/com/threerings/presents/server/ClientManager.java index 4c49aa06d..e6a33cbfc 100644 --- a/src/java/com/threerings/presents/server/ClientManager.java +++ b/src/java/com/threerings/presents/server/ClientManager.java @@ -51,8 +51,8 @@ import com.threerings.presents.server.net.*; * boot for application-defined reasons). */ public class ClientManager - implements ConnectionObserver, PresentsServer.Reporter, - PresentsServer.Shutdowner + implements ConnectionObserver, ClientResolutionListener, + PresentsServer.Reporter, PresentsServer.Shutdowner { /** * Used by {@link #applyToClient}. @@ -134,7 +134,7 @@ public class ClientManager */ public int getOutstandingResolutionCount () { - return _outstandingResolutions; + return _penders.size(); } /** @@ -207,21 +207,13 @@ public class ClientManager // client object, populate it and notify the listeners clr = _factory.createClientResolver(username); clr.init(username); - clr.addResolutionListener(new ClientResolutionListener() { - public void clientResolved (Name username, ClientObject clobj) { - listener.clientResolved(username, clobj); - _outstandingResolutions--; - } - public void resolutionFailed (Name username, Exception cause) { - listener.resolutionFailed(username, cause); - _outstandingResolutions--; - } - }); + clr.addResolutionListener(this); + clr.addResolutionListener(listener); + _penders.put(username, clr); - // request that the appropriate client object be created by - // the dobject manager which starts the whole business off + // request that the appropriate client object be created by the + // dobject manager which starts the whole business off PresentsServer.omgr.createObject(clr.getClientObjectClass(), clr); - _outstandingResolutions++; } catch (Exception e) { // let the listener know that we're hosed @@ -229,20 +221,6 @@ public class ClientManager } } - /** - * Called by the {@link ClientResolver} once a client object has been - * resolved. - */ - protected synchronized void mapClientObject ( - Name username, ClientObject clobj) - { - // stuff the object into the mapping table - _objmap.put(username, clobj); - - // and remove the resolution listener - _penders.remove(username); - } - /** * Releases a client object that was obtained via a call to {@link * #resolveClientObject}. If this caller is the last reference, the @@ -273,6 +251,23 @@ public class ClientManager PresentsServer.omgr.destroyObject(clobj.getOid()); } + // documentation inherited from interface ClientResolutionListener + public synchronized void clientResolved (Name username, ClientObject clobj) + { + // stuff the object into the mapping table + _objmap.put(username, clobj); + + // and remove the resolution listener + _penders.remove(username); + } + + // documentation inherited from interface ClientResolutionListener + public synchronized void resolutionFailed (Name username, Exception reason) + { + // clear out their pending record + _penders.remove(username); + } + // documentation inherited public synchronized void connectionEstablished ( Connection conn, AuthRequest req, AuthResponse rsp) @@ -481,9 +476,6 @@ public class ClientManager /** The client class in use. */ protected ClientFactory _factory = ClientFactory.DEFAULT; - /** A count of how many client objects are currently being resolved. */ - protected int _outstandingResolutions; - /** The frequency with which we check for expired clients. */ protected static final long CLIENT_FLUSH_INTERVAL = 60 * 1000L; } diff --git a/src/java/com/threerings/presents/server/ClientResolver.java b/src/java/com/threerings/presents/server/ClientResolver.java index 5b5ad9bdb..ce957b42a 100644 --- a/src/java/com/threerings/presents/server/ClientResolver.java +++ b/src/java/com/threerings/presents/server/ClientResolver.java @@ -105,21 +105,17 @@ public class ClientResolver extends Invoker.Unit // documentation inherited public void handleResult () { - // if we failed in invoke() report it here - if (_failure != null) { - // destroy the dangling user object - PresentsServer.omgr.destroyObject(_clobj.getOid()); - - // let our listener know that we're hosed - reportFailure(_failure); - - } else { - // otherwise, do more resolution goodness on the dobj thread - finishResolution(_clobj); - - // let the client manager know that we're all clear - PresentsServer.clmgr.mapClientObject(_username, _clobj); + // if we haven't failed, finish resolution on the dobj thread + if (_failure == null) { + try { + finishResolution(_clobj); + } catch (Exception e) { + _failure = e; + } + } + // if we still haven't failed, then we're good to go + if (_failure == null) { // and let the listeners in on the secret as well for (int ii = 0, ll = _listeners.size(); ii < ll; ii++) { ClientResolutionListener crl = _listeners.get(ii); @@ -134,6 +130,13 @@ public class ClientResolver extends Invoker.Unit Log.logStackTrace(e); } } + + } else { + // destroy the dangling user object + PresentsServer.omgr.destroyObject(_clobj.getOid()); + + // let our listener know that we're hosed + reportFailure(_failure); } }