Hi Guilhem, On Fri, 2004-04-02 at 21:52, Guilhem Lavaux wrote: > Here is the real patch for object serialization. I've added new static > methods to VMObjectStreamClass and changed the methods called in > ObjectStreamField accordingly. Note that we need to check all exceptions > now as the native functions may fail for some other obscure reasons. > > 2004-04-02 Guilhem Lavaux <[EMAIL PROTECTED]> > > * java/io/ObjectStreamField.java > (setBooleanField, setCharField, setByteField, setShortField, > setIntField, setLongField, setFloatField, setDoubleField): Use > native methods directly to be able to set final fields. > > * vm/reference/java/io/VMObjectStreamClass.java > (setBooleanNative, setCharNative, setByteNative, setShortNative, > setIntNative, setLongNative, setFloatNative, setDoubleNative): > New methods for serialization to be able to set final fields. > > * native/jni/java-io/java_io_VMObjectStreamClass.c: > Implemented new native methods of java.io.VMObjectStreamClass > accordingly. > > P.S.: If nobody is against, I'll check it in on sunday.
This looks very nice. Thanks. Some nitpicks: - You forgot the addition of setObjectNative() in the ChangeLog entry. - You should regenerate include/java_io_VMObjectStreamClass.h This is automatically done when configuring with --enable-regen-headers. But should be mentioned in the ChangeLog/patch. - Please chain exceptions. Don't just get the message, use initCause(). - Also catching Exception will miss Error (and subclasses) so you might want to catch Throwable. - But isn't it simpler to let the setXnative() methods throw InternalError() directly (or maybe VirtualMachineError or IllegalArgumentException which seem more appropriate). - Then also don't declare the setXNative() methods throws IllegalAccessException. Some are declared as such and some not. But it seems this is the only exception that should never be thrown. Could you also add a short notice to the NEWS file about this VM interface change. You should at least mention that the standard implementation assumes that the JNI setXField() methods can be used on final fields. BTW. Note to VM/Compiler implementers! -------------------------------------- We discussed "final" fields a bit more on irc. We came to the conclusion the the VM spec authors and the JLS spec authors have a different interpretation of "final". According to the VM spec a final field is non-private read-only, but rewritable by the Class defining the field. According the the JLS spec a final field is single assignable (in a [static] constructor method). JNI doesn't seem to define precisely what/how final fields can change. The above patch depends on the fact that in practice JNI setXField() can be used to set a final field (this was already the case for the System.setIn()/setOut()/setError() methods). The following bug reports are also interesting: http://developer.java.sun.com/developer/bugParade/bugs/4027808.html Synopsis: [JNI] SetObjectField modifies a final field value State: Closed, will not be fixed Evaluation: JNI does not guard against illegal access to objects [...]. http://developer.java.sun.com/developer/bugParade/bugs/4115974.html Synopsis: Need to be able to tell JIT not to optimize certain finals. State: Closed, will not be fixed Evaluation: These are the only thing which should ever behave this way, so there's no need for a special interface for this. The JIT just needs to handle these specially. http://developer.java.sun.com/developer/bugParade/bugs/4174797.html Synopsis: Serialization should not use reflection to set final fields State: Closed, fixed Evaluation: [...] Serialization needs to use JNI Set<type>Field Routines to set fields, instead of Field.set*(). [...] All this seems a bit unfortunate since it basically means that a runtime cannot optimize final field access since they can always be changed through JNI and this "feature" is at least necessary for both System.setIn/Out/Err() and reflection. Cheers, Mark
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath