Fix modification of map while iterating in SimpleJsonFormat

pkg/serialization/lib/src/format.dart

Index: pkg/serialization/lib/src/format.dart
diff --git a/pkg/serialization/lib/src/format.dart b/pkg/serialization/lib/src/format.dart
index ab602c92a2b80c29997324b736f7a592c07b1f08..fd6b10f59899d2fe646d46b28e09d12de3676a66 100644
--- a/pkg/serialization/lib/src/format.dart
+++ b/pkg/serialization/lib/src/format.dart
@@ -151,12 +151,20 @@ class SimpleJsonFormat extends Format {
 
   /**
    * For one particular entry, which is either a Map or a List, update it
-   * to turn References into a nested List/Map.
+   * to turn References into a nested List/Map. If it's a Map, and the key

[Jennifer Messerly, 2013/02/13 02:07:41]

Unless this is purely an internal type, I'd put this in a // comment inside the
method instead of doc comment. Users don't need to know the pains we go through
to implement the library :)

[Alan Knight, 2013/02/13 22:40:31]

Done.

+   * might be a reference, we need to bend over backwards to avoid
+   * concurrent modifications.
    */
   jsonifyEntry(map, Writer w) {
+    var keysToRemove = [];
+    var updates = new Map();
     keysAndValues(map).forEach((key, value) {
-      if (value is Reference) map[key] = w.stateForReference(value);
+      var newKey = (key is Reference) ? w.stateForReference(key) : key;
+      if (value is Reference) updates[newKey] = w.stateForReference(value);
+      if (key != newKey) keysToRemove.add(key);

[Jennifer Messerly, 2013/02/13 02:07:41]

We weren't removing keys before--is this also a bug fix?
Might be worth adding another test for this case (where the key is a Reference)

[Alan Knight, 2013/02/13 22:40:31]

I added that, because one of the symptoms was an unresolved Reference getting
through, but I believe the check isn't necessary, because that should never
happen. So I've removed it.

Non-primitive keys, which generate a reference, do arise already in the case of
the meta-serialization where the BasicRule does already use an empty array as a
field name. But I added a more explicit test case just to be on the safe side.

I'm pretty sure the underlying problem here is with iterating over a map and
replacing the values at keys not being concurrent-modification safe. When the
map is already at capacity, it can force a grow on the assumption it might need
to add a new element, and then the iteration fails.

     });
+    keysToRemove.forEach((each) => map.remove(each));
+    updates.forEach((k, v) => map[k] = v);
   }
 
   /**