From 5b6c9a71e5ce8ff7ffae4c9741b4ab66c80637ef Mon Sep 17 00:00:00 2001
From: Mike Thomas <mthomas@threerings.net>
Date: Thu, 3 Feb 2011 19:28:34 +0000
Subject: [PATCH] Fix a bit of a memory leak wherein if a DObject happened to
 be destroyed prior to its clients unsubscribing, that DObject would be kept
 until the sessions still subscribed to that object logged off.  Due to
 various client-side timing bits this could happen even if the server sends a
 message to clients that causes them to unsubscribe prior to calling
 destroyObject.

A concrete example of this is on PlaceObjects - If the server tells users to leave a place and then calls shutdown on the PlaceManager, thus destroying the object, the client will receive the leave-place message, queue up an unsubscribe, receive the destroy message, remove the object from their object cache, and then be unable to unsubscribe back to the server due to no longer having the DObject in question.

Now, if a PresentsSession received a destroy event for an object its client is subscribed to, it will delete the subscription, and instead note the oid as a destroyed subscription for use in validation if lingering unsubscribe requests are pending (they can frequently be).  This still potentially leaks the oid int until the session is ended, but the DObject can be collected.



git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@6475 542714f4-19e9-0310-aa3c-eee0fc999fb1
---
 .../presents/server/PresentsSession.java      | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/main/java/com/threerings/presents/server/PresentsSession.java b/src/main/java/com/threerings/presents/server/PresentsSession.java
index e53a3625b..1c3f4401d 100644
--- a/src/main/java/com/threerings/presents/server/PresentsSession.java
+++ b/src/main/java/com/threerings/presents/server/PresentsSession.java
@@ -23,6 +23,7 @@ package com.threerings.presents.server;
 
 import java.net.InetAddress;
 
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.TimeZone;
@@ -31,6 +32,7 @@ import java.io.IOException;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 
 import com.samskivert.util.IntMap;
@@ -47,6 +49,7 @@ import com.threerings.presents.data.ClientObject;
 import com.threerings.presents.dobj.DEvent;
 import com.threerings.presents.dobj.DObject;
 import com.threerings.presents.dobj.ObjectAccessException;
+import com.threerings.presents.dobj.ObjectDestroyedEvent;
 import com.threerings.presents.dobj.ProxySubscriber;
 import com.threerings.presents.net.AuthRequest;
 import com.threerings.presents.net.BootstrapData;
@@ -611,7 +614,10 @@ public class PresentsSession
         if (rec != null) {
             rec.unsubscribe();
         } else {
-            log.warning("Missing subscription in unmap", "client", this, "oid", oid);
+            boolean alreadyDestroyed = _destroyedSubs.remove(oid);
+            if (!alreadyDestroyed) {
+                log.warning("Missing subscription in unmap", "client", this, "oid", oid);
+            }
         }
     }
 
@@ -1022,6 +1028,27 @@ public class PresentsSession
             }
 
             postMessage(new EventNotification(event), _oconn);
+
+            if (event instanceof ObjectDestroyedEvent) {
+                // Make sure it's cleared out.  Otherwise, client-server timing can
+                // be such that a client stays subscribed to a no-longer-managed dobj.
+                // NOTE: We keep the oid itself on the destroyedSubs list to validate against since
+                // we may get a late unsubscribe after the destruction event has gone through. Thus
+                // there is still potentially a memory leak until logoff of the oid, but the dobj
+                // itself can be collected.
+                ClientProxy sub;
+                int oid = object.getOid();
+                synchronized(_subscrips) {
+                    sub = _subscrips.remove(oid);
+                    if (sub != null) {
+                        _destroyedSubs.add(oid);
+                    }
+                }
+
+                if (sub == ClientProxy.this) {
+                    unsubscribe();
+                }
+            }
         }
 
         // from interface ProxySubscriber
@@ -1173,6 +1200,9 @@ public class PresentsSession
     protected PresentsConnection _conn;
     protected ClientObject _clobj;
     protected IntMap<ClientProxy> _subscrips = IntMaps.newHashIntMap();
+
+    /** The Oids of objects that have been destroyed while we were subscribed. */
+    protected HashSet<Integer> _destroyedSubs = Sets.newHashSet();
     protected ClassLoader _loader;
 
     /** The time at which this client started their session. */