United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-6180194 : 142_xx: Missing exception/error messages while using unsynchronized Collection objects concurrently

Details
Type:
Enhancement
Submit Date:
2004-10-16
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2004-12-10
Component:
core-libs
OS:
generic
Sub-Component:
java.util
CPU:
sparc
Priority:
P2
Resolution:
Fixed
Affected Versions:
1.4.2_06
Fixed Versions:
1.4.2_08 (b01)

Related Reports
Backport:
Backport:

Sub Tasks

Description
Problem description is very simple.  In reality people should not use unsynchronized HashMap concurrently.  However, we have a situation with customer where they are using unsynchronized HashMap and running into OptionalDataException.  Here is the problem description.

  Under load they get into OptionalDataException while unmarshalling the arguments that are passed to a remote object when the argument happens to be either HashMap or an object that contains HashMap in its serialization chain.  The problem is very simple in the sense that serialized data of HashMap has size written as 5 entries but numbers of actually written elements are less than 5 and thats when it is encountering OptionalDataException since it is trying to read the entry and at that time it is encountering TC_END_BLOCKDATA (78 in hex).  However, to even figure this out we had to spend lot of time in the investigation by making 9 instrumented patches and also asked them to run it with different jvm too (Xint, hotspot, server and client) and the reason for wasting so much of time is just because of the fact that it is manifesting with various different fashion.  And the reason for having out of sync data in HashMap is that there is a small window in writeObject() between writing the size and writing each key, value pair iterating through the entries of HashMap that some other thread can remove an object from the HashMap.  This will result in situation where the size will not match with the number of actual elements written to the stream.  If number of actual elements written is greater than the size written then there is no problem since the data is Optional and extra entries after size will be skipped by the reader.  However the problem will only be there if the number of actual entries written is less than the size written since it will try to read all entries and encounters END_BLOCK_DATA after reading all actual entries.  HashMap.writeObject() code is as follows:

 private void writeObject(java.io.ObjectOutputStream s)
        throws IOException
    {
            // Write out the threshold, loadfactor, and any hidden stuff
         
            // Write out number of buckets

            // Write out size (number of Mappings)

            s.writeInt(size);

        // this is where the window is.  Between writing the size and getting the Iterator from the HashMap.

        // Write out keys and values (alternating)

        for (... ) {

         .................
        }

  }
###@###.### 10/16/04 00:32 GMT

Customer just clarified:

Please do note that this is not only with HashMap, but also the same
with lot of the Collection objects (Ex: HashSet, ArrayList, TreeMap,
TreeSet, LinkedList, etc.).

Changing the synposis accordingly..
###@###.### 10/18/04 21:45 GMT

                                    

Comments
SUGGESTED FIX

This request is for usability where Sun can throw an Exception with meaningful error message from the writeObject() when they see that the written size doesnt match with the number of elements actually written on the stream.  An example of it is as follows:

    private void writeObject(java.io.ObjectOutputStream s)

        throws IOException

    {

            // Write out the threshold, loadfactor, and any hidden stuff

            s.defaultWriteObject();

 

            // Write out number of buckets

            s.writeInt(table.length);

 

            // Write out size (number of Mappings)

            int sizeWrote = 0;

            s.writeInt((sizeWrote = size));

 

        int elementsWrote = 0;

        // Write out keys and values (alternating)

        for (Iterator i = entrySet().iterator(); i.hasNext(); ) {

            Map.Entry e = (Map.Entry) i.next();

            s.writeObject(e.getKey());

            s.writeObject(e.getValue());

            elementsWrote++;

        }

        if (elementsWrote < sizeWrote) {

          String errorMsg = "+++ written size as '" + sizeWrote + "' but" +

            " there are only '" + elementsWrote + "' elements got written" +

            ". Regular HashMap is not thread safe and there is a small" +

            " window between writing the size and writing the elements where" +

            " some other thread might have removed the elements. This" +

            " will result in OptionalDataException in reader side." +

            " You should use syncrhonized HashMap for avoiding this" +

            " condition.";

          throw new StreamCorruptedException(errorMsg);

        }

    }

 

  By doing this it is very clear that one should use synchronized HashMap when they know that there is a chance of concurrently modifying the entries in that HashMap.
###@###.### 10/16/04 00:32 GMT


Throwing an exception here seems reasonable, but choose a more
appropriate one and use a succinct
yet clear one-line exception message.  Perhaps:

    throw new ConcurrentModificationException(
         "Unsynchronized map was modified during serialization");
###@###.### 10/16/04 09:13 GMT
                                     
2004-10-16
EVALUATION

Commit to fix in 1.4.2_08. This is not a bug. But it will ease the effort
in debugging in case a Java App. uses un unsynchronized collection objects
like HashMap. The fix is to thorw a concurrent modification excetion, in
the event of unsynchronus access to collection objects.
###@###.### 2004-12-06 19:27:49 GMT
                                     
2004-10-16



Hardware and Software, Engineered to Work Together