# HG changeset patch # User coffeys # Date 1581285356 0 # Sun Feb 09 21:55:56 2020 +0000 # Node ID d59968631720c7aa40c43abbcc5fa1fa94ffe10a # Parent 49ef1eeed72540cf1fa55db0e7a08e944aef72e3 8223260: NamingManager should cache InitialContextFactory Reviewed-by: alanb, plevart, dfuchs diff --git a/src/java.base/share/classes/module-info.java b/src/java.base/share/classes/module-info.java --- a/src/java.base/share/classes/module-info.java +++ b/src/java.base/share/classes/module-info.java @@ -153,7 +153,8 @@ jdk.jlink; exports jdk.internal.loader to java.instrument, - java.logging; + java.logging, + java.naming; exports jdk.internal.jmod to jdk.compiler, jdk.jlink; diff --git a/src/java.naming/share/classes/javax/naming/spi/NamingManager.java b/src/java.naming/share/classes/javax/naming/spi/NamingManager.java --- a/src/java.naming/share/classes/javax/naming/spi/NamingManager.java +++ b/src/java.naming/share/classes/javax/naming/spi/NamingManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,13 +26,15 @@ package javax.naming.spi; import java.net.MalformedURLException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.*; - import javax.naming.*; import com.sun.naming.internal.VersionHelper; import com.sun.naming.internal.ResourceManager; import com.sun.naming.internal.FactoryEnumeration; +import jdk.internal.loader.ClassLoaderValue; /** * This class contains methods for creating context objects @@ -79,6 +81,9 @@ */ private static ObjectFactoryBuilder object_factory_builder = null; + private static final ClassLoaderValue FACTORIES_CACHE = + new ClassLoaderValue<>(); + /** * The ObjectFactoryBuilder determines the policy used when * trying to load object factories. @@ -672,6 +677,7 @@ */ public static Context getInitialContext(Hashtable env) throws NamingException { + ClassLoader loader; InitialContextFactory factory = null; InitialContextFactoryBuilder builder = getInitialContextFactoryBuilder(); @@ -689,39 +695,22 @@ throw ne; } - ServiceLoader loader = - ServiceLoader.load(InitialContextFactory.class); - - Iterator iterator = loader.iterator(); - try { - while (iterator.hasNext()) { - InitialContextFactory f = iterator.next(); - if (f.getClass().getName().equals(className)) { - factory = f; - break; - } - } - } catch (ServiceConfigurationError e) { - NoInitialContextException ne = - new NoInitialContextException( - "Cannot load initial context factory " - + "'" + className + "'"); - ne.setRootCause(e); - throw ne; + if (System.getSecurityManager() == null) { + loader = Thread.currentThread().getContextClassLoader(); + if (loader == null) loader = ClassLoader.getSystemClassLoader(); + } else { + PrivilegedAction pa = () -> { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + return (cl == null) ? ClassLoader.getSystemClassLoader() : cl; + }; + loader = AccessController.doPrivileged(pa); } - if (factory == null) { - try { - @SuppressWarnings("deprecation") - Object o = helper.loadClass(className).newInstance(); - factory = (InitialContextFactory) o; - } catch (Exception e) { - NoInitialContextException ne = - new NoInitialContextException( - "Cannot instantiate class: " + className); - ne.setRootCause(e); - throw ne; - } + var key = FACTORIES_CACHE.sub(className); + try { + factory = key.computeIfAbsent(loader, (ld, ky) -> getFactory(ky.key())); + } catch (FactoryInitializationError e) { + throw e.getCause(); } } else { factory = builder.createInitialContextFactory(env); @@ -730,6 +719,43 @@ return factory.getInitialContext(env); } + private static InitialContextFactory getFactory(String className) { + InitialContextFactory factory; + try { + ServiceLoader loader = + ServiceLoader.load(InitialContextFactory.class); + + factory = loader + .stream() + .filter(p -> p.type().getName().equals(className)) + .findFirst() + .map(ServiceLoader.Provider::get) + .orElse(null); + } catch (ServiceConfigurationError e) { + NoInitialContextException ne = + new NoInitialContextException( + "Cannot load initial context factory " + + "'" + className + "'"); + ne.setRootCause(e); + throw new FactoryInitializationError(ne); + } + + if (factory == null) { + try { + @SuppressWarnings("deprecation") + Object o = helper.loadClass(className).newInstance(); + factory = (InitialContextFactory) o; + } catch (Exception e) { + NoInitialContextException ne = + new NoInitialContextException( + "Cannot instantiate class: " + className); + ne.setRootCause(e); + throw new FactoryInitializationError(ne); + } + } + return factory; + } + /** * Sets the InitialContextFactory builder to be builder. @@ -921,4 +947,17 @@ return (answer != null) ? answer : obj; } + + private static class FactoryInitializationError extends Error { + static final long serialVersionUID = -5805552256848841560L; + + private FactoryInitializationError(NoInitialContextException cause) { + super(cause); + } + + @Override + public NoInitialContextException getCause() { + return (NoInitialContextException) super.getCause(); + } + } } diff --git a/test/jdk/javax/naming/spi/DummyContextFactory.java b/test/jdk/javax/naming/spi/DummyContextFactory.java new file mode 100644 --- /dev/null +++ b/test/jdk/javax/naming/spi/DummyContextFactory.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.io.File; +import java.lang.ref.WeakReference; +import java.net.URL; +import java.net.URLClassLoader; +import javax.naming.Context; +import javax.naming.InitialContext; +import javax.naming.NamingException; +import javax.naming.NoInitialContextException; +import javax.naming.spi.InitialContextFactory; +import javax.naming.spi.NamingManager; +import java.util.Hashtable; + +public class DummyContextFactory implements InitialContextFactory { + static final String DUMMY_FACTORY = "DummyContextFactory"; + static final String DUMMY_FACTORY2 = "DummyContextFactory2"; + static final String MISSING_FACTORY = "NonExistant"; + static int counter = 0; + ClassLoader origContextLoader = Thread.currentThread().getContextClassLoader(); + + public static void main(String[] s) throws Exception { + DummyContextFactory dcf = new DummyContextFactory(); + dcf.runTest(); + } + + private void runTest() throws Exception { + final String classes = System.getProperty("url.dir", "."); + final URL curl = new File(classes).toURI().toURL(); + URLClassLoader testLoader = new URLClassLoader(new URL[] {curl}, null); + WeakReference weakRef = new WeakReference<>(testLoader); + Thread.currentThread().setContextClassLoader(testLoader); + Hashtable env = new Hashtable<>(); + env.put(Context.INITIAL_CONTEXT_FACTORY, DUMMY_FACTORY); + testContextCalls(env); + + // now test with another factory + Thread.currentThread().setContextClassLoader(testLoader); + env.put(Context.INITIAL_CONTEXT_FACTORY, DUMMY_FACTORY2); + testContextCalls(env); + + // one count is derived from a default constructor call (ignored for test) + // class associated with this ClassLoader should have 2 counts + if (counter != 2) { + throw new RuntimeException("wrong count: " + counter); + } + + // a test for handling non-existent classes + env.put(Context.INITIAL_CONTEXT_FACTORY, MISSING_FACTORY); + testBadContextCall(env); + + // test that loader gets GC'ed + testLoader = null; + System.gc(); + while (weakRef.get() != null) { + Thread.sleep(100); + System.gc(); + } + } + + private void testContextCalls(Hashtable env) throws Exception { + // the context is returned here but it's the ContextFactory that + // we're mainly interested in. Hence the counter test. + + // 1st call populates the WeakHashMap + // Uses URLClassLoader + Context cxt = NamingManager.getInitialContext(env); + + // 2nd call uses cached factory + cxt = NamingManager.getInitialContext(env); + + Thread.currentThread().setContextClassLoader(origContextLoader); + + // 3rd call uses new factory + // AppClassLoader + cxt = NamingManager.getInitialContext(env); + + // test with null TCCL + // this shouldn't increase the count since a null TCCL + // means we default to System ClassLoader in this case (AppClassLoader) + Thread.currentThread().setContextClassLoader(null); + cxt = NamingManager.getInitialContext(env); + } + + private void testBadContextCall(Hashtable env) throws Exception { + try { + Context cxt = NamingManager.getInitialContext(env); + throw new RuntimeException("Expected NoInitialContextException"); + } catch (NoInitialContextException e) { + if (!(e.getCause() instanceof ClassNotFoundException)) { + throw new RuntimeException("unexpected cause", e.getCause()); + } + } + } + + public DummyContextFactory() { + System.out.println("New DummyContextFactory " + (++counter)); + //new Throwable().printStackTrace(System.out); + } + + @Override + public Context getInitialContext(Hashtable environment) throws NamingException { + return new DummyContext(environment); + } + + public class DummyContext extends InitialContext { + + private Hashtable env; + + DummyContext(Hashtable env) throws NamingException { + this.env = env; + } + + public Hashtable getEnvironment() { + return env; + } + } +} diff --git a/test/jdk/javax/naming/spi/DummyContextFactory2.java b/test/jdk/javax/naming/spi/DummyContextFactory2.java new file mode 100644 --- /dev/null +++ b/test/jdk/javax/naming/spi/DummyContextFactory2.java @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import javax.naming.Context; +import javax.naming.InitialContext; +import javax.naming.NamingException; +import javax.naming.spi.InitialContextFactory; +import java.util.Hashtable; + +public class DummyContextFactory2 implements InitialContextFactory { + static int counter = 0; + + public DummyContextFactory2() { + System.out.println("New DummyContextFactory2 " + (++counter)); + //new Throwable().printStackTrace(System.out); + } + + @Override + public Context getInitialContext(Hashtable environment) throws NamingException { + return new DummyContext(environment); + } + + public class DummyContext extends InitialContext { + + private Hashtable env; + + DummyContext(Hashtable env) throws NamingException { + this.env = env; + } + + public Hashtable getEnvironment() { + return env; + } + } +} \ No newline at end of file diff --git a/test/jdk/javax/naming/spi/FactoryCacheTest.java b/test/jdk/javax/naming/spi/FactoryCacheTest.java new file mode 100644 --- /dev/null +++ b/test/jdk/javax/naming/spi/FactoryCacheTest.java @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8223260 + * @summary NamingManager should cache InitialContextFactory + * @library /test/lib + * @build jdk.test.lib.util.JarUtils jdk.test.lib.process.* + * FactoryCacheTest + * DummyContextFactory + * DummyContextFactory2 + * @run main FactoryCacheTest + */ + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import jdk.test.lib.JDKToolFinder; +import jdk.test.lib.Utils; +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.util.JarUtils; + +import static java.nio.file.StandardOpenOption.CREATE; +import static java.util.Arrays.asList; + +import static jdk.test.lib.Utils.TEST_CLASSES; + +public class FactoryCacheTest { + + private static final Path SPIJAR = Path.of("testDir", "ContextFactory.jar"); + private static final String SPIJAR_CP = SPIJAR.toAbsolutePath().toString(); + + public static void main(String[] args) throws Throwable { + List argLine = new ArrayList<>(); + argLine.add(JDKToolFinder.getJDKTool("java")); + argLine.addAll(asList(Utils.getTestJavaOpts())); + argLine.addAll(List.of("-cp", TEST_CLASSES)); + argLine.addAll(List.of("-Durl.dir=" + TEST_CLASSES)); + argLine.add("DummyContextFactory"); + + ProcessTools.executeCommand(argLine.stream() + .filter(t -> !t.isEmpty()) + .toArray(String[]::new)) + .shouldHaveExitValue(0); + + // now test the ServiceLoader approach + setupService(); + argLine = new ArrayList<>(); + argLine.add(JDKToolFinder.getJDKTool("java")); + argLine.addAll(asList(Utils.getTestJavaOpts())); + argLine.addAll(List.of("-cp", SPIJAR_CP)); + argLine.addAll(List.of("-Durl.dir=" + TEST_CLASSES)); + argLine.add("DummyContextFactory"); + + ProcessTools.executeCommand(argLine.stream() + .filter(t -> !t.isEmpty()) + .toArray(String[]::new)) + .shouldHaveExitValue(0); + } + + private static void setupService() throws Exception { + Path xdir = Path.of(TEST_CLASSES); + Path config = xdir.resolve(Path.of(TEST_CLASSES,"META-INF/services/javax.naming.spi.InitialContextFactory")); + Files.createDirectories(config.getParent()); + Files.write(config, "DummyContextFactory".getBytes(), CREATE); + JarUtils.createJarFile(SPIJAR, xdir); + } +}