Issue #12094 restore classloader association during comp/env creation

jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java

@@ -93,9 +93,31 @@ public Object getObjectInstance(Object obj,
     {
         Context ctx = null;
 
-        //If the thread context classloader is set, then try its hierarchy to find a matching context
+        //See if there is a classloader already set to use for finding the comp
+        //naming Context
+        ClassLoader loader = (ClassLoader)__threadClassLoader.get();
+        if (loader != null)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("Using threadlocal classloader");
+            try (AutoLock l = __lock.lock())
+            {
+                ctx = getContextForClassLoader(loader);
+                if (ctx == null)
+                {
+                    ctx = newNamingContext(obj, loader, env, name, nameCtx);
+                    __contextMap.put(loader, ctx);
+                    if (LOG.isDebugEnabled())
+                        LOG.debug("Made context {} for classloader {}", name.get(0), loader);
+                }
+                return ctx;
+            }
+        }
+
+        //If the thread context classloader is set, then try it and its
+        //classloader hierarchy to find a matching naming Context
         ClassLoader tccl = Thread.currentThread().getContextClassLoader();
-        ClassLoader loader = tccl;
+        loader = tccl;
         if (loader != null)
         {
             if (LOG.isDebugEnabled())
@@ -121,7 +143,7 @@ public Object getObjectInstance(Object obj,
         }
 
         //If trying thread context classloader hierarchy failed, try the
-        //classloader associated with the current context
+        //classloader associated with the current ContextHandler
         if (ContextHandler.getCurrentContext() != null)
         {
             if (LOG.isDebugEnabled())
@@ -189,6 +211,18 @@ public Context getContextForClassLoader(ClassLoader loader)
         }
     }
 
+    public static ClassLoader associateClassLoader(final ClassLoader loader)
+    {
+        ClassLoader prev = (ClassLoader)__threadClassLoader.get();
+        __threadClassLoader.set(loader);
+        return prev;
+    }
+
+    public static void disassociateClassLoader()
+    {
+        __threadClassLoader.set(null);
+    }
+
     public static void dump(Appendable out, String indent) throws IOException
     {
         try (AutoLock l = __lock.lock())

jetty-ee10/jetty-ee10-plus/src/main/java/module-info.java

@@ -23,6 +23,7 @@
 
     // Only required if using Transaction.
     requires static transitive jakarta.transaction;
+    requires org.eclipse.jetty.jndi;
 
     exports org.eclipse.jetty.ee10.plus.jndi;
     exports org.eclipse.jetty.ee10.plus.webapp;

jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java

@@ -28,6 +28,7 @@
 import org.eclipse.jetty.ee10.webapp.WebAppClassLoader;
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
 import org.eclipse.jetty.ee10.webapp.WebXmlConfiguration;
+import org.eclipse.jetty.jndi.ContextFactory;
 import org.eclipse.jetty.plus.jndi.EnvEntry;
 import org.eclipse.jetty.plus.jndi.NamingEntryUtil;
 import org.eclipse.jetty.util.jndi.NamingDump;
@@ -123,6 +124,7 @@ public void deconfigure(WebAppContext context) throws Exception
         //get rid of any bindings for comp/env for webapp
         ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
         Thread.currentThread().setContextClassLoader(context.getClassLoader());
+        ContextFactory.associateClassLoader(context.getClassLoader());
         try
         {
             Context ic = new InitialContext();
@@ -147,6 +149,7 @@ public void deconfigure(WebAppContext context) throws Exception
         }
         finally
         {
+            ContextFactory.disassociateClassLoader();
             Thread.currentThread().setContextClassLoader(oldLoader);
         }
     }
@@ -218,14 +221,26 @@ protected void createEnvContext(WebAppContext wac)
     {
         ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
         Thread.currentThread().setContextClassLoader(wac.getClassLoader());
+        //ensure that we create a unique comp/env context for this webapp based off
+        //its classloader
+        ContextFactory.associateClassLoader(wac.getClassLoader());
         try
         {
-            Context context = new InitialContext();
-            Context compCtx = (Context)context.lookup("java:comp");
-            compCtx.createSubcontext("env");
+            WebAppClassLoader.runWithServerClassAccess(() ->
+            {
+                Context context = new InitialContext();
+                Context compCtx = (Context)context.lookup("java:comp");
+                compCtx.createSubcontext("env");
+                return null;
+            });
+        }
+        catch (Exception e)
+        {
+            throw new RuntimeException(e);
         }
         finally
         {
+            ContextFactory.disassociateClassLoader();
             Thread.currentThread().setContextClassLoader(oldLoader);
         }
     }

jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java

@@ -15,19 +15,31 @@
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
 
+import org.eclipse.jetty.ee.WebAppClassLoading;
+import org.eclipse.jetty.ee10.webapp.Configuration;
+import org.eclipse.jetty.ee10.webapp.WebAppClassLoader;
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
 import org.eclipse.jetty.plus.jndi.NamingEntryUtil;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Isolated;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+@Isolated("jndi entries")
 public class EnvConfigurationTest
 {
     Server _server;
@@ -73,4 +85,79 @@ public void testWithJettyEEWebXml() throws Exception
         assertNotNull(NamingEntryUtil.lookupNamingEntry(context, "peach"));
         assertNull(NamingEntryUtil.lookupNamingEntry(context, "cabbage"));
     }
+
+    @Test
+    public void testCompEnvCreation() throws Exception
+    {
+        EnvConfiguration envConfigurationA = new EnvConfiguration();
+        EnvConfiguration envConfigurationB = new EnvConfiguration();
+        WebAppContext webappA = null;
+        WebAppContext webappB = null;
+        try
+        {
+            webappA = new WebAppContext();
+            webappA.setConfigurations(new Configuration[]{new PlusConfiguration(), new EnvConfiguration()});
+            webappA.setClassLoader(new WebAppClassLoader(Thread.currentThread().getContextClassLoader(), webappA));
+
+            //ensure that a java:comp/env Context was created for webappA
+            envConfigurationA.preConfigure(webappA);
+            Context namingContextA = getCompEnvFor(webappA);
+
+            webappB = new WebAppContext();
+            webappB.setConfigurations(new Configuration[]{new PlusConfiguration(), new EnvConfiguration()});
+            webappB.setClassLoader(new WebAppClassLoader(Thread.currentThread().getContextClassLoader(), webappB));
+
+            //ensure that a different java:comp/env Context was created for webappB
+            envConfigurationB.preConfigure(webappB);
+            Context namingContextB = getCompEnvFor(webappB);
+
+            assertThat(namingContextA, is(not(namingContextB)));
+        }
+        catch (Throwable t)
+        {
+            t.printStackTrace();
+        }
+        finally
+        {
+            envConfigurationA.deconfigure(webappA);
+            envConfigurationB.deconfigure(webappB);
+        }
+    }
+
+    @Test
+    public void testPriorCompCreation() throws Exception
+    {
+        //pre-create java:comp on the app classloader
+        new InitialContext().lookup("java:comp");
+        //test that each webapp still gets its own naming Context
+        testCompEnvCreation();
+    }
+
+    /**
+     * Find the java:comp/env naming Context for the given webapp
+     * @param webapp the WebAppContext whose naming comp/env Context to find
+     * @return the comp/env naming Context specific to the given WebAppContext
+     * @throws NamingException
+     */
+    private Context getCompEnvFor(WebAppContext webapp)
+        throws NamingException
+    {
+        if (webapp == null)
+            return null;
+
+        ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
+        Context namingContext = null;
+        try
+        {
+            Thread.currentThread().setContextClassLoader(webapp.getClassLoader());
+            InitialContext ic = new InitialContext();
+            namingContext = (Context)ic.lookup("java:comp/env");
+            return namingContext;
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(oldLoader);
+        }
+    }
+
 }

jetty-ee9/jetty-ee9-plus/src/main/java/module-info.java

@@ -24,6 +24,7 @@
 
     // Only required if using Transaction.
     requires static transitive jakarta.transaction;
+    requires org.eclipse.jetty.jndi;
 
     exports org.eclipse.jetty.ee9.plus.jndi;
     exports org.eclipse.jetty.ee9.plus.webapp;

jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java

@@ -29,6 +29,7 @@
 import org.eclipse.jetty.ee9.webapp.WebAppClassLoader;
 import org.eclipse.jetty.ee9.webapp.WebAppContext;
 import org.eclipse.jetty.ee9.webapp.WebXmlConfiguration;
+import org.eclipse.jetty.jndi.ContextFactory;
 import org.eclipse.jetty.plus.jndi.EnvEntry;
 import org.eclipse.jetty.plus.jndi.NamingEntryUtil;
 import org.eclipse.jetty.util.IO;
@@ -139,6 +140,7 @@ public void deconfigure(WebAppContext context) throws Exception
         //get rid of any bindings for comp/env for webapp
         ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
         Thread.currentThread().setContextClassLoader(context.getClassLoader());
+        ContextFactory.associateClassLoader(context.getClassLoader());
         try
         {
             Context ic = new InitialContext();
@@ -163,6 +165,7 @@ public void deconfigure(WebAppContext context) throws Exception
         }
         finally
         {
+            ContextFactory.disassociateClassLoader();
             Thread.currentThread().setContextClassLoader(oldLoader);
             IO.close(_resourceFactory);
             _resourceFactory = null;
@@ -236,14 +239,27 @@ protected void createEnvContext(WebAppContext wac)
     {
         ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
         Thread.currentThread().setContextClassLoader(wac.getClassLoader());
+        //ensure that we create a unique comp/env context for this webapp based off
+        //its classloader
+        ContextFactory.associateClassLoader(wac.getClassLoader());
+
         try
         {
-            Context context = new InitialContext();
-            Context compCtx = (Context)context.lookup("java:comp");
-            compCtx.createSubcontext("env");
+            WebAppClassLoader.runWithServerClassAccess(() ->
+            {
+                Context context = new InitialContext();
+                Context compCtx = (Context)context.lookup("java:comp");
+                compCtx.createSubcontext("env");
+                return null;
+            });
+        }
+        catch (Exception e)
+        {
+            throw new RuntimeException(e);
         }
         finally
         {
+            ContextFactory.disassociateClassLoader();
             Thread.currentThread().setContextClassLoader(oldLoader);
         }
     }