Unify object tracking in client- and server-side SerializationStreamWriters. This will correct a condition in which unique objects will not be written into the stream if there is a hash collision in System.identityHashCode(). Patch by: bobv Reported by: shuniu Review by: scottb git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2335 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStreamWriter.java b/user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStreamWriter.java index 366fd05..60c2e0c 100644 --- a/user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStreamWriter.java +++ b/user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStreamWriter.java
@@ -1,5 +1,5 @@ /* - * Copyright 2007 Google Inc. + * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of @@ -18,14 +18,36 @@ import com.google.gwt.user.client.rpc.SerializationException; import com.google.gwt.user.client.rpc.SerializationStreamWriter; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; + /** * Base class for the client and server serialization streams. This class * handles the basic serialization and deserialization formatting for primitive - * types since these are common between the client and the server. + * types since these are common between the client and the server. It also + * handles Object- and String-tracking for building graph references. */ public abstract class AbstractSerializationStreamWriter extends AbstractSerializationStream implements SerializationStreamWriter { + private int objectCount; + + private Map<Object, Integer> objectMap = new IdentityHashMap<Object, Integer>(); + + private Map<String, Integer> stringMap = new HashMap<String, Integer>(); + + private List<String> stringTable = new ArrayList<String>(); + + public void prepareToWrite() { + objectCount = 0; + objectMap.clear(); + stringMap.clear(); + stringTable.clear(); + } + @Override public abstract String toString(); @@ -97,7 +119,20 @@ * @param string the string to add * @return the index to the string */ - protected abstract int addString(String string); + protected int addString(String string) { + if (string == null) { + return 0; + } + Integer o = stringMap.get(string); + if (o != null) { + return o; + } + stringTable.add(string); + // index is 1-based + int index = stringTable.size(); + stringMap.put(string, index); + return index; + } /** * Append a token to the underlying output buffer. @@ -114,7 +149,9 @@ * @return the index associated with this object, or -1 if this object hasn't * been seen before */ - protected abstract int getIndexForObject(Object instance); + protected int getIndexForObject(Object instance) { + return objectMap.containsKey(instance) ? objectMap.get(instance) : -1; + } /** * Compute and return the type signature for an object. @@ -125,11 +162,20 @@ protected abstract String getObjectTypeSignature(Object instance); /** + * Gets the string table. + */ + protected List<String> getStringTable() { + return stringTable; + } + + /** * Remember this object as having been seen before. * * @param instance the object to remember */ - protected abstract void saveIndexForObject(Object instance); + protected void saveIndexForObject(Object instance) { + objectMap.put(instance, objectCount++); + } /** * Serialize an object into the stream.
diff --git a/user/src/com/google/gwt/user/client/rpc/impl/ClientSerializationStreamWriter.java b/user/src/com/google/gwt/user/client/rpc/impl/ClientSerializationStreamWriter.java index d466958..85dfc22 100644 --- a/user/src/com/google/gwt/user/client/rpc/impl/ClientSerializationStreamWriter.java +++ b/user/src/com/google/gwt/user/client/rpc/impl/ClientSerializationStreamWriter.java
@@ -1,5 +1,5 @@ /* - * Copyright 2007 Google Inc. + * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of @@ -15,10 +15,9 @@ */ package com.google.gwt.user.client.rpc.impl; -import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.user.client.rpc.SerializationException; -import java.util.ArrayList; +import java.util.List; /** * For internal use only. Used for server call serialization. @@ -36,29 +35,10 @@ private final String moduleBaseURL; - private int objectCount; - - /* - * Accessed from JSNI code, so ignore unused warning. - */ - @SuppressWarnings("unused") - private JavaScriptObject objectMap; - private final String serializationPolicyStrongName; private final Serializer serializer; - /* - * Accesses need to be prefixed with ':' to prevent conflict with built-in - * JavaScript properties. - * - * Accessed from JSNI code, so ignore unused warning. - */ - @SuppressWarnings("unused") - private JavaScriptObject stringMap; - - private ArrayList<String> stringTable = new ArrayList<String>(); - /** * Constructs a <code>ClientSerializationStreamWriter</code> that does not * use a serialization policy file. @@ -94,10 +74,7 @@ * implementation <b>must</b> be called by any overridden version. */ public void prepareToWrite() { - objectCount = 0; - objectMap = JavaScriptObject.createObject(); - stringMap = JavaScriptObject.createObject(); - stringTable.clear(); + super.prepareToWrite(); encodeBuffer = new StringBuffer(); if (hasSerializationPolicyInfo()) { @@ -119,23 +96,6 @@ append(Long.toString(fieldValue, 16)); } - @Override - protected int addString(String string) { - if (string == null) { - return 0; - } - - int index = getIntForString(string); - if (index > 0) { - return index; - } - stringTable.add(string); - // index is 1-based (that's why we're taking the size AFTER add) - index = stringTable.size(); - setIntForString(string, index); - return index; - } - /** * Appends a token to the end of the buffer. */ @@ -145,11 +105,6 @@ } @Override - protected int getIndexForObject(Object instance) { - return getIntForInt(System.identityHashCode(instance)); - } - - @Override protected String getObjectTypeSignature(Object o) { Class<?> clazz = o.getClass(); @@ -159,7 +114,7 @@ } String typeName = clazz.getName(); - + String serializationSignature = serializer.getSerializationSignature(typeName); if (serializationSignature != null) { typeName += "/" + serializationSignature; @@ -168,36 +123,11 @@ } @Override - protected void saveIndexForObject(Object instance) { - setIntForInt(System.identityHashCode(instance), objectCount++); - } - - @Override protected void serialize(Object instance, String typeSignature) throws SerializationException { serializer.serialize(this, instance, typeSignature); } - private native int getIntForInt(int key) /*-{ - var result = this.@com.google.gwt.user.client.rpc.impl.ClientSerializationStreamWriter::objectMap[key]; - return (result == null) ? -1 : result; - }-*/; - - // prefix needed to prevent conflict with built-in JavaScript properties. - private native int getIntForString(String key) /*-{ - var result = this.@com.google.gwt.user.client.rpc.impl.ClientSerializationStreamWriter::stringMap[':' + key]; - return (result == null) ? 0 : result; - }-*/; - - private native void setIntForInt(int key, int value) /*-{ - this.@com.google.gwt.user.client.rpc.impl.ClientSerializationStreamWriter::objectMap[key] = value; - }-*/; - - // prefix needed to prevent conflict with built-in JavaScript properties. - private native void setIntForString(String key, int value) /*-{ - this.@com.google.gwt.user.client.rpc.impl.ClientSerializationStreamWriter::stringMap[':' + key] = value; - }-*/; - private void writeHeader(StringBuffer buffer) { append(buffer, String.valueOf(getVersion())); append(buffer, String.valueOf(getFlags())); @@ -208,10 +138,10 @@ } private StringBuffer writeStringTable(StringBuffer buffer) { - int stringTableSize = stringTable.size(); - append(buffer, String.valueOf(stringTableSize)); - for (int i = 0; i < stringTableSize; ++i) { - append(buffer, stringTable.get(i)); + List<String> stringTable = getStringTable(); + append(buffer, String.valueOf(stringTable.size())); + for (String s : stringTable) { + append(buffer, s); } return buffer; }
diff --git a/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java b/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java index 9dc1e9d..2b87eef 100644 --- a/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java +++ b/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java
@@ -25,8 +25,8 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; -import java.util.HashMap; import java.util.IdentityHashMap; +import java.util.List; import java.util.Map; /** @@ -330,13 +330,13 @@ } /** - * Returns the {@link Class} instance to use for serialization. Enumerations - * are serialized as their declaring class while all others are serialized + * Returns the {@link Class} instance to use for serialization. Enumerations + * are serialized as their declaring class while all others are serialized * using their true class instance. */ private static Class<?> getClassForSerialization(Object instance) { assert (instance != null); - + if (instance instanceof Enum) { Enum<?> e = (Enum<?>) instance; return e.getDeclaringClass(); @@ -436,14 +436,6 @@ } } - private int objectCount; - - private IdentityHashMap<Object, Integer> objectMap = new IdentityHashMap<Object, Integer>(); - - private HashMap<String, Integer> stringMap = new HashMap<String, Integer>(); - - private ArrayList<String> stringTable = new ArrayList<String>(); - private ArrayList<String> tokenList = new ArrayList<String>(); private int tokenListCharCount; @@ -454,13 +446,11 @@ this.serializationPolicy = serializationPolicy; } + @Override public void prepareToWrite() { - objectCount = 0; - objectMap.clear(); + super.prepareToWrite(); tokenList.clear(); tokenListCharCount = 0; - stringMap.clear(); - stringTable.clear(); } public void serializeValue(Object value, Class<?> type) @@ -505,22 +495,6 @@ } @Override - protected int addString(String string) { - if (string == null) { - return 0; - } - Integer o = stringMap.get(string); - if (o != null) { - return o; - } - stringTable.add(string); - // index is 1-based - int index = stringTable.size(); - stringMap.put(string, index); - return index; - } - - @Override protected void append(String token) { tokenList.add(token); if (token != null) { @@ -529,18 +503,9 @@ } @Override - protected int getIndexForObject(Object instance) { - Integer o = objectMap.get(instance); - if (o != null) { - return o; - } - return -1; - } - - @Override protected String getObjectTypeSignature(Object instance) { assert (instance != null); - + Class<?> clazz = getClassForSerialization(instance); if (shouldEnforceTypeVersioning()) { return SerializabilityUtil.encodeSerializedInstanceReference(clazz); @@ -550,11 +515,6 @@ } @Override - protected void saveIndexForObject(Object instance) { - objectMap.put(instance, objectCount++); - } - - @Override protected void serialize(Object instance, String typeSignature) throws SerializationException { assert (instance != null); @@ -691,6 +651,7 @@ } private void writeStringTable(StringBuffer buffer) { + List<String> stringTable = getStringTable(); if (tokenList.size() > 0) { buffer.append(","); }