From 3bbe3a63303b9a3c9e108d87446d73a40ccae4dd Mon Sep 17 00:00:00 2001
From: "Ray J. Greenwell" <ray@threerings.net>
Date: Sat, 26 Oct 2013 18:33:53 -0700
Subject: [PATCH] Suppress logging of common peer condition.

This has been happening for a while on PX and I've been meaning to track
it down. When a peer shuts down an object that's proxied on other peers,
the other peers receive an ObjectDestroyedEvent, and clear the object
from their omgrs. Other listeners may also receive the event and will need
to clear the proxying information, but that causes an unsubscribe to be
queued up, which when it runs will find the DObject already gone, and log
an error.

At first I considered not queuing up the unsubscribe action if the object
is no longer present in the omgr, but in the case that I'm working with
we have 2 or more objects that will all be destroyed simultaneously.
Even if I test to see if the first object is around or not, this same
test would always pass for the second object even though its death event
is next on the queue. The unsubscribe would be enqueued after THAT, so
there's no test to detect that the two objects are linked.

Instead, just suppress this message if we're a peer client. I didn't
want to suppress it altogether because we might want this message logged for
normal client connections. Maybe not.

Perhaps there is a better way. Maybe the PeerNode should add its own
ObjectDeathListener and clear out the proxy information and ensure that
the proxied object is also destoyed, and clients need not unproxy manually
in that case.

RFC.
---
 .../threerings/presents/client/Client.java    | 21 +++++++++++++++++++
 .../presents/client/ClientDObjectMgr.java     |  2 +-
 .../presents/peer/server/PeerNode.java        |  4 ++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/com/threerings/presents/client/Client.java b/core/src/main/java/com/threerings/presents/client/Client.java
index 4c875735b..1c4ea7b27 100644
--- a/core/src/main/java/com/threerings/presents/client/Client.java
+++ b/core/src/main/java/com/threerings/presents/client/Client.java
@@ -971,6 +971,27 @@ public class Client
         return false;
     }
 
+    /**
+     * Should we be logging the specified failure?
+     */
+    protected boolean isFailureLoggable (FailureType type)
+    {
+        return true;
+    }
+
+    /**
+     * The types of failures that may occur in our associated classes, which may call
+     * isFailureLoggable() for use to arbitrate whether we want to log the error.
+     */
+    protected enum FailureType
+    {
+        /** An attempt was made to unsubscribe from an object that is not proxied. */
+        UNSUBSCRIBE_NOT_PROXIED,
+
+        // TODO: fill out more
+        ;
+    }
+
     /** Handles the process of switching between servers. See {@link Client#moveToServer}. */
     protected class ServerSwitcher extends ClientAdapter
     {
diff --git a/core/src/main/java/com/threerings/presents/client/ClientDObjectMgr.java b/core/src/main/java/com/threerings/presents/client/ClientDObjectMgr.java
index 089043813..b8b294f43 100644
--- a/core/src/main/java/com/threerings/presents/client/ClientDObjectMgr.java
+++ b/core/src/main/java/com/threerings/presents/client/ClientDObjectMgr.java
@@ -442,7 +442,7 @@ public class ClientDObjectMgr
         if (dobj != null) {
             dobj.removeSubscriber(target);
 
-        } else {
+        } else if (_client.isFailureLoggable(Client.FailureType.UNSUBSCRIBE_NOT_PROXIED)) {
             log.info("Requested to remove subscriber from non-proxied object", "oid", oid,
                      "sub", target);
         }
diff --git a/core/src/main/java/com/threerings/presents/peer/server/PeerNode.java b/core/src/main/java/com/threerings/presents/peer/server/PeerNode.java
index 4f38879ac..0a062e1a8 100644
--- a/core/src/main/java/com/threerings/presents/peer/server/PeerNode.java
+++ b/core/src/main/java/com/threerings/presents/peer/server/PeerNode.java
@@ -80,6 +80,10 @@ public class PeerNode
             @Override protected Communicator createCommunicator () {
                 return PeerNode.this.createCommunicator(this);
             }
+            @Override protected boolean isFailureLoggable (FailureType type) {
+                return (type != FailureType.UNSUBSCRIBE_NOT_PROXIED) &&
+                        super.isFailureLoggable(type);
+            }
         };
         _client.addClientObserver(this);
     }