Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-5041584

(coll) TreeMap.SubMap should not be Serializable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P4
    • Resolution: Not an Issue
    • Affects Version/s: 1.4.2
    • Fix Version/s: None
    • Component/s: core-libs
    • Labels:

      Description



      Name: jl125535 Date: 05/03/2004


      FULL PRODUCT VERSION :
      java version "1.4.1_01"
      Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.1_01-b01)
      Java HotSpot(TM) Client VM (build 1.4.1_01-b01, mixed mode)

      java version "1.5.0-beta"
      Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-beta-b32c)
      Java HotSpot(TM) Client VM (build 1.5.0-beta-b32c, mixed mode)


      ADDITIONAL OS VERSION INFORMATION :
      Windows 98 [Version 4.10.2222]


      A DESCRIPTION OF THE PROBLEM :
      Problem found when testing Apache Jakarta Commons Collections.

      TreeMap headMap/tailMap/subMap are declared as Serializable:
      were wounded in
          private class SubMap extends AbstractMap
                                   implements SortedMap, java.io.Serializable {

      However, the implementation contains a transient variable for the entrySet which is returned by the method:

              private transient Set entrySet = new EntrySetView();

              public Set entrySet() {
                  return entrySet;
              }

      There is no code to reinitialise the instance variable entrySet when the class is deserialized. This results in NPEs from most methods of the SubMap.

      The commons collections test case determines whether to run the serialization test by checking whether the map implements Serializable. Thats how I found the bug. We have experience that shows that some users do use the instanceof Serializable test in their own code to determine serializability.

      The documentation of TreeMap seems fine, however as Sun makes the source
      code available, the "implements Serializable" is plain to see (as well as
      being testable by an instanceof). And I would thus expect it to work. I would equally accept removal of the "implements Serializable" clause.


      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Run the following test case:

      import java.io.ByteArrayInputStream;
      import java.io.ByteArrayOutputStream;
      import java.io.ObjectInputStream;
      import java.io.ObjectOutputStream;
      import java.util.Map;
      import java.util.TreeMap;

      /**
       * Bug in deserialization of TreeMap submap/headmap/tailmap.
       */
      public class TreeMapBug {
          public static void main(String[] args) {
              try {
                  TreeMap map = new TreeMap();
                  map.put("A", "a");
                  map.put("B", "b");
                  map.put("C", "c");
                  map.put("D", "d");

                  // bug also for headmap/tailmap
                  Map subMap = map.subMap("A", "C");

                  ByteArrayOutputStream out = new ByteArrayOutputStream();
                  ObjectOutputStream oos = new ObjectOutputStream(out);
                  // serialize the submap (or headmap/tailmap)
                  oos.writeObject(subMap);
                  out.close();

                  ByteArrayInputStream in =
                      new ByteArrayInputStream(out.toByteArray());
                  ObjectInputStream ois = new ObjectInputStream(in);
                  Map bugMap = (Map) ois.readObject();

                  // some things work
                  bugMap.get("A"); // OK
                  bugMap.put("A", "z"); // OK

                  // but entySet is null (as transient variable)
                  bugMap.entrySet();

                  // methods that depend on entrySet then have problems...
                  bugMap.size(); // NPE
                  bugMap.isEmpty(); // NPE
                  bugMap.keySet().iterator(); // NPE
                  bugMap.values().iterator(); // NPE

              } catch (Exception ex) {
                  ex.printStackTrace();
              }
          }
      }

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Correct deserialization, no NPE
      ACTUAL -
      java.lang.NullPointerException
       at java.util.AbstractMap.size(AbstractMap.java:63)
       at bug.TreeMapBug.main(TreeMapBug.java:41)


      ERROR MESSAGES/STACK TRACES THAT OCCUR :
      Example from commons-collections test case, where a NPE occurs after deserialization. Line 63 refers to
       entrySet().size()
      and NPEs because entrySet() incorrectly deserialized to null.

      3) testSerializeDeserializeThenCompare(TestFixedSizeSortedMap.bulkTestHeadMap.testSerializeDeserializeThenCompare) java.lang.NullPointerException
      at java.util.AbstractMap.size(AbstractMap.java:63)
      at org.apache.commons.collections.map.AbstractMapDecorator.size(AbstractMapDecorator.java:118)
      at java.util.AbstractMap.equals(AbstractMap.java:504)
      at org.apache.commons.collections.map.AbstractMapDecorator.equals(AbstractMapDecorator.java:129)
      at org.apache.commons.collections.AbstractTestObject.testSerializeDeserializeThenCompare(AbstractTestObject.java:135)
      at sun.reflect.GeneratedMethodAccessor75.invoke(Unknown Source)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      at org.apache.commons.collections.map.TestAll.main(TestAll.java:38)


      REPRODUCIBILITY :
      This bug can be reproduced always.

      CUSTOMER SUBMITTED WORKAROUND :
      No workaround - SubMap serialization is broken.

      Proposed fix
      ------------------
      Initialise the variable in the method, and ensure all uses go via the method (see isEmpty()).

              private transient Set entrySet = null;

              public Set entrySet() {
                  if (entrySet == null) {
                      entrySet = new EntrySetView();
                  }
                  return entrySet;
              }

      I would equally accept removal of the implements Serializable clause.
      (Incident Review ID: 246113)
      ======================================================================

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                mduigou Michael Duigou
                Reporter:
                jleesunw Jon Lee (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Imported:
                  Indexed: