JDK-6785441 : Graphs with cycles and writeReplace()/readResole() fail based on instance names
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.io:serialization
  • Affected Version: 6u10,6u13
  • Priority: P3
  • Status: Closed
  • Resolution: Won't Fix
  • OS: windows_xp
  • CPU: x86
  • Submitted: 2008-12-16
  • Updated: 2010-04-04
  • Resolved: 2009-01-05
Related Reports
Duplicate :  
JDK-6842012 - "writeReplace" and "readResolve" problem with cyclic objet graph
Description
FULL PRODUCT VERSION :
java version "1.6.0_10"
Java(TM) SE Runtime Environment (build 1.6.0_10-b33)
Java HotSpot(TM) Client VM (build 11.0-b15, mixed mode, sharing)

ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows [Version 5.2.3790]
Microsoft Windows XP [Version 5.1.2600]

(Tested on XP Pro 64 and XP Pro 32)

EXTRA RELEVANT SYSTEM CONFIGURATION :
None (though my test uses log4j)

A DESCRIPTION OF THE PROBLEM :
/**
 * When a graph has bi-directional links with intermediate forms that are saved
 * separately using writeReplace() and readResolve() the order in which the
 * serialization occurs (as determined by the names chosen for the instance
 * variables in the containing object) will consistently cause Serialization to
 * fail with class cast exceptions as the intermediate form's readResolve() call
 * is skipped.
 *
 * Test definition:
 *
 * A <-> B <- C
 *       ^- stored externally in D
 *
 * So, A and B each refer to the other, and C also refers to B.
 *
 * When B is serialized, it uses writeReplace to <em>actually</em> save
 * instances of D. On de-serialization, D instances should be converted back
 * into B instances through readResolve().
 *
 * In ContainerFails, the C instance is processed first ("l1" is less than "l2")
 * and this fails on de-serialization with a class-cast exception, unable to
 * cast a D to a B.
 *
 * The ContainerPasses, the only difference is that l2 is renamed to l0. In this
 * way, the A instance will be processed first, and de-serialization works.
 *
 * Notice that in both cases, serialization passes.
 *
 * @author Brian Jones
 *
 */


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Paste this class and run it. The log4j/logger stuff can be stripped out and not effect the results, but I found it useful to watch as it ran.

The failure occurs every time. The ContainerFails always fails and ContainerPasses always passes.

The only difference is instance names (which cause sorting before processing).


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The names chosen for instance variables should not have affected the behavior, so I expected BOTH sequences to succeed.
ACTUAL -
The first sequence, ContainerFails, throws an exception instead of succeeding.


ERROR MESSAGES/STACK TRACES THAT OCCUR :

java.lang.ClassCastException: cannot assign instance of SerializationTesting$B$D to field SerializationTesting$A.b of type SerializationTesting$B in instance of SerializationTesting$A
	at java.io.ObjectStreamClass$FieldReflector.setObjFieldValues(Unknown Source)
	at java.io.ObjectStreamClass.setObjFieldValues(Unknown Source)
	at java.io.ObjectInputStream.defaultReadFields(Unknown Source)
	at java.io.ObjectInputStream.readSerialData(Unknown Source)
	at java.io.ObjectInputStream.readOrdinaryObject(Unknown Source)
	at java.io.ObjectInputStream.readObject0(Unknown Source)
	at java.io.ObjectInputStream.defaultReadFields(Unknown Source)
	at java.io.ObjectInputStream.readSerialData(Unknown Source)
	at java.io.ObjectInputStream.readOrdinaryObject(Unknown Source)
	at java.io.ObjectInputStream.readObject0(Unknown Source)
	at java.io.ObjectInputStream.defaultReadFields(Unknown Source)
	at java.io.ObjectInputStream.readSerialData(Unknown Source)
	at java.io.ObjectInputStream.readOrdinaryObject(Unknown Source)
	at java.io.ObjectInputStream.readObject0(Unknown Source)
	at java.io.ObjectInputStream.defaultReadFields(Unknown Source)
	at java.io.ObjectInputStream.readSerialData(Unknown Source)
	at java.io.ObjectInputStream.readOrdinaryObject(Unknown Source)
	at java.io.ObjectInputStream.readObject0(Unknown Source)
	at java.io.ObjectInputStream.readObject(Unknown Source)
	at SerializationTesting.main(SerializationTesting.java:257)

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.log4j.BasicConfigurator;

/**
 * When a graph has bi-directional links with intermediate forms that are saved
 * separately using writeReplace() and readResolve() the order in which the
 * serialization occurs (as determined by the names chosen for the instance
 * variables in the containing object) will consistently cause Serialization to
 * fail with class cast exceptions as the intermediate form's readResolve() call
 * is skipped.
 *
 * Test definition:
 *
 * A <-> B <- C
 *       ^- stored externally in D
 *
 * So, A and B each refer to the other, and C also refers to B.
 *
 * When B is serialized, it uses writeReplace to <em>actually</em> save
 * instances of D. On de-serialization, D instances should be converted back
 * into B instances through readResolve().
 *
 * In ContainerFails, the C instance is processed first ("l1" is less than "l2")
 * and this fails on de-serialization with a class-cast exception, unable to
 * cast a D to a B.
 *
 * The ContainerPasses, the only difference is that l2 is renamed to l0. In this
 * way, the A instance will be processed first, and de-serialization works.
 *
 * Notice that in both cases, serialization passes.
 *
 * @author Brian Jones
 *
 */

public class SerializationTesting
{
    private static final org.apache.log4j.Logger logger = org.apache.log4j.Logger
                                                                .getLogger(A.class);

    public static class A implements Serializable
    {
        public A()
        {
            logger.debug("Entering A() where A's counter = " + counter);
            b = new B(this);
            logger.debug("A() finished (A.counter = " + counter + ")");
        }

        public B getB()
        {
            logger.debug("A.getB() where A.counter=" + counter
                    + ", returning B = " + b);
            return b;
        }

        private final B           b;

        private final long        counter          = MasterCounter
                                                           .incrementAndGet();

        private final static long serialVersionUID = 1L;

        @Override
        public String toString()
        {
            return "A:" + counter;
        }
    }

    public static class B implements Serializable
    {
        public B(A a)
        {
            logger.debug("Entering B(A) with a = " + a
                    + " and this B's counter=" + counter);
            this.a = a;
            logger.debug("B() finished, B.counter = " + counter);
        }

        public A getA()
        {
            logger.debug("B.getA() where A.counter=" + counter
                    + ", returning A = " + a);
            return a;
        }

        private Object writeReplace()
        {
            logger
                    .debug("Entering writeReplace() in B with counter="
                            + counter);
            return new D(a);
        }

        private static class D implements Serializable
        {

            public D(A a)
            {
                logger.debug("Storage class D entered, counter = " + counter
                        + " and a=" + a);
                this.a = a;
            }

            private Object readResolve()
            {
                logger.debug("D.readResolve() with counter = " + counter
                        + " and a=" + a);
                return new B(a);
            }

            private final A           a;

            private final long        counter          = MasterCounter
                                                               .incrementAndGet();

            private final static long serialVersionUID = 1L;
        }

        private final A           a;

        private final long        counter          = MasterCounter
                                                           .incrementAndGet();

        private final static long serialVersionUID = 1L;

        @Override
        public String toString()
        {
            return "B:" + counter;
        }
    }

    public static class C implements Serializable
    {
        public C(B b)
        {
            logger.debug("Entering C(B) with b = " + b
                    + " and this C's counter=" + counter);
            this.b = b;
            logger.debug("C() finished, C.counter = " + counter);
        }

        public B getB()
        {
            logger.debug("C.getB() where C.counter=" + counter
                    + ", returning B = " + b);
            return b;
        }

        private final B           b;

        private final long        counter          = MasterCounter
                                                           .incrementAndGet();

        private final static long serialVersionUID = 1L;

        @Override
        public String toString()
        {
            return "C:" + counter;
        }
    }

    public static class ContainerFails implements Serializable
    {
        public final A l2 = new A();

        public final C l1 = new C(l2.getB());

        public void sanityCheck()
        {
            List<String> errors = new ArrayList<String>();
            logger.debug("Begin sanity checks");
            if (l2 != l1.getB().getA())
                errors.add("SANITY FAILURE 1 (a != c->b->a)");
            if (l2.getB() != l1.getB())
                errors.add("SANITY FAILURE 2 (a->b != c->b)");
            logger.debug("End sanity checks");
            if (errors.isEmpty())
                logger.debug("SANITY PASS");
            else
            {
                logger.fatal("Errors in sanity (listed below)");
                for (String error : errors)
                {
                    logger.fatal(error);
                }
            }
        }

        private final static long serialVersionUID = 1L;
    }

    public static class ContainerPasses implements Serializable
    {
        public final A l0 = new A();

        public final C l1 = new C(l0.getB());

        public void sanityCheck()
        {
            List<String> errors = new ArrayList<String>();
            logger.debug("Begin sanity checks");
            if (l0 != l1.getB().getA())
                errors.add("SANITY FAILURE 1 (a != c->b->a)");
            if (l0.getB() != l1.getB())
                errors.add("SANITY FAILURE 2 (a->b != c->b)");
            logger.debug("End sanity checks");
            if (errors.isEmpty())
                logger.debug("SANITY PASS");
            else
            {
                logger.fatal("Errors in sanity (listed below)");
                for (String error : errors)
                {
                    logger.fatal(error);
                }
            }
        }

        private final static long serialVersionUID = 1L;
    }

    public static void main(String[] args) throws Exception
    {
        BasicConfigurator.configure();

        try
        {
            ContainerFails cont1 = new ContainerFails();
            cont1.sanityCheck();

            logger
                    .debug("Flush the cont1 (fails case) into serialized byte array");
            ByteArrayOutputStream bas1 = new ByteArrayOutputStream();
            ObjectOutputStream out1 = new ObjectOutputStream(bas1);
            out1.writeObject(cont1);
            out1.close();

            byte[] bytes1 = bas1.toByteArray();
            bas1.close();

            logger.debug("De-serializing from byte array bytes1");
            ByteArrayInputStream bis1 = new ByteArrayInputStream(bytes1);
            ObjectInputStream in2 = new ObjectInputStream(bis1);
            Object raw2 = in2.readObject();
            logger.debug("raw2 reads OK");
            ContainerFails cont2 = (ContainerFails) raw2;
            logger.debug("con2 loaded");
            cont2.sanityCheck();
            logger.info("Load passes");
        }
        catch (Exception e)
        {
            logger.fatal("Caught expected exception on fails");
            e.printStackTrace();
        }

        try
        {
            ContainerPasses cont1 = new ContainerPasses();
            cont1.sanityCheck();

            logger
                    .debug("Flush the cont1 (passes case) into serialized byte array");
            ByteArrayOutputStream bas1 = new ByteArrayOutputStream();
            ObjectOutputStream out1 = new ObjectOutputStream(bas1);
            out1.writeObject(cont1);
            out1.close();

            byte[] bytes1 = bas1.toByteArray();
            bas1.close();

            logger.debug("De-serializing from byte array bytes1");
            ByteArrayInputStream bis1 = new ByteArrayInputStream(bytes1);
            ObjectInputStream in2 = new ObjectInputStream(bis1);
            Object raw2 = in2.readObject();
            logger.debug("raw2 reads OK");
            ContainerPasses cont2 = (ContainerPasses) raw2;
            logger.debug("con2 loaded");
            cont2.sanityCheck();
            logger.info("Load passes");
        }
        catch (Exception e)
        {
            logger.fatal("Caught UN-expected exception on fails");
            e.printStackTrace();
        }
    }

    private static final AtomicLong MasterCounter = new AtomicLong();
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
The ContainerPasses works because I ensured that the instance name that was bi-directionally coupled was processed before the instance that was not.

The problem can  be fixed (even in the real system where I got burned) by re-naming instance variables to precisely control the re-load order of the graph.

However, in a production system, you don't find this problem until something doesn't de-serialize. And at that point, you can't get the data back. We hit it in test (thankfully) but it could easily be lurking if the testing misses some cases.

I don't know what would happen if we added another bi-directional link such that we couldn't _ever_ load in the right order. That's why I ranked the bug as "difficult".
Comments
EVALUATION The general problem here is that the readResolve feature does not interact well with circularities in the graph of serialized objects-- in particular, when the to-be-resolved object's serialized form contains a reference that, through a chain of one or more references, refers to the original object. The specification warns about this general limitation: http://java.sun.com/javase/6/docs/platform/serialization/spec/input.html#5903 Note - The readResolve method is not invoked on the object until the object is fully constructed, so any references to this object in its object graph will not be updated to the new object nominated by readResolve. However, during the serialization of an object with the writeReplace method, all references to the original object in the replacement object's object graph are replaced with references to the replacement object. Therefore in cases where an object being serialized nominates a replacement object whose object graph has a reference to the original object, deserialization will result in an incorrect graph of objects. Furthermore, if the reference types of the object being read (nominated by writeReplace) and the original object are not compatible, the construction of the object graph will raise a ClassCastException. If the order of objects in the serialization stream is such that the object in the circular reference chain that refers to the to-be-resolved object appears before the to-be-resolved object, then deserialization can work, because the latter object can be resolved before the referring object's field is assigned. That is why the names of the fields in the supplied example matter: an object's serializable fields are serialized and deserialized in the order of their names. The same effect could be achieved by other means of controlling the order of objects in the stream, such as by directly writing an A followed by a C to the stream vs. doing so in the opposite order. In the ContainerPasses case, the A is serialized first (because of the field names), so the A/B circularity is not a problem on deserialization, as described above. In the ContainerFails case, the C is serialized first, which triggers the serialization of the A/B cycle starting with the B (because of the C's reference), which causes the problem the spec warns about-- the A.b field contains a reference to the already encountered D object, which has not yet been resolved to a B object, hence the ClassCastException when attempting to assign it to the A.b field. Here is a makeshift diagram of the structure of the serialized stream, and comments about what happens during deserialization, in both cases: (legend) #N [X] : serialized object of class X, with index N .x -> : serializable field named "x" ref(N) : reference to previously described object with index N #1 [ContainerPasses] .l0 -> #2 [A] .b -> #3 [D] // resolved to B after its contents deserialized .a -> ref(2) .l1 -> #4 [C] .b -> ref(3) // #3 has been resolved to B by now, so assignment works #1 [ContainerFails] .l1 -> #2 [C] .b -> #3 [D] // would be resolved to B if no ClassCastException .a -> #4 [A] .b -> ref(3) // ClassCastException here, because #3 is still a D .l2 -> // never gets here on deserialization ref(4) These potentially surprising behaviors are fairly baked into the long-standing object serialization format and mechanisms. A stream that would cause this sort of problem at deserialization time cannot be automatically detected at serialization time (without actually deserializing the resulting stream) because a class's writeReplace and readResolve methods are not necessarily coupled or required to have symmetric behavior anyway (the system can't tell what a readResolve method would do without executing it).
05-01-2009