From 54fde0e8beec476c247410db4c457a62c8ca834c Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Sun, 4 Aug 2024 16:48:43 +1000 Subject: [PATCH] Issue #12094 restore classloader association during comp/env creation (#12107) * Issue #12094 restore classloader association during comp/env creation --- .../eclipse/jetty/jndi/ContextFactory.java | 40 ++++++++- .../src/main/java/module-info.java | 1 + .../ee10/plus/webapp/EnvConfiguration.java | 21 ++++- .../plus/webapp/EnvConfigurationTest.java | 87 +++++++++++++++++++ .../src/main/java/module-info.java | 1 + .../ee9/plus/webapp/EnvConfiguration.java | 22 ++++- .../ee9/plus/webapp/EnvConfigurationTest.java | 81 ++++++++++++++++- 7 files changed, 243 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java b/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java index bb187ae59b6d..7ef6100b63c4 100644 --- a/jetty-core/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java +++ b/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()) diff --git a/jetty-ee10/jetty-ee10-plus/src/main/java/module-info.java b/jetty-ee10/jetty-ee10-plus/src/main/java/module-info.java index d4fc7b583129..683fa71c06ec 100644 --- a/jetty-ee10/jetty-ee10-plus/src/main/java/module-info.java +++ b/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; diff --git a/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java b/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java index 96b2e775fbb5..5ba425ef15da 100644 --- a/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java +++ b/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); } } diff --git a/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java b/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java index 6dd104b62e23..3c4ae7c17cc0 100644 --- a/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java +++ b/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); + } + } + } diff --git a/jetty-ee9/jetty-ee9-plus/src/main/java/module-info.java b/jetty-ee9/jetty-ee9-plus/src/main/java/module-info.java index 98ddffbfac05..6f5995d5ad5a 100644 --- a/jetty-ee9/jetty-ee9-plus/src/main/java/module-info.java +++ b/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; diff --git a/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java b/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java index 7a87152e7a3b..372f86b4ab2c 100644 --- a/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java +++ b/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); } } diff --git a/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java b/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java index c7feca819e82..9944ba497a6e 100644 --- a/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java +++ b/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java @@ -15,20 +15,29 @@ 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.ee9.webapp.Configuration; +import org.eclipse.jetty.ee9.webapp.WebAppClassLoader; import org.eclipse.jetty.ee9.webapp.WebAppContext; import org.eclipse.jetty.plus.jndi.NamingEntryUtil; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; -import org.eclipse.jetty.util.jndi.NamingUtil; import org.junit.jupiter.api.AfterEach; 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; @@ -74,4 +83,74 @@ 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))); + } + 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); + } + } }