More DObject fun! We need to keep a careful separation for container

objects (DSet, arrays) in a DEvent (which should not change as a result of
other events being applied) and those in the object itself (which do
change and evolve as events are applied to the object).

This is important both because the DEvent is passed on to another thread
for delivery to remote clients, thus changes to the values in the event
could take place before they were serialized and sent over the network,
and because compound events are applied to an object before they are sent
to the other thread for delivery and thus, for example, setting a DSet and
then adding a few entries to it in a compound event would result in the
DEvent copy of the DSet becoming corrupted.

Two problems remain (note, neither of these are new, the one issue
introduced when I rewrote the DObject stuff is fixed by these checkins):

1. Object subscription requests are supposed to deliver a snapshot of the
   object at the point in the event stream at which the subscription
   request was processed, but presently we pass only a reference to the
   object off to the networking thread which means that before the object
   is serialized and sent to clients, subsequent events could be applied
   to it and then those events would be sent to the client as well
   resulting in funny business (probably nothing more than duplicate DSet
   entry warnings, but imagination and Chapter 17 tell us that worse
   things could happen).

2. The use of Streamable instances could result in badness. If a field in
   a Streamable is modified and the whole Streamable set() back into the
   object to broadcast the update, then further changes were made to the
   Streamable before the attribute change event was serialized and sent
   over the network, the second modifications would be reflected in the
   event triggered by the first modifications.

The first problem may be solvable (albeit inefficiently) by serializing
the DObject on the event dispatcher thread and sending that serialized
copy off to the network thread for delivery to the client. It would be
much less efficient as we would be unable to make use of the client's
already "primed" ObjectOutputStream which may have already mapped many of
the classes in the object to two byte codes, but object subscription is
fairly uncommon compared to delivery of events, so inefficiency might not
be a big problem in this case.

The second problem might be solved by requiring that all Streamable
implementations implement clone() and then cloning any Streamable
attribute just as we do an array or DSet during an attribute, array
element or DSet entry change. This would be a more significant performance
hit as well as require a review of all of our Streamable classes (to
determine if they need a custom clone() implementation), and it has up to
now not actually manifested as a problem.

In any case I'm not going to tackle either of these remedies at the moment
because I'm on vacation, dammit.


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

src/java/com/threerings/presents/dobj/AttributeChangedEvent.java

@@ -21,6 +21,8 @@
 
 package com.threerings.presents.dobj;
 
+import java.util.HashMap;
+
 import com.samskivert.util.StringUtil;
 
 /**
@@ -114,9 +116,24 @@ public class AttributeChangedEvent extends NamedEvent
         // actually apply the attribute change
         if (_oldValue == UNSET_OLD_VALUE) {
             _oldValue = target.getAttribute(_name);
-            // pass the new value on to the object (this uses reflection
-            // and is slow)
-            target.setAttribute(_name, _value);
+            Object value = _value;
+            if (value != null) {
+                Class vclass = value.getClass();
+                if (vclass.isPrimitive()) {
+                    // do nothing; we check this to avoid the more
+                    // expensive isAssignableFrom check on primitives
+                    // which are far and away the most common case
+                } else if (vclass.isArray()) {
+                    Cloner cloner = (Cloner)_cloners.get(vclass);
+                    if (cloner != null) {
+                        value = cloner.clone(value);
+                    }
+                } else if (DSet.class.isAssignableFrom(vclass)) {
+                    value = ((DSet)value).clone();
+                }
+            }
+            // pass the new value on to the object
+            target.setAttribute(_name, value);
         }
         return true;
     }
@@ -160,6 +177,66 @@ public class AttributeChangedEvent extends NamedEvent
         StringUtil.toString(buf, _value);
     }
 
+    /** Used to clone object attributes without a zillion calls to
+     * instanceof. */
+    protected static interface Cloner {
+        public Object clone (Object object);
+    }
+
     protected Object _value;
     protected transient Object _oldValue = UNSET_OLD_VALUE;
+
+    /** Contains {@link Cloner}s for types that need cloning. */
+    protected static HashMap _cloners = new HashMap();
+
+    /** Object prototypes we can use to get the appropriate class objects
+     * for use when setting up {@link #_cloners}. */
+    protected static final Object[] ARRAY_PROTOS = new Object[] {
+        new String[0], new byte[0], new char[0], new short[0], new int[0],
+        new long[0], new float[0], new double[0]
+    };
+
+    // set up our cloners
+    static {
+        _cloners.put(ARRAY_PROTOS[0], new Cloner() {
+            public Object clone (Object obj) {
+                return ((String[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[1], new Cloner() {
+            public Object clone (Object obj) {
+                return ((byte[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[2], new Cloner() {
+            public Object clone (Object obj) {
+                return ((char[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[3], new Cloner() {
+            public Object clone (Object obj) {
+                return ((short[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[4], new Cloner() {
+            public Object clone (Object obj) {
+                return ((int[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[5], new Cloner() {
+            public Object clone (Object obj) {
+                return ((long[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[6], new Cloner() {
+            public Object clone (Object obj) {
+                return ((float[])obj).clone();
+            }
+        });
+        _cloners.put(ARRAY_PROTOS[7], new Cloner() {
+            public Object clone (Object obj) {
+                return ((double[])obj).clone();
+            }
+        });
+    }
 }

src/java/com/threerings/presents/tools/GenDObjectTask.java

@@ -259,6 +259,8 @@ public class GenDObjectTask extends Task
             ctx.put("type", GenUtil.simpleName(ftype));
             ctx.put("wrapfield", GenUtil.boxArgument(ftype, "value"));
             ctx.put("wrapofield", GenUtil.boxArgument(ftype, "ovalue"));
+            ctx.put("clonefield",
+                    GenUtil.cloneArgument(_dsclass, ftype, "value"));
             ctx.put("capfield", StringUtil.unStudlyName(fname).toUpperCase());
             ctx.put("upfield", StringUtils.capitalize(fname));
             if (ftype.isArray()) {

src/java/com/threerings/presents/tools/GenUtil.java

@@ -116,6 +116,19 @@ public class GenUtil
         }
     }
 
+    /**
+     * Potentially clones the supplied argument if it is the type that
+     * needs such treatment.
+     */
+    public static String cloneArgument (Class dsclazz, Class clazz, String name)
+    {
+        if (clazz.isArray() || dsclazz.isAssignableFrom(clazz)) {
+            return "(" + simpleName(clazz) + ")" + name + ".clone()";
+        } else {
+            return name;
+        }
+    }
+
     /**
      * Reads in the supplied source file and locates the package and class
      * or interface name and returns a fully qualified class name.

src/java/com/threerings/presents/tools/dobject_field.tmpl

@@ -11,7 +11,7 @@
         $type ovalue = this.$field;
         requestAttributeChange(
             $capfield, $wrapfield, $wrapofield);
-        this.$field = value;
+        this.$field = $clonefield;
     }
 #if ($elemtype)
 

src/java/com/threerings/presents/tools/dobject_set.tmpl

@@ -38,8 +38,8 @@
      * change. Proxied copies of this object (on clients) will apply the
      * value change when they received the attribute changed notification.
      */
-    public void set$upfield ($type $field)
+    public void set$upfield ($type value)
     {
-        requestAttributeChange($capfield, $field, this.$field);
-        this.$field = $field;
+        requestAttributeChange($capfield, value, this.$field);
+        this.$field = $clonefield;
     }