classpath
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Object serialization and final fields


From: Guilhem Lavaux
Subject: Re: Object serialization and final fields
Date: Tue, 06 Apr 2004 21:56:44 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030630

Mark Wielaard wrote:
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 <address@hidden>

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

Ok.

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

Ok.

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

Ok, I'm modifying this.

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

It was a mistake. First I wanted to make them throw nothing because I don't really know what may happen in the VM. At some point JNI calls may raise any type of exceptions. Maybe we may catch all exceptions directly in the native code and only throw InternalError.




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.


Ok.

The mauve testcase is in CVS (added into gnu/testlet/java/io/ObjectInputOutput/Test). Here is the new patch and the new changelog entry (hoping everything is alright now):

2004-04-06  Guilhem Lavaux <address@hidden>

        * java/io/ObjectStreamField.java
        (setBooleanField, setCharField, setByteField, setShortField,
        setIntField, setLongField, setFloatField, setDoubleField,
        setObjectField): 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,
        setObjectNative): 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.

        * include/java_io_VMObjectStreamClass.h: Regenerated.

        * NEWS: Added a warning clause about the VM Interface change.

Cheers,

Guilhem.
Index: NEWS
===================================================================
RCS file: /cvsroot/classpath/classpath/NEWS,v
retrieving revision 1.36
diff -u -b -B -r1.36 NEWS
--- NEWS        13 Mar 2004 02:18:39 -0000      1.36
+++ NEWS        6 Apr 2004 19:55:20 -0000
@@ -1,3 +1,11 @@
+New in release 0.09
+
+VM Interface changes:
+
+* GNU Classpath now assumes that JNI calls SetXField can modify final
+fields. This was previously used silently for System.in/out/err and should
+be considered as a feature now. 
+
 New in release 0.08 (2004/12/03)
 
 * java.util.regexp implementation through gnu.regexp wrappers.
Index: native/jni/java-io/java_io_VMObjectStreamClass.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/java-io/java_io_VMObjectStreamClass.c,v
retrieving revision 1.3
diff -u -b -B -r1.3 java_io_VMObjectStreamClass.c
--- native/jni/java-io/java_io_VMObjectStreamClass.c    29 Mar 2004 07:07:27 
-0000      1.3
+++ native/jni/java-io/java_io_VMObjectStreamClass.c    6 Apr 2004 19:55:20 
-0000
@@ -59,3 +59,238 @@
     }
   return JNI_TRUE;
 }
+
+static void throwInternalError(JNIEnv *env)
+{
+  jclass internalErrorClass;
+  jthrowable previousException, newException;
+  jmethodID initException, getMessageID, initCauseID;
+  jstring message;
+
+  internalErrorClass = (*env)->FindClass(env, "java/lang/InternalError");
+  previousException = (*env)->ExceptionOccurred(env);
+
+  if (previousException == NULL)
+    {
+      (*env)->ThrowNew(env, internalErrorClass, "Unknown error raised by the 
VM");
+      return;
+    }
+  
+  initException = (*env)->GetMethodID
+    (env, internalErrorClass, "<init>", "(Ljava/lang/String;)V");
+  getMessageID = (*env)->GetMethodID
+    (env, (*env)->GetObjectClass(env, previousException),
+     "getMessage", "()Ljava/lang/String;");
+  initCauseID = (*env)->GetMethodID
+    (env, internalErrorClass, "initCause", "(Ljava/lang/Throwable;)V");
+
+  message = (*env)->CallObjectMethod(env, previousException,
+                                    getMessageID);
+
+  newException = (*env)->NewObject(env, internalErrorClass, initException,
+                                  message);
+  (*env)->CallVoidMethod(env, newException, initCauseID,
+                        previousException);
+
+  (*env)->ExceptionClear(env);
+  (*env)->Throw(env, newException);
+}
+
+static jfieldID getFieldReference(JNIEnv *env, jobject field, 
+                                 jobject object, const char *type)
+{
+  jclass fieldClass;
+  jclass objectClass;
+  jfieldID fid;
+  const jbyte *field_name;
+  jmethodID mid;
+  jstring name;
+
+  fieldClass = (*env)->GetObjectClass(env, field);
+
+  mid = (*env)->GetMethodID(env, fieldClass, "getName", 
"()Ljava/lang/String;");
+  if (mid == NULL || (*env)->ExceptionOccurred(env) != NULL)
+    {
+      throwInternalError(env);
+      return NULL;
+    }
+
+  name = (*env)->CallObjectMethod(env, field, mid);
+  field_name = (*env)->GetStringUTFChars(env, name, NULL);
+  
+  objectClass = (*env)->GetObjectClass(env, object);
+  fid = (*env)->GetFieldID(env, objectClass, field_name, type);
+  if (fid == NULL)
+    {
+      throwInternalError(env);
+      return NULL;
+    }
+  (*env)->ReleaseStringUTFChars(env, name, field_name);
+  
+  return fid;
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setBooleanNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;Z)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setBooleanNative( JNIEnv * env,
+                                                  jclass vmosklass,
+                                                  jobject field,
+                                                  jobject object,
+                                                  jboolean value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "Z");
+
+  if (fid != NULL)
+    (*env)->SetBooleanField(env, object, fid, value);
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setCharNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;C)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setCharNative( JNIEnv * env,
+                                               jclass vmosklass,
+                                               jobject field,
+                                               jobject object,
+                                               jchar value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "C");
+
+  if (fid != NULL)
+    (*env)->SetCharField(env, object, fid, value);
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setByteNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;B)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setByteNative( JNIEnv * env,
+                                               jclass vmosklass,
+                                               jobject field,
+                                               jobject object,
+                                               jbyte value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "B");
+
+  if (fid != NULL)
+    (*env)->SetByteField(env, object, fid, value);
+}
+
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setShortNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;S)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setShortNative( JNIEnv * env,
+                                                jclass vmosklass,
+                                                jobject field,
+                                                jobject object,
+                                                jshort value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "S");
+
+  if (fid != NULL)
+    (*env)->SetShortField(env, object, fid, value);
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setIntNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;I)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setIntNative( JNIEnv * env,
+                                              jclass vmosklass,
+                                              jobject field,
+                                              jobject object,
+                                              jint value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "I");
+
+  if (fid != NULL)
+    (*env)->SetIntField(env, object, fid, value);
+}
+
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setLongNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;J)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setLongNative( JNIEnv * env,
+                                              jclass vmosklass,
+                                              jobject field,
+                                              jobject object,
+                                              jlong value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "J");
+
+  if (fid != NULL)
+    (*env)->SetLongField(env, object, fid, value);
+}
+
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setFloatNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;F)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setFloatNative( JNIEnv * env,
+                                                jclass vmosklass,
+                                                jobject field,
+                                                jobject object,
+                                                jfloat value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "F");
+
+  if (fid != NULL)
+    (*env)->SetFloatField(env, object, fid, value);
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setDoubleNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;D)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setDoubleNative( JNIEnv * env,
+                                                jclass vmosklass,
+                                                jobject field,
+                                                jobject object,
+                                                jdouble value )
+{
+  jfieldID fid = getFieldReference (env, field, object, "D");
+
+  if (fid != NULL)
+    (*env)->SetDoubleField(env, object, fid, value);
+}
+
+/*
+ * Class:     java_io_VMObjectOutputStream
+ * Method:    setObjectNative
+ * Signature: (Ljava/lang/reflect/Field;Ljava/lang/Object;Ljava/lang/Object;)V
+ */
+JNIEXPORT void JNICALL
+Java_java_io_VMObjectStreamClass_setObjectNative( JNIEnv * env,
+                                                jclass vmosklass,
+                                                jobject field,
+                                                jobject object,
+                                                jobject value )
+{
+  jfieldID fid = getFieldReference (env, field, object, NULL);
+
+  if (fid != NULL)
+    (*env)->SetObjectField(env, object, fid, value);
+}
Index: vm/reference/java/io/VMObjectStreamClass.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/vm/reference/java/io/VMObjectStreamClass.java,v
retrieving revision 1.1
diff -u -b -B -r1.1 VMObjectStreamClass.java
--- vm/reference/java/io/VMObjectStreamClass.java       17 Jan 2003 16:45:29 
-0000      1.1
+++ vm/reference/java/io/VMObjectStreamClass.java       6 Apr 2004 19:55:20 
-0000
@@ -38,7 +38,7 @@
 
 package java.io;
 
-import java.lang.reflect.Method;
+import java.lang.reflect.Field;
 
 final class VMObjectStreamClass
 {
@@ -47,4 +47,114 @@
     * (a.k.a. <clinit>).
     */
   static native boolean hasClassInitializer (Class clazz);
+
+  /**
+   * Sets the value of the specified field. This method handles "double".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setDoubleNative(Field field, Object obj, double val)
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "float".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setFloatNative(Field field, Object obj, float val)
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "long".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setLongNative(Field field, Object obj, long val)
+    throws InternalError;
+  
+  /**
+   * Sets the value of the specified field. This method handles "int".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setIntNative(Field field, Object obj, int val) 
+    throws InternalError;
+  
+  /**
+   * Sets the value of the specified field. This method handles "short".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setShortNative(Field field, Object obj, short val) 
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "char".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setCharNative(Field field, Object obj, char val) 
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "byte".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setByteNative(Field field, Object obj, byte val) 
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "boolean".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setBooleanNative(Field field, Object obj, boolean val) 
+    throws InternalError;
+
+  /**
+   * Sets the value of the specified field. This method handles "object".
+   * Warning ! The types are not truely checked here and final values may be
+   * assigned.
+   *
+   * @param field Field to set the value.
+   * @param obj Instance which will have its field set.
+   * @param val Value to put in the field.
+   */
+  static native void setObjectNative(Field field, Object obj, Object val) 
+    throws InternalError;
+  
 }
+
Index: java/io/ObjectStreamField.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectStreamField.java,v
retrieving revision 1.14
diff -u -b -B -r1.14 ObjectStreamField.java
--- java/io/ObjectStreamField.java      26 Feb 2004 07:53:15 -0000      1.14
+++ java/io/ObjectStreamField.java      6 Apr 2004 19:55:20 -0000
@@ -38,9 +38,9 @@
 
 package java.io;
 
+import gnu.java.lang.reflect.TypeSignature;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import gnu.java.lang.reflect.TypeSignature;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 
@@ -64,7 +64,6 @@
   {
     this (field.getName(), field.getType());
     this.field = field;
-    toset = !Modifier.isFinal(field.getModifiers());
   }
 
   /**
@@ -355,109 +354,46 @@
 
   final void setBooleanField(Object obj, boolean val)
   {
-    try
-      {
-       field.setBoolean(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setBooleanNative(field, obj, val);  
   }
   
   final void setByteField(Object obj, byte val)
   {
-    try
-      {
-       field.setByte(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setByteNative(field, obj, val);
   }
   
   final void setCharField(Object obj, char val)
   {
-    try
-      {
-       field.setChar(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setCharNative(field, obj, val);
   }
   
   final void setShortField(Object obj, short val)
   {
-    try
-      {
-       field.setShort(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setShortNative(field, obj, val);
   }
   
   final void setIntField(Object obj, int val)
   {
-    try
-      {
-       field.setInt(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setIntNative(field, obj, val);
   }
   
   final void setLongField(Object obj, long val)
   {
-    try
-      {
-       field.setLong(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setLongNative(field, obj, val);
   }
   
   final void setFloatField(Object obj, float val)
   {
-    try
-      {
-       field.setFloat(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setFloatNative(field, obj, val);
   }
   
   final void setDoubleField(Object obj, double val)
   {
-    try
-      {
-       field.setDouble(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setDoubleNative(field, obj, val);
   }
   
   final void setObjectField(Object obj, Object val)
   { 
-    try
-      {
-       field.set(obj, val);
-      }
-    catch(IllegalAccessException x)
-      {
-       throw new InternalError(x.getMessage());
-      }
+    VMObjectStreamClass.setObjectNative(field, obj, val);
   }
 }

Attachment: pgpIYgBQDJRta.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]