Serialization bug fixes
Andrew Haley
aph@redhat.com
Thu Jun 17 15:13:00 GMT 2004
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 ();
+ }
+
More information about the Java
mailing list