Allow better configuration of WebAppContext classloader (#10163)

* Allow better configuration of WebAppContext classloader

Moved the creation of a WebAppContext classloader into an extensible method.

* Allow better configuration of WebAppContext classloader

EE8/9 also

* Allow better configuration of WebAppContext classloader

Only close loader if context created it.

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebAppClassLoader.java

@@ -151,10 +151,8 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
      * Constructor.
      *
      * @param context the context for this classloader
-     * @throws IOException if unable to initialize from context
      */
     public WebAppClassLoader(Context context)
-        throws IOException
     {
         this(null, context);
     }
@@ -164,10 +162,8 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
      *
      * @param parent the parent classloader
      * @param context the context for this classloader
-     * @throws IOException if unable to initialize classloader
      */
     public WebAppClassLoader(ClassLoader parent, Context context)
-        throws IOException
     {
         super(new URL[]{}, parent != null ? parent
             : (Thread.currentThread().getContextClassLoader() != null ? Thread.currentThread().getContextClassLoader()
@@ -595,11 +591,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
 
             return defineClass(name, bytes, 0, bytes.length);
         }
-        catch (IOException e)
+        catch (IOException | IllegalClassFormatException e)
-        {
-            throw new ClassNotFoundException(name, e);
-        }
-        catch (IllegalClassFormatException e)
         {
             throw new ClassNotFoundException(name, e);
         }

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebAppContext.java

@@ -257,8 +257,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
     {
         super.setDisplayName(servletContextName);
         ClassLoader cl = getClassLoader();
-        if (cl instanceof WebAppClassLoader && servletContextName != null)
+        if (servletContextName != null && cl instanceof WebAppClassLoader webAppClassLoader)
-            ((WebAppClassLoader)cl).setName(servletContextName);
+            webAppClassLoader.setName(servletContextName);
     }
 
     /**
@@ -334,8 +334,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         if (name == null)
             name = getContextPath();
 
-        if (classLoader instanceof WebAppClassLoader && getDisplayName() != null)
+        if (classLoader instanceof WebAppClassLoader webAppClassLoader && getDisplayName() != null)
-            ((WebAppClassLoader)classLoader).setName(name);
+            webAppClassLoader.setName(name);
     }
 
     public ResourceFactory getResourceFactory()
@@ -436,12 +436,12 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
 
         // Configure classloader
         _initialClassLoader = getClassLoader();
-        if (!(_initialClassLoader instanceof WebAppClassLoader))
+        ClassLoader loader = configureClassLoader(_initialClassLoader);
-            setClassLoader(new WebAppClassLoader(_initialClassLoader, this));
+        if (loader != _initialClassLoader)
+            setClassLoader(loader);
 
         if (LOG.isDebugEnabled())
         {
-            ClassLoader loader = getClassLoader();
             LOG.debug("Thread Context classloader {}", loader);
             loader = loader.getParent();
             while (loader != null)
@@ -454,6 +454,18 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         _configurations.preConfigure(this);
     }
 
+    /**
+     * Configure the context {@link ClassLoader}, potentially wrapping it.
+     * @param loader The loader initially set on this context by {@link #setClassLoader(ClassLoader)}
+     * @return Either the configured loader, or a new {@link ClassLoader} that uses the loader.
+     */
+    protected ClassLoader configureClassLoader(ClassLoader loader)
+    {
+        if (loader instanceof WebAppClassLoader)
+            return loader;
+        return new WebAppClassLoader(loader, this);
+    }
+
     @Override
     protected void createTempDirectory()
     {
@@ -1228,13 +1240,13 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         }
         finally
         {
-            if (!(_initialClassLoader instanceof WebAppClassLoader))
+            ClassLoader loader = getClassLoader();
+            if (loader != _initialClassLoader)
             {
-                ClassLoader loader = getClassLoader();
+                if (loader instanceof URLClassLoader urlClassLoader)
-                if (loader instanceof URLClassLoader)
+                    urlClassLoader.close();
-                    ((URLClassLoader)loader).close();
+                setClassLoader(_initialClassLoader);
             }
-            setClassLoader(_initialClassLoader);
 
             _unavailableException = null;
         }

jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebAppClassLoader.java

@@ -124,7 +124,6 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
      * <p>Run the passed {@link PrivilegedExceptionAction} with the classloader
      * configured so as to allow server classes to be visible</p>
      *
-     * @param <T> The type returned by the action
      * @param action The action to run
      * @param <T> the type of PrivilegedExceptionAction
      * @return The return from the action
@@ -312,8 +311,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
     public PermissionCollection getPermissions(CodeSource cs)
     {
         PermissionCollection permissions = _context.getPermissions();
-        PermissionCollection pc = (permissions == null) ? super.getPermissions(cs) : permissions;
+        return (permissions == null) ? super.getPermissions(cs) : permissions;
-        return pc;
     }
 
     @Override
@@ -359,7 +357,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
 
     /**
      * Get a resource from the classloader
-     *
+     * <p>
      * NOTE: this method provides a convenience of hacking off a leading /
      * should one be present. This is non-standard and it is recommended
      * to not rely on this behavior
@@ -428,8 +426,8 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
         synchronized (getClassLoadingLock(name))
         {
             ClassNotFoundException ex = null;
-            Class<?> parentClass = null;
+            Class<?> parentClass;
-            Class<?> webappClass = null;
+            Class<?> webappClass;
 
             // Has this loader loaded the class already?
             webappClass = findLoadedClass(name);
@@ -595,11 +593,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
 
             return defineClass(name, bytes, 0, bytes.length);
         }
-        catch (IOException e)
+        catch (IOException | IllegalClassFormatException e)
-        {
-            throw new ClassNotFoundException(name, e);
-        }
-        catch (IllegalClassFormatException e)
         {
             throw new ClassNotFoundException(name, e);
         }

jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebAppContext.java

@@ -473,12 +473,12 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
 
         // Configure classloader
         _initialClassLoader = getClassLoader();
-        if (!(_initialClassLoader instanceof WebAppClassLoader))
+        ClassLoader loader = configureClassLoader(_initialClassLoader);
-            setClassLoader(new WebAppClassLoader(_initialClassLoader, this));
+        if (loader != _initialClassLoader)
+            setClassLoader(loader);
 
         if (LOG.isDebugEnabled())
         {
-            ClassLoader loader = getClassLoader();
             LOG.debug("Thread Context classloader {}", loader);
             loader = loader.getParent();
             while (loader != null)
@@ -491,6 +491,18 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         _configurations.preConfigure(this);
     }
 
+    /**
+     * Configure the context {@link ClassLoader}, potentially wrapping it.
+     * @param loader The loader initially set on this context by {@link #setClassLoader(ClassLoader)}
+     * @return Either the configured loader, or a new {@link ClassLoader} that uses the loader.
+     */
+    protected ClassLoader configureClassLoader(ClassLoader loader) throws IOException
+    {
+        if (loader instanceof WebAppClassLoader)
+            return loader;
+        return new WebAppClassLoader(loader, this);
+    }
+
     public boolean configure() throws Exception
     {
         return _configurations.configure(this);
@@ -1302,13 +1314,13 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         }
         finally
         {
-            if (!(_initialClassLoader instanceof WebAppClassLoader))
+            ClassLoader loader = getClassLoader();
+            if (loader != _initialClassLoader)
             {
-                ClassLoader loader = getClassLoader();
+                if (loader instanceof URLClassLoader urlClassLoader)
-                if (loader instanceof URLClassLoader)
+                    urlClassLoader.close();
-                    ((URLClassLoader)loader).close();
+                setClassLoader(_initialClassLoader);
             }
-            setClassLoader(_initialClassLoader);
 
             _unavailableException = null;
         }