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

Graphs with cycles and writeReplace()/readResole() fail based on instance names

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P3
    • Resolution: Won't Fix
    • Affects Version/s: 6u10, 6u13
    • Fix Version/s: None
    • Component/s: core-libs
    • Labels:

      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".

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                peterjones Peter Jones
                Reporter:
                ryeung Roger Yeung (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Imported:
                  Indexed: