Re: svn commit: r518426 - in /geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel: classloader/ config/

Thu, 29 Mar 2007 07:13:50 -0800 


On Mar 29, 2007, at 6:58 AM, Donald Woods wrote:


David, the below change to IOUtil.java -
  -    return Collections.singleton(match.toURL());
+ return Collections.singleton(match.toURI().normalize().toURL ()); looks like it is causing the G3016 reported failure, that the server will no longer start on Windows when there is a white space in the GERONIMO_HOME path.
Was there some reason we need to normalize the URI before its converted to a URL?
The problem I was trying to solve is that if you have manifest classpatch entries in a war in an ear (that refer to stuff outside the war, in the ear), and you try webClassLoader.getResources("/META- INF") you get a lot of copies of urls from those mcp jars , except most of them aren't normailzed..... <pathToEar>/myWebApp.war/../ lib/foo/foo.jar!/META-INF but some of them are. This confuses a bunch of other apps that expect that if there are duplicate URLS, they have the same path. 

This is the main complaint in GERONIMO-2693

Can you figure out something that works for both  problems at once?

thanks
david jencks





-Donald


[EMAIL PROTECTED] wrote:

Author: djencks
Date: Wed Mar 14 18:22:56 2007
New Revision: 518426
URL: http://svn.apache.org/viewvc?view=rev&rev=518426
Log:
GERONIMO-2693 eliminate most duplicate entries from getResources. This should also produce only normailized urls. This should make jetty start quickly 
Modified:
geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/JarFileClassLoader.java geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UnionEnumeration.java geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UrlResourceFinder.java geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/IOUtil.java geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/MultiParentClassLoader.java Modified: geronimo/server/trunk/modules/geronimo-kernel/src/main/ java/org/apache/geronimo/kernel/classloader/JarFileClassLoader.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/ geronimo-kernel/src/main/java/org/apache/geronimo/kernel/ classloader/JarFileClassLoader.java? view=diff&rev=518426&r1=518425&r2=518426 ===================================================================== ========= --- geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/JarFileClassLoader.java (original) +++ geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/JarFileClassLoader.java Wed Mar 14 18:22:56 2007 
@@ -184,6 +184,7 @@
     /**
      * [EMAIL PROTECTED]
      */
+/*
public Enumeration findResources(final String resourceName) throws IOException { 
         // todo this is not right
         // first get the resources from the parent classloaders
@@ -199,6 +200,15 @@
         // join the two together
Enumeration resources = new UnionEnumeration (parentResources, myResources); 
         return resources;
+    }
+*/
+
+ protected Enumeration<URL> internalfindResources(final String name) throws IOException { + return AccessController.doPrivileged(new PrivilegedAction<Enumeration<URL>>() { 
+            public Enumeration<URL> run() {
+                return resourceFinder.findResources(name);
+            }
+        }, acc);
     }
      /**
Modified: geronimo/server/trunk/modules/geronimo-kernel/src/main/ java/org/apache/geronimo/kernel/classloader/UnionEnumeration.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/ geronimo-kernel/src/main/java/org/apache/geronimo/kernel/ classloader/UnionEnumeration.java? view=diff&rev=518426&r1=518425&r2=518426 ===================================================================== ========= --- geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UnionEnumeration.java (original) +++ geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UnionEnumeration.java Wed Mar 14 18:22:56 2007 
@@ -24,14 +24,14 @@
 /**
  * @version $Rev$ $Date$
  */
-public final class UnionEnumeration implements Enumeration {
-    private final LinkedList enumerations = new LinkedList();
+public final class UnionEnumeration<T> implements Enumeration<T> {
+ private final LinkedList<Enumeration<T>> enumerations = new LinkedList<Enumeration<T>>(); 
 -    public UnionEnumeration(List enumerations) {
+    public UnionEnumeration(List<Enumeration<T>> enumerations) {
         this.enumerations.addAll(enumerations);
     }
- public UnionEnumeration(Enumeration first, Enumeration second) { + public UnionEnumeration(Enumeration<T> first, Enumeration<T> second) { if (first == null) throw new NullPointerException("first is null"); if (second == null) throw new NullPointerException ("second is null"); 
 @@ -41,7 +41,7 @@
      public boolean hasMoreElements() {
         while (!enumerations.isEmpty()) {
- Enumeration enumeration = (Enumeration) enumerations.getFirst(); 
+            Enumeration enumeration = enumerations.getFirst();
             if (enumeration.hasMoreElements()) {
                 return true;
             }
@@ -50,9 +50,9 @@
         return false;
     }
 -    public Object nextElement() {
+    public T nextElement() {
         while (!enumerations.isEmpty()) {
- Enumeration enumeration = (Enumeration) enumerations.getFirst(); 
+            Enumeration<T> enumeration = enumerations.getFirst();
             if (enumeration.hasMoreElements()) {
                 return enumeration.nextElement();
             }
Modified: geronimo/server/trunk/modules/geronimo-kernel/src/main/ java/org/apache/geronimo/kernel/classloader/UrlResourceFinder.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/ geronimo-kernel/src/main/java/org/apache/geronimo/kernel/ classloader/UrlResourceFinder.java? view=diff&rev=518426&r1=518425&r2=518426 ===================================================================== ========= --- geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UrlResourceFinder.java (original) +++ geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/classloader/UrlResourceFinder.java Wed Mar 14 18:22:56 2007 
@@ -25,7 +25,6 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
@@ -41,9 +40,9 @@
 public class UrlResourceFinder implements ResourceFinder {
     private final Object lock = new Object();
 -    private final LinkedHashSet urls = new LinkedHashSet();
-    private final LinkedHashMap classPath = new LinkedHashMap();
-    private final LinkedHashSet watchedFiles = new LinkedHashSet();
+ private final LinkedHashSet<URL> urls = new LinkedHashSet<URL> (); + private final LinkedHashMap<URL,ResourceLocation> classPath = new LinkedHashMap<URL,ResourceLocation>(); + private final LinkedHashSet<File> watchedFiles = new LinkedHashSet<File>(); 
      private boolean destroyed = false;
 @@ -61,8 +60,7 @@
             }
             destroyed = true;
             urls.clear();
- for (Iterator iterator = classPath.values().iterator (); iterator.hasNext();) { - ResourceLocation resourceLocation = (ResourceLocation) iterator.next(); + for (ResourceLocation resourceLocation : classPath.values()) { 
                 resourceLocation.close();
             }
             classPath.clear();
@@ -74,9 +72,8 @@
             if (destroyed) {
                 return null;
             }
- for (Iterator iterator = getClassPath().entrySet ().iterator(); iterator.hasNext();) { 
-                Map.Entry entry = (Map.Entry) iterator.next();
- ResourceLocation resourceLocation = (ResourceLocation) entry.getValue(); + for (Map.Entry<URL, ResourceLocation> entry : getClassPath().entrySet()) { + ResourceLocation resourceLocation = entry.getValue (); ResourceHandle resourceHandle = resourceLocation.getResourceHandle(resourceName); if (resourceHandle != null && ! resourceHandle.isDirectory()) { 
                     return resourceHandle;
@@ -91,9 +88,8 @@
             if (destroyed) {
                 return null;
             }
- for (Iterator iterator = getClassPath().entrySet ().iterator(); iterator.hasNext();) { 
-                Map.Entry entry = (Map.Entry) iterator.next();
- ResourceLocation resourceLocation = (ResourceLocation) entry.getValue(); + for (Map.Entry<URL, ResourceLocation> entry : getClassPath().entrySet()) { + ResourceLocation resourceLocation = entry.getValue (); ResourceHandle resourceHandle = resourceLocation.getResourceHandle(resourceName); 
                 if (resourceHandle != null) {
                     return resourceHandle.getUrl();
@@ -105,7 +101,7 @@
      public Enumeration findResources(String resourceName) {
         synchronized (lock) {
- return new ResourceEnumeration(new ArrayList (getClassPath().values()), resourceName); + return new ResourceEnumeration(new ArrayList<ResourceLocation>(getClassPath().values()), resourceName); 
         }
     }
 @@ -115,7 +111,7 @@
      public URL[] getUrls() {
         synchronized (lock) {
-            return (URL[]) urls.toArray(new URL[urls.size()]);
+            return urls.toArray(new URL[urls.size()]);
         }
     }
 @@ -131,7 +127,7 @@
      * Adds a list of urls to the end of this class loader.
      * @param urls the URLs to add
      */
-    protected void addUrls(List urls) {
+    protected void addUrls(List<URL> urls) {
         synchronized (lock) {
             if (destroyed) {
throw new IllegalStateException ("UrlResourceFinder has been destroyed"); 
@@ -144,11 +140,10 @@
         }
     }
 -    private LinkedHashMap getClassPath() {
+    private LinkedHashMap<URL, ResourceLocation> getClassPath() {
assert Thread.holdsLock(lock): "This method can only be called while holding the lock"; - for (Iterator iterator = watchedFiles.iterator(); iterator.hasNext();) { 
-            File file = (File) iterator.next();
+        for (File file : watchedFiles) {
             if (file.canRead()) {
                 rebuildClassPath();
                 break;
@@ -167,21 +162,21 @@
assert Thread.holdsLock(lock): "This method can only be called while holding the lock"; // copy all of the existing locations into a temp map and clear the class path 
-        Map existingJarFiles = new LinkedHashMap(classPath);
+ Map<URL,ResourceLocation> existingJarFiles = new LinkedHashMap<URL,ResourceLocation>(classPath); 
         classPath.clear();
 -        LinkedList locationStack = new LinkedList(urls);
+        LinkedList<URL> locationStack = new LinkedList<URL>(urls);
         try {
             while (!locationStack.isEmpty()) {
-                URL url = (URL) locationStack.removeFirst();
+                URL url = locationStack.removeFirst();
 -                // Skip any duplicate urls in the claspath
+                // Skip any duplicate urls in the classpath
                 if (classPath.containsKey(url)) {
                     continue;
                 }
                  // Check is this URL has already been opened
- ResourceLocation resourceLocation = (ResourceLocation) existingJarFiles.remove(url); + ResourceLocation resourceLocation = existingJarFiles.remove(url); // If not opened, cache the url and wrap it with a resource location 
                 if (resourceLocation == null) {
@@ -208,7 +203,7 @@
classPath.put(resourceLocation.getCodeSource(), resourceLocation); // push the manifest classpath on the stack (make sure to maintain the order) - List manifestClassPath = getManifestClassPath (resourceLocation); + List<URL> manifestClassPath = getManifestClassPath (resourceLocation); 
                 locationStack.addAll(0, manifestClassPath);
             }
         } catch (Error e) {
@@ -216,8 +211,7 @@
             throw e;
         }
- for (Iterator iterator = existingJarFiles.values ().iterator(); iterator.hasNext();) { - ResourceLocation resourceLocation = (ResourceLocation) iterator.next(); + for (ResourceLocation resourceLocation : existingJarFiles.values()) { 
             resourceLocation.close();
         }
     }
@@ -246,7 +240,7 @@
throw new IOException("File is not readable: " + cacheFile.getAbsolutePath()); 
         }
 -        ResourceLocation resourceLocation = null;
+        ResourceLocation resourceLocation;
         if (cacheFile.isDirectory()) {
// DirectoryResourceLocation will only return "file" URLs within this directory // do not user the DirectoryResourceLocation for non file based urls 
@@ -257,7 +251,7 @@
         return resourceLocation;
     }
- private List getManifestClassPath(ResourceLocation resourceLocation) { + private List<URL> getManifestClassPath(ResourceLocation resourceLocation) { 
         try {
             // get the manifest, if possible
             Manifest manifest = resourceLocation.getManifest();
@@ -275,7 +269,7 @@
             // build the urls...
             // the class-path attribute is space delimited
             URL codeSource = resourceLocation.getCodeSource();
-            LinkedList classPathUrls = new LinkedList();
+            LinkedList<URL> classPathUrls = new LinkedList<URL>();
for (StringTokenizer tokenizer = new StringTokenizer (manifestClassPath, " "); tokenizer.hasMoreTokens();) { 
                 String entry = tokenizer.nextToken();
                 try {
Modified: geronimo/server/trunk/modules/geronimo-kernel/src/main/ java/org/apache/geronimo/kernel/config/IOUtil.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/ geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/ IOUtil.java?view=diff&rev=518426&r1=518425&r2=518426 ===================================================================== ========= --- geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/IOUtil.java (original) +++ geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/IOUtil.java Wed Mar 14 18:22:56 2007 
@@ -189,7 +189,7 @@
             if (!SelectorUtils.hasWildcards(pattern)) {
                 File match = new File(root, pattern);
                 if (match.exists() && match.canRead()) {
-                    return Collections.singleton(match.toURL());
+ return Collections.singleton(match.toURI ().normalize().toURL()); 
                 } else {
                     return Collections.EMPTY_SET;
                 }
@@ -201,7 +201,7 @@
                     String fileName = (String) entry.getKey();
if (SelectorUtils.matchPath(pattern, fileName)) { 
                         File file = (File) entry.getValue();
-                        matches.add(file.toURL());
+ matches.add(file.toURI().normalize().toURL ()); 
                     }
                 }
                 return matches;
Modified: geronimo/server/trunk/modules/geronimo-kernel/src/main/ java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/ geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/ MultiParentClassLoader.java?view=diff&rev=518426&r1=518425&r2=518426 ===================================================================== ========= --- geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/MultiParentClassLoader.java (original) +++ geronimo/server/trunk/modules/geronimo-kernel/src/main/java/ org/apache/geronimo/kernel/config/MultiParentClassLoader.java Wed Mar 14 18:22:56 2007 
@@ -29,10 +29,13 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
  import org.apache.commons.logging.LogFactory;
+import org.apache.geronimo.kernel.classloader.UnionEnumeration;
 import org.apache.geronimo.kernel.repository.Artifact;
 import org.apache.geronimo.kernel.util.ClassLoaderRegistry;
 @@ -145,7 +148,7 @@
         if (source instanceof MultiParentClassLoader) {
return new MultiParentClassLoader ((MultiParentClassLoader) source); 
         } else if (source instanceof URLClassLoader) {
- return new URLClassLoader(((URLClassLoader) source).getURLs(), source.getParent()); + return new URLClassLoader(((URLClassLoader) source).getURLs(), source.getParent()); 
         } else {
             return new URLClassLoader(new URL[0], source);
         }
@@ -175,7 +178,7 @@
      */
public MultiParentClassLoader(Artifact id, URL[] urls, ClassLoader[] parents, URLStreamHandlerFactory factory) { 
         super(urls, null, factory);
-        this.id = id;        +        this.id = id;
         this.parents = copyParents(parents);
         inverseClassLoading = false;
         hiddenClasses = new String[0];
@@ -205,7 +208,7 @@
throw new NullPointerException("parent[" + i + "] is null"); 
             }
             if (parent instanceof MultiParentClassLoader) {
-                parent = ((MultiParentClassLoader)parent).copy();
+                parent = ((MultiParentClassLoader) parent).copy();
             }
             newParentsArray[i] = parent;
         }
@@ -243,23 +246,23 @@
         if (cachedClass != null) {
             return resolveClass(cachedClass, resolve);
         }
-        +
// This is a reasonable hack. We can add some classes to the list below. // Since we know these classes are in the system class loader let's not waste our 
         // time going through the hierarchy.
         //
// The order is based on profiling the server. It may not be optimal for all 
         // workloads.
-        -        if ( name.startsWith("java.") ||
-                name.equals("boolean")   ||
-                name.equals("int")       ||
-                name.equals("double")    ||
-                name.equals("long")) {
+
+        if (name.startsWith("java.") ||
+                name.equals("boolean") ||
+                name.equals("int") ||
+                name.equals("double") ||
+                name.equals("long")) {
Class clazz = ClassLoader.getSystemClassLoader ().loadClass(name); 
             return resolveClass(clazz, resolve);
         }
-        +
         //
// if we are using inverse class loading, check local urls first 
         //
@@ -275,8 +278,7 @@
         // Check parent class loaders
         //
         if (!isHiddenClass(name)) {
-            for (int i = 0; i < parents.length; i++) {
-                ClassLoader parent = parents[i];
+            for (ClassLoader parent : parents) {
                 try {
                     Class clazz = parent.loadClass(name);
                     return resolveClass(clazz, resolve);
@@ -304,8 +306,8 @@
     }
      private boolean isNonOverridableClass(String name) {
-        for (int i = 0; i < nonOverridableClasses.length; i++) {
-            if (name.startsWith(nonOverridableClasses[i])) {
+        for (String nonOverridableClass : nonOverridableClasses) {
+            if (name.startsWith(nonOverridableClass)) {
                 return true;
             }
         }
@@ -313,8 +315,8 @@
     }
      private boolean isHiddenClass(String name) {
-        for (int i = 0; i < hiddenClasses.length; i++) {
-            if (name.startsWith(hiddenClasses[i])) {
+        for (String hiddenClass : hiddenClasses) {
+            if (name.startsWith(hiddenClass)) {
                 return true;
             }
         }
@@ -347,8 +349,7 @@
         // Check parent class loaders
         //
         if (!isHiddenResource(name)) {
-            for (int i = 0; i < parents.length; i++) {
-                ClassLoader parent = parents[i];
+            for (ClassLoader parent : parents) {
                 URL url = parent.getResource(name);
                 if (url != null) {
                     return url;
@@ -370,44 +371,94 @@
         return null;
     }
- public Enumeration findResources(String name) throws IOException { + public Enumeration<URL> findResources(String name) throws IOException { 
         if (isDestroyed()) {
             return Collections.enumeration(Collections.EMPTY_SET);
         }
 -        List resources = new ArrayList();
+ Set<ClassLoader> knownClassloaders = new HashSet<ClassLoader>(); + List<Enumeration<URL>> enumerations = new ArrayList<Enumeration<URL>>(); 
 -        //
- // if we are using inverse class loading, add the resources from local urls first 
-        //
-        if (inverseClassLoading && !isDestroyed()) {
- List myResources = Collections.list (super.findResources(name)); 
-            resources.addAll(myResources);
-        }
+        recursiveFind(knownClassloaders, enumerations, name);
 -        //
-        // Add parent resources
-        //
-        for (int i = 0; i < parents.length; i++) {
-            ClassLoader parent = parents[i];
- List parentResources = Collections.list (parent.getResources(name)); 
-            resources.addAll(parentResources);
+        return new UnionEnumeration<URL>(enumerations);
+        /*
+             List<URL> resources = new ArrayList<URL>();
+             Set<URL> found = new HashSet<URL>();
+
+             //
+ // if we are using inverse class loading, add the resources from local urls first 
+             //
+             if (inverseClassLoading && !isDestroyed()) {
+ for (Enumeration myResources = super.findResources(name); myResources.hasMoreElements();) { 
+                     URL url = (URL) myResources.nextElement();
+                     if (!found.contains(url)) {
+                         found.add(url);
+                         resources.add(url);
+                     }
+                 }
+             }
+
+             //
+             // Add parent resources
+             //
+             for (ClassLoader parent : parents) {
+ for (Enumeration parentResources = parent.getResources(name); parentResources.hasMoreElements();) { 
+                     URL url = (URL) parentResources.nextElement();
+                     if (!found.contains(url)) {
+                         found.add(url);
+                         resources.add(url);
+                     }
+                 }
+             }
+
+             //
+ // if we are not using inverse class loading, add the resources from local urls now 
+             //
+             if (!inverseClassLoading && !isDestroyed()) {
+ for (Enumeration myResources = super.findResources(name); myResources.hasMoreElements();) { 
+                     URL url = (URL) myResources.nextElement();
+                     if (!found.contains(url)) {
+                         found.add(url);
+                         resources.add(url);
+                     }
+                 }
+             }
+
+             return Collections.enumeration(resources);
+        */
+    }
+
+ protected void recursiveFind(Set<ClassLoader> knownClassloaders, List<Enumeration<URL>> enumerations, String name) throws IOException { 
+        if (isDestroyed() || knownClassloaders.contains(this)) {
+            return;
+        }
+        knownClassloaders.add(this);
+        if (inverseClassLoading) {
+            enumerations.add(internalfindResources(name));
         }
-
-        //
- // if we are not using inverse class loading, add the resources from local urls now 
-        //
-        if (!inverseClassLoading && !isDestroyed()) {
- List myResources = Collections.list (super.findResources(name)); 
-            resources.addAll(myResources);
+        for (ClassLoader parent : parents) {
+            if (parent instanceof MultiParentClassLoader) {
+ ((MultiParentClassLoader) parent).recursiveFind (knownClassloaders, enumerations, name); 
+            } else {
+                if (!knownClassloaders.contains(parent)) {
+                    enumerations.add(parent.getResources(name));
+                    knownClassloaders.add(parent);
+                }
+            }
+        }
+        if (!inverseClassLoading) {
+            enumerations.add(internalfindResources(name));
         }
+    }
 -        return Collections.enumeration(resources);
+ protected Enumeration<URL> internalfindResources(String name) throws IOException { 
+        return super.findResources(name);
     }
      private boolean isNonOverridableResource(String name) {
-        for (int i = 0; i < nonOverridableResources.length; i++) {
-            if (name.startsWith(nonOverridableResources[i])) {
+ for (String nonOverridableResource : nonOverridableResources) { 
+            if (name.startsWith(nonOverridableResource)) {
                 return true;
             }
         }
@@ -415,8 +466,8 @@
     }
      private boolean isHiddenResource(String name) {
-        for (int i = 0; i < hiddenResources.length; i++) {
-            if (name.startsWith(hiddenResources[i])) {
+        for (String hiddenResource : hiddenResources) {
+            if (name.startsWith(hiddenResource)) {
                 return true;
             }
         }
@@ -432,7 +483,7 @@
     }
      public void destroy() {
-        synchronized(this) {
+        synchronized (this) {
             if (destroyed) return;
             destroyed = true;
         }
@@ -447,7 +498,7 @@
// it has introspected. If we don't flush the cache, we may run out of 
         // Permanent Generation space.
         Introspector.flushCaches();
-        +
         ClassLoaderRegistry.remove(this);
     }
 @@ -475,8 +526,10 @@
             }
         }
     }
-    protected void finalize(){
+
+    protected void finalize() throws Throwable {
         ClassLoaderRegistry.remove(this);
+        super.finalize();
     }
  }

Reply via email to