Hi, I did some RMI debugging and I found that we're using the wrong class loader in a couple of cases. The attached patch cleans up the code a little and uses the caller's class loader instead of the thread's context class loader in a couple of places.
I would appreciate it if someone with more RMI knowledge could review this. Note that I removed VMObjectInputStream.currentClassLoader() and added VMStackWalker.firstNonNullClassLoader(). They are not identical, because VMObjectInputStream.currentClassLoader() used to return the thread context class loader if it couldn't find any non-null class loader on the stack, but firstNonNullClassLoader() doesn't do this. I could think of no good reason for the old behavior, so I removed it. Thanks, Jeroen
Index: gnu/java/rmi/server/RMIClassLoaderImpl.java =================================================================== RCS file: /cvsroot/classpath/classpath/gnu/java/rmi/server/RMIClassLoaderImpl.java,v retrieving revision 1.1 diff -u -r1.1 RMIClassLoaderImpl.java --- gnu/java/rmi/server/RMIClassLoaderImpl.java 28 Sep 2005 19:51:31 -0000 1.1 +++ gnu/java/rmi/server/RMIClassLoaderImpl.java 13 Aug 2006 11:22:24 -0000 @@ -1,5 +1,5 @@ /* RMIClassLoaderImpl.java -- FIXME: briefly describe file purpose - Copyright (C) 2005 Free Software Foundation, Inc. + Copyright (C) 2005, 2006 Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -38,6 +38,7 @@ package gnu.java.rmi.server; +import java.lang.reflect.Proxy; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; @@ -186,6 +187,7 @@ { defaultClassLoader = new MyClassLoader (new URL[] { defaultCodebase }, null, defaultAnnotation); + // XXX using getContextClassLoader here *cannot* be right cacheLoaders.put (new CacheKey (defaultAnnotation, Thread.currentThread().getContextClassLoader()), defaultClassLoader); @@ -216,47 +218,53 @@ ClassLoader defaultLoader) throws MalformedURLException, ClassNotFoundException { - ClassLoader loader; - if (defaultLoader == null) - loader = Thread.currentThread().getContextClassLoader(); - else - loader = defaultLoader; - - //try context class loader first try { - return Class.forName(name, false, loader); + if (defaultLoader != null) + return Class.forName(name, false, defaultLoader); } catch (ClassNotFoundException e) { - // class not found in the local classpath + } + + return Class.forName(name, false, getClassLoader(codeBase)); + } + + public Class loadProxyClass(String codeBase, String[] interfaces, + ClassLoader defaultLoader) + throws MalformedURLException, ClassNotFoundException + { + Class clss[] = new Class[interfaces.length]; + + for (int i = 0; i < interfaces.length; i++) + { + clss[i] = loadClass(codeBase, interfaces[i], defaultLoader); } - if (codeBase.length() == 0) //=="" + // Chain all class loaders (they may differ). + ArrayList loaders = new ArrayList(clss.length); + ClassLoader loader = null; + for (int i = 0; i < clss.length; i++) { - loader = defaultClassLoader; + loader = clss[i].getClassLoader(); + if (! loaders.contains(loader)) + { + loaders.add(0, loader); + } } - else + if (loaders.size() > 1) { - loader = getClassLoader(codeBase); + loader = new CombinedClassLoader(loaders); } - if (loader == null) + try { - //do not throw NullPointerException - throw new ClassNotFoundException ("Could not find class (" + name + - ") at codebase (" + codeBase + ")"); + return Proxy.getProxyClass(loader, clss); + } + catch (IllegalArgumentException e) + { + throw new ClassNotFoundException(null, e); } - - return Class.forName(name, false, loader); - } - - public Class loadProxyClass(String codeBase, String[] interfaces, - ClassLoader defaultLoader) - throws MalformedURLException, ClassNotFoundException - { - // FIXME: Implement this. - return null; } /** @@ -272,6 +280,9 @@ public ClassLoader getClassLoader(String codebase) throws MalformedURLException { + if (codebase == null || codebase.length() == 0) + return Thread.currentThread().getContextClassLoader(); + ClassLoader loader; CacheKey loaderKey = new CacheKey (codebase, Thread.currentThread().getContextClassLoader()); Index: gnu/java/rmi/server/RMIObjectInputStream.java =================================================================== RCS file: /cvsroot/classpath/classpath/gnu/java/rmi/server/RMIObjectInputStream.java,v retrieving revision 1.10 diff -u -r1.10 RMIObjectInputStream.java --- gnu/java/rmi/server/RMIObjectInputStream.java 20 Feb 2006 08:49:37 -0000 1.10 +++ gnu/java/rmi/server/RMIObjectInputStream.java 13 Aug 2006 11:23:51 -0000 @@ -1,5 +1,5 @@ /* RMIObjectInputStream.java -- - Copyright (c) 1996, 1997, 1998, 1999, 2002, 2004 + Copyright (c) 1996, 1997, 1998, 1999, 2002, 2004, 2006 Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -39,11 +39,11 @@ package gnu.java.rmi.server; +import gnu.classpath.VMStackWalker; import java.io.IOException; import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectStreamClass; -import java.lang.reflect.Proxy; import java.net.MalformedURLException; import java.rmi.server.RMIClassLoader; import java.util.ArrayList; @@ -57,16 +57,14 @@ } protected Class resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException { - String annotation = (String)getAnnotation(); - try { - if(annotation == null) - return (RMIClassLoader.loadClass(desc.getName())); - else - return (RMIClassLoader.loadClass(annotation, desc.getName())); + return RMIClassLoader.loadClass( + (String)getAnnotation(), + desc.getName(), + VMStackWalker.firstNonNullClassLoader()); } - catch (MalformedURLException _) { - throw new ClassNotFoundException(desc.getName()); + catch (MalformedURLException x) { + throw new ClassNotFoundException(desc.getName(), x); } } @@ -81,45 +79,16 @@ protected Class resolveProxyClass(String intfs[]) throws IOException, ClassNotFoundException { - String annotation = (String) getAnnotation(); - - Class clss[] = new Class[intfs.length]; - - for (int i = 0; i < intfs.length; i++) - { - if (annotation == null) - clss[i] = RMIClassLoader.loadClass(intfs[i]); - else - clss[i] = RMIClassLoader.loadClass(annotation, intfs[i]); - } - - ClassLoader loader; - - if (clss.length > 0) - { - // Chain all class loaders (they may differ). - ArrayList loaders = new ArrayList(intfs.length); - ClassLoader cx; - for (int i = 0; i < clss.length; i++) - { - cx = clss[i].getClassLoader(); - if (!loaders.contains(cx)) - { - loaders.add(0, cx); - } - } - loader = new CombinedClassLoader(loaders); - } - else - loader = ClassLoader.getSystemClassLoader(); - - try + try { - return Proxy.getProxyClass(loader, clss); + return RMIClassLoader.loadProxyClass( + (String)getAnnotation(), + intfs, + VMStackWalker.firstNonNullClassLoader()); } - catch (IllegalArgumentException e) + catch (MalformedURLException x) { - throw new ClassNotFoundException(null, e); + throw new ClassNotFoundException(null, x); } } Index: gnu/javax/rmi/CORBA/UtilDelegateImpl.java =================================================================== RCS file: /cvsroot/classpath/classpath/gnu/javax/rmi/CORBA/UtilDelegateImpl.java,v retrieving revision 1.9 diff -u -r1.9 UtilDelegateImpl.java --- gnu/javax/rmi/CORBA/UtilDelegateImpl.java 6 Nov 2005 11:32:32 -0000 1.9 +++ gnu/javax/rmi/CORBA/UtilDelegateImpl.java 13 Aug 2006 11:25:31 -0000 @@ -1,5 +1,5 @@ /* UtilDelegateImpl.java -- - Copyright (C) 2002, 2005 Free Software Foundation, Inc. + Copyright (C) 2002, 2005, 2006 Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -38,6 +38,8 @@ package gnu.javax.rmi.CORBA; +import gnu.classpath.VMStackWalker; + import gnu.CORBA.Minor; import gnu.CORBA.ObjectCreator; import gnu.CORBA.Poa.ORB_1_4; @@ -70,6 +72,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; +import java.net.MalformedURLException; import java.rmi.AccessException; import java.rmi.MarshalException; import java.rmi.NoSuchObjectException; @@ -374,37 +377,24 @@ throws ClassNotFoundException { if (loader == null) - loader = Thread.currentThread().getContextClassLoader(); + loader = VMStackWalker.firstNonNullClassLoader(); String p_useCodebaseOnly = System.getProperty("java.rmi.server.useCodebaseOnly"); boolean useCodebaseOnly = p_useCodebaseOnly != null && p_useCodebaseOnly.trim().equalsIgnoreCase("true"); - try - { - if (remoteCodebase != null && !useCodebaseOnly) - return RMIClassLoader.loadClass(remoteCodebase, className); - } - catch (Exception e) - { - // This failed but try others. - } + if (useCodebaseOnly) + remoteCodebase = null; try { - if (remoteCodebase == null || useCodebaseOnly) - return RMIClassLoader.loadClass(remoteCodebase, className); + return RMIClassLoader.loadClass(remoteCodebase, className, loader); } - catch (Exception e) + catch (MalformedURLException x) { - // This failed but try others. + throw new ClassNotFoundException(className, x); } - - if (loader != null) - return Class.forName(className, true, loader); - - throw new ClassNotFoundException(className + " at " + remoteCodebase); } /** Index: java/io/ObjectInputStream.java =================================================================== RCS file: /cvsroot/classpath/classpath/java/io/ObjectInputStream.java,v retrieving revision 1.77 diff -u -r1.77 ObjectInputStream.java --- java/io/ObjectInputStream.java 11 Aug 2006 07:56:07 -0000 1.77 +++ java/io/ObjectInputStream.java 13 Aug 2006 07:35:28 -0000 @@ -39,6 +39,7 @@ package java.io; +import gnu.classpath.VMStackWalker; import gnu.java.io.ObjectIdentityWrapper; import java.lang.reflect.Array; @@ -817,7 +818,7 @@ */ private ClassLoader currentLoader() { - return VMObjectInputStream.currentClassLoader(); + return VMStackWalker.firstNonNullClassLoader(); } /** Index: vm/reference/gnu/classpath/VMStackWalker.java =================================================================== RCS file: /cvsroot/classpath/classpath/vm/reference/gnu/classpath/VMStackWalker.java,v retrieving revision 1.6 diff -u -r1.6 VMStackWalker.java --- vm/reference/gnu/classpath/VMStackWalker.java 13 Nov 2005 22:29:45 -0000 1.6 +++ vm/reference/gnu/classpath/VMStackWalker.java 13 Aug 2006 11:26:24 -0000 @@ -1,5 +1,5 @@ /* VMStackWalker.java -- Reference implementation of VM hooks for stack access - Copyright (C) 2005 Free Software Foundation + Copyright (C) 2005, 2006 Free Software Foundation This file is part of GNU Classpath. @@ -112,5 +112,20 @@ * is here to work around access permissions. */ public static native ClassLoader getClassLoader(Class cl); -} + /** + * Walk up the stack and return the first non-null class loader. + * If there aren't any non-null class loaders on the stack, return null. + */ + public static ClassLoader firstNonNullClassLoader() + { + Class[] stack = getClassContext(); + for (int i = 0; i < stack.length; i++) + { + ClassLoader loader = getClassLoader(stack[i]); + if (loader != null) + return loader; + } + return null; + } +} Index: vm/reference/java/io/VMObjectInputStream.java =================================================================== RCS file: /cvsroot/classpath/classpath/vm/reference/java/io/VMObjectInputStream.java,v retrieving revision 1.5 diff -u -r1.5 VMObjectInputStream.java --- vm/reference/java/io/VMObjectInputStream.java 27 Jan 2006 16:49:15 -0000 1.5 +++ vm/reference/java/io/VMObjectInputStream.java 13 Aug 2006 11:16:52 -0000 @@ -40,10 +40,7 @@ package java.io; import gnu.classpath.Configuration; -import gnu.classpath.VMStackWalker; import java.lang.reflect.Constructor; -import java.security.AccessController; -import java.security.PrivilegedAction; final class VMObjectInputStream { @@ -56,42 +53,6 @@ } /** - * PrivilegedAction needed for Class.getClassLoader() - */ - private static PrivilegedAction loaderAction = new PrivilegedAction() - { - /** - * Returns the first user defined class loader on the call stack, or the - * context class loader of the current thread, when no non-null class loader - * was found. - */ - 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 Thread.currentThread().getContextClassLoader(); - } - }; - - /** - * Returns the first user defined class loader on the call stack, or the - * context class loader of the current thread, when no non-null class loader - * was found. - * - * @return the class loader - */ - 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