This is the mail archive of the java@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Serialization bug fixes


I found some serialization bugs when debugging JOnAS.  The fixes are
on gcj-abi-2-dev-branch for some while, but I've kept them to myself
for too long.

The current implementation doesn't handle nested objects very well,
and doesn't use the correct class loader.  I fixed that, and a few
minor things as well.

The good news is that with these fixes, serialization seems to work
well, even in a very complex app.

The bad news is that I've added a ton of extra debugging which
obfuscates the real changes.  Sorry, but I couldn't have found the
bugs without it and it might be useful in the future.

Also, I needed a native method to walk the stack efficiently to find
the caller's class loader.  This will clearly cause problems for
Classpath.  I'm hoping that we'll get a cleaner way to do this.

For Tom Tromey: any method that needs to find its caller needs to be
compiled without sibcalls,so we need some common infrastructure for
this.

Andrew.


	* java/io/ObjectOutputStream.java: Add DEBUG statements
	everywhere.
	(dumpElementln): New method.
	(depth): New field.
	* java/io/ObjectInputStream.java
	(currentClassLoader): Make native.
	(callersClassLoader): New field.
	(depth): New field.
	* java/io/natObjectInputStream.cc (getCallersClassLoader): New
	method.

	(readObject): ENDBLOCKDATA is generated if the class has a write
	method, not if it has a read method.

Index: java/io/ObjectInputStream.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/ObjectInputStream.java,v
retrieving revision 1.31
diff -c -2 -p -w -r1.31 ObjectInputStream.java
*** java/io/ObjectInputStream.java	20 Apr 2004 11:37:41 -0000	1.31
--- java/io/ObjectInputStream.java	17 Jun 2004 14:45:57 -0000
*************** exception statement from your version. *
*** 39,51 ****
  package java.io;
  
- import gnu.classpath.Configuration;
- import gnu.java.io.ObjectIdentityWrapper;
- 
  import java.lang.reflect.Array;
- import java.lang.reflect.Field;
- import java.lang.reflect.InvocationTargetException;
- import java.lang.reflect.Method;
  import java.lang.reflect.Modifier;
  import java.lang.reflect.Proxy;
  import java.util.Arrays;
  import java.util.Hashtable;
--- 39,47 ----
  package java.io;
  
  import java.lang.reflect.Array;
  import java.lang.reflect.Modifier;
  import java.lang.reflect.Proxy;
+ import java.security.PrivilegedAction;
+ import java.security.AccessController;
  import java.util.Arrays;
  import java.util.Hashtable;
*************** import java.util.Vector;
*** 53,56 ****
--- 49,60 ----
  
  
+ import gnu.java.io.ObjectIdentityWrapper;
+ import gnu.java.lang.reflect.TypeSignature;
+ import java.lang.reflect.Field;
+ import java.lang.reflect.Method;
+ import java.lang.reflect.InvocationTargetException;
+ 
+ import gnu.classpath.Configuration;
+ 
  public class ObjectInputStream extends InputStream
    implements ObjectInput, ObjectStreamConstants
*************** public class ObjectInputStream extends I
*** 121,124 ****
--- 125,137 ----
    public final Object readObject() throws ClassNotFoundException, IOException
    {
+       if (callersClassLoader == null)
+ 	{
+ 	  callersClassLoader = getCallersClassLoader ();
+ 	  if (Configuration.DEBUG && dump)
+ 	    {
+ 	      dumpElementln ("CallersClassLoader = " + callersClassLoader);
+ 	    }
+ 	}
+ 
      if (this.useSubclassMethod)
        return readObjectOverride();
*************** public class ObjectInputStream extends I
*** 135,138 ****
--- 148,154 ----
  
      byte marker = this.realInputStream.readByte();
+ 
+     depth += 2;
+ 
      if(dump) dumpElement("MARKER: 0x" + Integer.toHexString(marker) + " ");
  
*************** public class ObjectInputStream extends I
*** 152,158 ****
  	    {
  	      if (marker == TC_BLOCKDATALONG)
! 		if(dump) dumpElementln("BLOCKDATALONG");
  	      else
! 		if(dump) dumpElementln("BLOCKDATA");
  	      readNextBlock(marker);
  	      throw new StreamCorruptedException("Unexpected blockData");
--- 168,174 ----
  	    {
  	      if (marker == TC_BLOCKDATALONG)
! 		{ if(dump) dumpElementln("BLOCKDATALONG"); }
  	      else
! 		{ if(dump) dumpElementln("BLOCKDATA"); }
  	      readNextBlock(marker);
  	      throw new StreamCorruptedException("Unexpected blockData");
*************** public class ObjectInputStream extends I
*** 320,323 ****
--- 336,342 ----
  	      
  	      int handle = assignNewHandle(obj);
+ 	      Object prevObject = this.currentObject;
+ 	      ObjectStreamClass prevObjectStreamClass = this.currentObjectStreamClass;
+ 	      
  	      this.currentObject = obj;
  	      ObjectStreamClass[] hierarchy =
*************** public class ObjectInputStream extends I
*** 342,350 ****
  		      callReadMethod(readObjectMethod, this.currentObjectStreamClass.forClass(), obj);
  		      setBlockDataMode(oldmode);
  		      if(dump) dumpElement("ENDBLOCKDATA? ");
  		      try
  			{
! 			  // FIXME: XXX: This try block is to catch EOF which is
! 			  // thrown for some objects.  That indicates a bug in the logic.
  			  if (this.realInputStream.readByte() != TC_ENDBLOCKDATA)
  			    throw new IOException
--- 361,380 ----
  		      callReadMethod(readObjectMethod, this.currentObjectStreamClass.forClass(), obj);
  		      setBlockDataMode(oldmode);
+ 		    }
+ 		  else
+ 		    {
+ 		      readFields(obj, currentObjectStreamClass);
+ 		    }
+ 
+ 		  if (this.currentObjectStreamClass.hasWriteMethod())
+ 		    {
  		      if(dump) dumpElement("ENDBLOCKDATA? ");
  		      try
  			{
! 			  // FIXME: XXX: This try block is to
! 			  // catch EOF which is thrown for some
! 			  // objects.  That indicates a bug in
! 			  // the logic.
! 
  			  if (this.realInputStream.readByte() != TC_ENDBLOCKDATA)
  			    throw new IOException
*************** public class ObjectInputStream extends I
*** 352,359 ****
  			  if(dump) dumpElementln("yes");
  			}
! 		      catch (EOFException e)
! 			{
! 			  if(dump) dumpElementln("no, got EOFException");
! 			}
  		      catch (IOException e)
  			{
--- 382,389 ----
  			  if(dump) dumpElementln("yes");
  			}
! // 		      catch (EOFException e)
! // 			{
! // 			  if(dump) dumpElementln("no, got EOFException");
! // 			}
  		      catch (IOException e)
  			{
*************** public class ObjectInputStream extends I
*** 361,373 ****
  			}
  		    }
- 		  else
- 		    {
- 		      readFields(obj, currentObjectStreamClass);
- 		    }
  		}
  
! 	      this.currentObject = null;
! 	      this.currentObjectStreamClass = null;
  	      ret_val = processResolution(osc, obj, handle);
  	      break;
  	    }
--- 391,400 ----
  			}
  		    }
  		}
  
! 	      this.currentObject = prevObject;
! 	      this.currentObjectStreamClass = prevObjectStreamClass;
  	      ret_val = processResolution(osc, obj, handle);
+ 		  
  	      break;
  	    }
*************** public class ObjectInputStream extends I
*** 398,401 ****
--- 425,430 ----
  	this.isDeserializing = was_deserializing;
  
+ 	depth -= 2;
+ 	
  	if (! was_deserializing)
  	  {
*************** public class ObjectInputStream extends I
*** 711,715 ****
      throws ClassNotFoundException, IOException
    {
!     return Class.forName(osc.getName(), true, currentLoader());
    }
  
--- 740,744 ----
      throws ClassNotFoundException, IOException
    {
!     return Class.forName(osc.getName(), true, callersClassLoader);
    }
  
*************** public class ObjectInputStream extends I
*** 1803,1811 ****
     * @return The current class loader in the calling stack.
     */
!   private static ClassLoader currentClassLoader (SecurityManager sm)
!   {
!     // FIXME: This is too simple.
!     return ClassLoader.getSystemClassLoader ();
!   }
  
    private void callReadMethod (Method readObject, Class klass, Object obj) throws IOException
--- 1832,1838 ----
     * @return The current class loader in the calling stack.
     */
!   private static native ClassLoader currentClassLoader (SecurityManager sm);
!   
!   private native ClassLoader getCallersClassLoader();
  
    private void callReadMethod (Method readObject, Class klass, Object obj) throws IOException
*************** public class ObjectInputStream extends I
*** 1865,1868 ****
--- 1892,1899 ----
    private static boolean dump = false && Configuration.DEBUG;
  
+   private ClassLoader callersClassLoader;
+ 
+   private int depth = 0;
+ 
    private void dumpElement (String msg)
    {	
*************** public class ObjectInputStream extends I
*** 1873,1876 ****
--- 1904,1910 ----
    {
      System.out.println(msg);
+     for (int i = 0; i < depth; i++)
+       System.out.print (" ");
+     System.out.print (Thread.currentThread() + ": ");
    }
  
Index: java/io/ObjectOutputStream.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/ObjectOutputStream.java,v
retrieving revision 1.24
diff -c -2 -p -w -r1.24 ObjectOutputStream.java
*** java/io/ObjectOutputStream.java	31 Dec 2003 11:04:21 -0000	1.24
--- java/io/ObjectOutputStream.java	17 Jun 2004 14:45:57 -0000
*************** public class ObjectOutputStream extends 
*** 145,148 ****
--- 145,155 ----
      useSubclassMethod = false;
      writeStreamHeader();
+ 
+     if (Configuration.DEBUG)
+       {
+ 	String val = System.getProperty("gcj.dumpobjects");
+ 	if (val != null && !val.equals(""))
+ 	  dump = true;
+       }
    }
  
*************** public class ObjectOutputStream extends 
*** 173,180 ****
--- 180,195 ----
      if (useSubclassMethod)
        {
+ 	if (dump)
+ 	  dumpElementln ("WRITE OVERRIDE: " + obj);
+ 	  
  	writeObjectOverride(obj);
  	return;
        }
  
+     if (dump)
+       dumpElementln ("WRITE: " + obj);
+     
+     depth += 2;    
+ 
      boolean was_serializing = isSerializing;
      boolean old_mode = setBlockDataMode(false);
*************** public class ObjectOutputStream extends 
*** 319,322 ****
--- 334,339 ----
  	    if (obj instanceof Serializable)
  	      {
+ 		Object prevObject = this.currentObject;
+ 		ObjectStreamClass prevObjectStreamClass = this.currentObjectStreamClass;
  		currentObject = obj;
  		ObjectStreamClass[] hierarchy =
*************** public class ObjectOutputStream extends 
*** 330,344 ****
  		    if (currentObjectStreamClass.hasWriteMethod())
  		      {
  			setBlockDataMode(true);
  			callWriteMethod(obj, currentObjectStreamClass);
  			setBlockDataMode(false);
  			realOutput.writeByte(TC_ENDBLOCKDATA);
  		      }
  		    else
  		      writeFields(obj, currentObjectStreamClass);
  		  }
  
! 		currentObject = null;
! 		currentObjectStreamClass = null;
  		currentPutField = null;
  		break;
--- 347,369 ----
  		    if (currentObjectStreamClass.hasWriteMethod())
  		      {
+ 			if (dump)
+ 			  dumpElementln ("WRITE METHOD CALLED FOR: " + obj);
  			setBlockDataMode(true);
  			callWriteMethod(obj, currentObjectStreamClass);
  			setBlockDataMode(false);
  			realOutput.writeByte(TC_ENDBLOCKDATA);
+ 			if (dump)
+ 			  dumpElementln ("WRITE ENDBLOCKDATA FOR: " + obj);
  		      }
  		    else
+ 		      {
+ 			if (dump)
+ 			  dumpElementln ("WRITE FIELDS CALLED FOR: " + obj);
  			writeFields(obj, currentObjectStreamClass);
  		      }
+ 		  }
  
! 		this.currentObject = prevObject;
! 		this.currentObjectStreamClass = prevObjectStreamClass;
  		currentPutField = null;
  		break;
*************** public class ObjectOutputStream extends 
*** 361,370 ****
  	try
  	  {
  	    writeObject(e);
  	  }
  	catch (IOException ioe)
  	  {
! 	    throw new StreamCorruptedException
! 	      ("Exception " + ioe + " thrown while exception was being written to stream.");
  	  }
  
--- 386,405 ----
  	try
  	  {
+ 	    if (Configuration.DEBUG)
+ 	      {
+ 		e.printStackTrace(System.out);
+ 	      }
  	    writeObject(e);
  	  }
  	catch (IOException ioe)
  	  {
! 	    StreamCorruptedException ex = 
! 	      new StreamCorruptedException
! 	      (ioe + " thrown while exception was being written to stream.");
! 	    if (Configuration.DEBUG)
! 	      {
! 		ex.printStackTrace(System.out);
! 	      }
! 	    throw ex;
  	  }
  
*************** public class ObjectOutputStream extends 
*** 376,379 ****
--- 411,418 ----
  	isSerializing = was_serializing;
  	setBlockDataMode(old_mode);
+ 	depth -= 2;
+ 
+ 	if (dump)
+ 	  dumpElementln ("END: " + obj);
        }
    }
*************** public class ObjectOutputStream extends 
*** 1172,1175 ****
--- 1211,1217 ----
  	type = fields[i].getType();
  
+ 	if (dump)
+ 	  dumpElementln ("WRITE FIELD: " + field_name + " type=" + type);
+ 
  	if (type == Boolean.TYPE)
  	  realOutput.writeBoolean(getBooleanField(obj, osc.forClass(), field_name));
*************** public class ObjectOutputStream extends 
*** 1513,1516 ****
--- 1555,1566 ----
    }
  
+   private void dumpElementln (String msg)
+   {
+     for (int i = 0; i < depth; i++)
+       System.out.print (" ");
+     System.out.print (Thread.currentThread() + ": ");
+     System.out.println(msg);
+   }
+ 
    // this value comes from 1.2 spec, but is used in 1.1 as well
    private final static int BUFFER_SIZE = 1024;
*************** public class ObjectOutputStream extends 
*** 1535,1538 ****
--- 1585,1592 ----
    private boolean useSubclassMethod;
  
+   private int depth = 0;
+ 
+   private boolean dump = false;
+ 
    static
    {
Index: java/io/natObjectInputStream.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/io/natObjectInputStream.cc,v
retrieving revision 1.7
diff -c -2 -p -w -r1.7 natObjectInputStream.cc
*** java/io/natObjectInputStream.cc	20 Apr 2004 01:38:45 -0000	1.7
--- java/io/natObjectInputStream.cc	17 Jun 2004 14:45:57 -0000
*************** details.  */
*** 20,23 ****
--- 20,25 ----
  #include <java/lang/reflect/Modifier.h>
  #include <java/lang/reflect/Method.h>
+ #include <java/lang/ArrayIndexOutOfBoundsException.h>
+ #include <java/lang/SecurityManager.h>
  
  #ifdef DEBUG
*************** java::io::ObjectInputStream::allocateObj
*** 39,43 ****
        else
  	{
! 	  obj = _Jv_AllocObject (klass);
  	}
      }
--- 41,45 ----
        else
  	{
! 	  obj = JvAllocObject (klass);
  	}
      }
*************** java::io::ObjectInputStream::callConstru
*** 70,71 ****
--- 72,103 ----
    _Jv_CallAnyMethodA (obj, JvPrimClass (void), meth, false, arg_types, NULL);
  }
+ 
+ java::lang::ClassLoader* 
+ java::io::ObjectInputStream::getCallersClassLoader ()
+ {
+   java::lang::ClassLoader *loader = NULL;
+   gnu::gcj::runtime::StackTrace *t 
+     = new gnu::gcj::runtime::StackTrace(4);
+   java::lang::Class *klass = NULL;
+   try
+     {
+       for (int i = 2; !klass; i++)
+ 	{
+ 	  klass = t->classAt (i);
+ 	}
+       loader = klass->getClassLoaderInternal();
+     }
+   catch (::java::lang::ArrayIndexOutOfBoundsException *e)
+     {
+       // FIXME: RuntimeError
+     }
+ 
+   return loader;
+ }
+ 
+ java::lang::ClassLoader*
+ java::io::ObjectInputStream::currentClassLoader (::java::lang::SecurityManager *sm)
+ {
+   return sm->currentClassLoader ();
+ }
+ 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]