[cp-patches] ObjectInputStream currentLoader()

[Mark Wielaard]

Hi,

This is an unfortunate patch so late before the release. But the latest
shuffle of the currentLoader() method from ObjectInputStream to
VMObjectInputStream broke serialization involving user defined class
loaders. The use of SecurityManager.getCurrentClassLoader() was wrong to
start with. It just happened to work. But the semantics of that method
are not really what is needed in ObjectInputStream (for example
SecurityManager.getCurrentClassLoader() returns null is
checkPermission(new AllPermission) succeeds!). So this patch introduces
a new currentLoader() vm interface helper method plus reference
implementation that doesn't rely on SecurityManager.

2005-04-30  Mark Wielaard  <address@hidden>

        * java/io/ObjectInputStream.java
        (currentLoader): Don't create SecurityManager, directly call
        VMObjectInputStream.currentClassLoader().
        (resolveProxyClass): Use currentLoader().
        * vm/reference/java/io/VMObjectInputStream.java
        (currentClassLoader(SecurityManager)): Removed.
        (currentClassLoader): New method.
        * native/jni/java-io/java_io_VMObjectInputStream.c
        (Java_java_io_VMObjectInputStream_currentClassLoader): Removed.
        * include/java_io_VMObjectInputStream.h: Regenerated.

        * NEWS: Document new interface and reference implementation.

With this patch applied all the serialization regressions we were seeing
in mauve have been fixed when using the reference implementation of
VMObjectInputStream.

Cheers,

Mark

Index: NEWS
===================================================================
RCS file: /cvsroot/classpath/classpath/NEWS,v
retrieving revision 1.75
diff -u -r1.75 NEWS
--- NEWS        29 Apr 2005 20:34:08 -0000      1.75
+++ NEWS        30 Apr 2005 13:45:49 -0000
@@ -43,6 +43,9 @@
   to define the location of the JNI libraries. It is by all means meant
   ONLY for VM implementors and GNU Classpath hackers. See the hacking
   guide for more information.
+* The helper methods currentLoader() and allocateObject() for
+  java.io.ObjectInputStream have been moved to a VMObjectInputStream class.
+  Reference implementations are provided.
 
 Other changes:
 
Index: include/java_io_VMObjectInputStream.h
===================================================================
RCS file: /cvsroot/classpath/classpath/include/java_io_VMObjectInputStream.h,v
retrieving revision 1.1
diff -u -r1.1 java_io_VMObjectInputStream.h
--- include/java_io_VMObjectInputStream.h       16 Apr 2005 11:05:07 -0000      
1.1
+++ include/java_io_VMObjectInputStream.h       30 Apr 2005 13:45:50 -0000
@@ -10,7 +10,6 @@
 {
 #endif
 
-JNIEXPORT jobject JNICALL Java_java_io_VMObjectInputStream_currentClassLoader 
(JNIEnv *env, jclass, jobject);
 JNIEXPORT jobject JNICALL Java_java_io_VMObjectInputStream_allocateObject 
(JNIEnv *env, jclass, jclass, jclass, jobject);
 
 #ifdef __cplusplus
Index: java/io/ObjectInputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/ObjectInputStream.java,v
retrieving revision 1.55
diff -u -r1.55 ObjectInputStream.java
--- java/io/ObjectInputStream.java      16 Apr 2005 11:05:07 -0000      1.55
+++ java/io/ObjectInputStream.java      30 Apr 2005 13:45:50 -0000
@@ -783,20 +783,12 @@
   }
 
   /**
-   * This method invokes the method currentClassLoader for the
-   * current security manager (or build an empty one if it is not
-   * present).
-   *
-   * @return The most recent non-system ClassLoader on the execution stack.
-   * @see java.lang.SecurityManager#currentClassLoader()
+   * Returns he most recent user defined ClassLoader on the execution stack
+   * or null of none is found.
    */
   private ClassLoader currentLoader()
   {
-    SecurityManager sm = System.getSecurityManager();
-    if (sm == null)
-      sm = new SecurityManager () {};
-    
-    return VMObjectInputStream.currentClassLoader(sm);
+    return VMObjectInputStream.currentClassLoader();
   }
 
   /**
@@ -883,12 +875,7 @@
   protected Class resolveProxyClass(String[] intfs)
     throws IOException, ClassNotFoundException
   {
-    SecurityManager sm = System.getSecurityManager();
-    
-    if (sm == null)
-      sm = new SecurityManager() {};
-    
-    ClassLoader cl = VMObjectInputStream.currentClassLoader(sm);
+    ClassLoader cl = currentLoader();
     
     Class[] clss = new Class[intfs.length];
     if(cl == null)
Index: native/jni/java-io/java_io_VMObjectInputStream.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/java-io/java_io_VMObjectInputStream.c,v
retrieving revision 1.1
diff -u -r1.1 java_io_VMObjectInputStream.c
--- native/jni/java-io/java_io_VMObjectInputStream.c    16 Apr 2005 11:05:08 
-0000      1.1
+++ native/jni/java-io/java_io_VMObjectInputStream.c    30 Apr 2005 13:45:50 
-0000
@@ -49,28 +49,6 @@
 
 /*
  * Class:     java_io_VMObjectInputStream
- * Method:    currentClassLoader
- * Signature: (Ljava/lang/SecurityManager;)Ljava/lang/ClassLoader;
- */
-JNIEXPORT jobject JNICALL
-Java_java_io_VMObjectInputStream_currentClassLoader (JNIEnv * env,
-                                                  jclass clazz,
-                                                  jobject loader)
-{
-  jmethodID id = (*env)->GetMethodID (env,
-                                     (*env)->GetObjectClass (env, loader),
-                                     "currentClassLoader",
-                                     "()Ljava/lang/ClassLoader;");
-
-  if (id == NULL)
-    return NULL;
-
-  return (*env)->CallObjectMethod (env, loader, id, clazz);
-}
-
-
-/*
- * Class:     java_io_VMObjectInputStream
  * Method:    allocateObject
  * Signature: (Ljava/lang/Class;)Ljava/lang/Object;
  */
Index: vm/reference/java/io/VMObjectInputStream.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/vm/reference/java/io/VMObjectInputStream.java,v
retrieving revision 1.1
diff -u -r1.1 VMObjectInputStream.java
--- vm/reference/java/io/VMObjectInputStream.java       16 Apr 2005 11:05:08 
-0000      1.1
+++ vm/reference/java/io/VMObjectInputStream.java       30 Apr 2005 13:45:50 
-0000
@@ -39,27 +39,48 @@
 
 package java.io;
 
-import gnu.classpath.Configuration;
-import gnu.java.io.ObjectIdentityWrapper;
-
-import java.lang.reflect.Array;
+import gnu.classpath.VMStackWalker;
 import java.lang.reflect.Constructor;
-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.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.Arrays;
-import java.util.Hashtable;
-import java.util.Vector;
 
 final class VMObjectInputStream
 {
-  static native ClassLoader currentClassLoader(SecurityManager sm);
+  private static Class oisClass = ObjectInputStream.class;
+  private static Class vmoisClass = VMObjectInputStream.class;
+
+  // PrivilegedAction needed for Class.getClassLoader()
+  private static PrivilegedAction loaderAction = new PrivilegedAction()
+    {
+      public Object run()
+      {
+       Class[] ctx = VMStackWalker.getClassContext();
+       for (int i = 0; i < ctx.length; i++)
+         {
+           ClassLoader cl = ctx[i].getClassLoader();
+           if (cl != null)
+             return cl;
+         }
+       return null;
+      }
+    };
 
-  static native Object allocateObject(Class clazz, Class constr_clazz, 
Constructor constructor)
+  /**
+   * Returns the first user defined class loader on the call stack, or
+   * null when no non-null class loader was found.
+   */
+  static ClassLoader currentClassLoader()
+  {
+    return (ClassLoader) AccessController.doPrivileged(loaderAction);
+  }
+
+  /**
+   * Allocates a new Object of type clazz but without running the
+   * default constructor on it. It then calls the given constructor on
+   * it. The given constructor method comes from the constr_clazz
+   * which is a super class of the given clazz.
+   */
+  static native Object allocateObject(Class clazz, Class constr_clazz,
+                                     Constructor constructor)
     throws InstantiationException;
 }
-

signature.asc
Description: This is a digitally signed message part