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 (); + } + _______________________________________________ Classpath mailing list [EMAIL PROTECTED] http://lists.gnu.org/mailman/listinfo/classpath