# HG changeset patch # User jgish # Date 1367339919 14400 # Node ID 6a9d4ce85c1e0f9a5f169399278c5de9b5117b0c # Parent 49d6596100db4e937d35e09b789f6ee869aacadc 8013380: Removal of stack walk to find resource bundle breaks Glassfish startup Summary: Use caller's classloader to load resource as an alternative to thread context classloader and system classloader Reviewed-by: duke diff --git a/src/share/classes/java/util/logging/LogManager.java b/src/share/classes/java/util/logging/LogManager.java --- a/src/share/classes/java/util/logging/LogManager.java +++ b/src/share/classes/java/util/logging/LogManager.java @@ -433,11 +433,13 @@ // add a new Logger or return the one that has been added previously // as a LogManager subclass may override the addLogger, getLogger, // readConfiguration, and other methods. - Logger demandLogger(String name, String resourceBundleName) { + Logger demandLogger(String name, String resourceBundleName, + ClassLoader callersClassLoader) { Logger result = getLogger(name); if (result == null) { // only allocate the new logger once - Logger newLogger = new Logger(name, resourceBundleName); + Logger newLogger = new Logger(name, resourceBundleName, + callersClassLoader); do { if (addLogger(newLogger)) { // We successfully added the new Logger that we @@ -519,7 +521,7 @@ Logger demandLogger(String name, String resourceBundleName) { // a LogManager subclass may have its own implementation to add and // get a Logger. So delegate to the LogManager to do the work. - return manager.demandLogger(name, resourceBundleName); + return manager.demandLogger(name, resourceBundleName, null); } synchronized Logger findLogger(String name) { diff --git a/src/share/classes/java/util/logging/Logger.java b/src/share/classes/java/util/logging/Logger.java --- a/src/share/classes/java/util/logging/Logger.java +++ b/src/share/classes/java/util/logging/Logger.java @@ -218,6 +218,7 @@ private ArrayList kids; // WeakReferences to loggers that have us as parent private volatile Level levelObject; private volatile int levelValue; // current effective level value + private WeakReference callersClassLoaderRef; /** * GLOBAL_LOGGER_NAME is a name for the global logger. @@ -277,14 +278,22 @@ * @throws MissingResourceException if the resourceBundleName is non-null and * no corresponding resource can be found. */ + @CallerSensitive protected Logger(String name, String resourceBundleName) { + this(name, resourceBundleName, + Reflection.getCallerClass().getClassLoader()); + } + + Logger(String name, String resourceBundleName, + ClassLoader callersClassLoader) { this.manager = LogManager.getLogManager(); + this.callersClassLoaderRef = new WeakReference(callersClassLoader); if (resourceBundleName != null) { // MissingResourceException or IllegalArgumentException can // be thrown by setupResourceInfo(). Since this is the Logger // constructor, the resourceBundleName field is null so // IllegalArgumentException cannot happen here. - setupResourceInfo(resourceBundleName); + setupResourceInfo(resourceBundleName, callersClassLoader); } this.name = name; levelValue = Level.INFO.intValue(); @@ -343,7 +352,8 @@ return manager.demandSystemLogger(name, resourceBundleName); } } - return manager.demandLogger(name, resourceBundleName); + return manager.demandLogger(name, resourceBundleName, + caller.getClassLoader()); } /** @@ -436,11 +446,12 @@ // adding a new Logger object is handled by LogManager.addLogger(). @CallerSensitive public static Logger getLogger(String name, String resourceBundleName) { - Logger result = demandLogger(name, resourceBundleName, Reflection.getCallerClass()); + Class callerClass = Reflection.getCallerClass(); + Logger result = demandLogger(name, resourceBundleName, callerClass); // MissingResourceException or IllegalArgumentException can be // thrown by setupResourceInfo(). - result.setupResourceInfo(resourceBundleName); + result.setupResourceInfo(resourceBundleName, callerClass.getClassLoader()); return result; } @@ -527,7 +538,9 @@ * @return localization bundle (may be null) */ public ResourceBundle getResourceBundle() { - return findResourceBundle(getResourceBundleName()); + return findResourceBundle(getResourceBundleName(), + (callersClassLoaderRef != null ? callersClassLoaderRef.get() : + null)); } /** @@ -609,7 +622,10 @@ String ebname = getEffectiveResourceBundleName(); if (ebname != null && !ebname.equals(SYSTEM_LOGGER_RB_NAME)) { lr.setResourceBundleName(ebname); - lr.setResourceBundle(findResourceBundle(ebname)); + lr.setResourceBundle(findResourceBundle(ebname, + (callersClassLoaderRef != null + ? callersClassLoaderRef.get() + : null))); } log(lr); } @@ -936,7 +952,10 @@ lr.setLoggerName(name); if (rbname != null) { lr.setResourceBundleName(rbname); - lr.setResourceBundle(findResourceBundle(rbname)); + lr.setResourceBundle(findResourceBundle(rbname, + (callersClassLoaderRef != null + ? callersClassLoaderRef.get() + : null))); } log(lr); } @@ -1611,7 +1630,8 @@ * @param name the ResourceBundle to locate * @return ResourceBundle specified by name or null if not found */ - private synchronized ResourceBundle findResourceBundle(String name) { + private synchronized ResourceBundle findResourceBundle(String name, + ClassLoader callersClassLoader) { // Return a null bundle for a null name. if (name == null) { return null; @@ -1644,8 +1664,24 @@ catalogLocale = currentLocale; return catalog; } catch (MissingResourceException ex) { + // Woops. We can't find the ResourceBundle in the default + // ClassLoader. Drop through. + } + + // Try with the caller's ClassLoader + if (callersClassLoader == null) { return null; } + + try { + catalog = ResourceBundle.getBundle(name, currentLocale, + callersClassLoader); + catalogName = name; + catalogLocale = currentLocale; + return catalog; + } catch (MissingResourceException ex) { + return null; // no luck + } } // Private utility method to initialize our one entry @@ -1654,7 +1690,8 @@ // that a suitable ResourceBundle exists before setting the // resourceBundleName field. // Synchronized to prevent races in setting the field. - private synchronized void setupResourceInfo(String name) { + private synchronized void setupResourceInfo(String name, + ClassLoader callersClassLoader) { if (name == null) { return; } @@ -1672,9 +1709,10 @@ resourceBundleName + " != " + name); } - if (findResourceBundle(name) == null) { + if (findResourceBundle(name, callersClassLoader) == null) { // We've failed to find an expected ResourceBundle. - throw new MissingResourceException("Can't find " + name + " bundle", name, ""); + throw new MissingResourceException("Can't find " + name + " bundle", + name, ""); } resourceBundleName = name; } diff --git a/test/java/util/logging/bundlesearch/IndirectlyLoadABundle.java b/test/java/util/logging/bundlesearch/IndirectlyLoadABundle.java --- a/test/java/util/logging/bundlesearch/IndirectlyLoadABundle.java +++ b/test/java/util/logging/bundlesearch/IndirectlyLoadABundle.java @@ -29,14 +29,14 @@ /** * This class is used to ensure that a resource bundle loadable by a classloader - * is on the caller's stack, but not on the classpath or TCCL to ensure that - * Logger.getLogger() can't load the bundle via a stack search + * is on the caller's stack, but not on the classpath or TCCL. It tests that + * Logger.getLogger() can load the bundle via the immediate caller's classloader * * @author Jim Gish */ public class IndirectlyLoadABundle { - private final static String rbName = "StackSearchableResource"; + private final static String rbName = "CallerSearchableResource"; public boolean loadAndTest() throws Throwable { // Find out where we are running from so we can setup the URLClassLoader URLs @@ -45,7 +45,6 @@ String testDir = System.getProperty("test.src", System.getProperty("user.dir")); String testClassesDir = System.getProperty("test.classes", System.getProperty("user.dir")); - String sep = System.getProperty("file.separator"); URL[] urls = new URL[2]; @@ -70,7 +69,7 @@ + " able to. Test config problem"); } - Class loadItUpClazz = Class.forName("LoadItUp", true, yetAnotherResourceCL); + Class loadItUpClazz = Class.forName("LoadItUp1", true, yetAnotherResourceCL); ClassLoader actual = loadItUpClazz.getClassLoader(); if (actual != yetAnotherResourceCL) { throw new Exception("LoadItUp was loaded by an unexpected CL: " + actual); diff --git a/test/java/util/logging/bundlesearch/LoadItUp.java b/test/java/util/logging/bundlesearch/LoadItUp1.java rename from test/java/util/logging/bundlesearch/LoadItUp.java rename to test/java/util/logging/bundlesearch/LoadItUp1.java --- a/test/java/util/logging/bundlesearch/LoadItUp.java +++ b/test/java/util/logging/bundlesearch/LoadItUp1.java @@ -32,7 +32,7 @@ * * @author Jim Gish */ -public class LoadItUp { +public class LoadItUp1 { private final static boolean DEBUG = false; @@ -46,17 +46,10 @@ private boolean lookupBundle(String rbName) { // See if Logger.getLogger can find the resource in this directory try { - Logger aLogger = Logger.getLogger("NestedLogger", rbName); + Logger aLogger = Logger.getLogger("NestedLogger1", rbName); + return true; } catch (MissingResourceException re) { - if (DEBUG) { - System.out.println( - "As expected, LoadItUp.lookupBundle() did not find the bundle " - + rbName); - } - return false; + throw re; } - System.out.println("FAILED: LoadItUp.lookupBundle() found the bundle " - + rbName + " using a stack search."); - return true; } } diff --git a/test/java/util/logging/bundlesearch/LoadItUp2.java b/test/java/util/logging/bundlesearch/LoadItUp2.java new file mode 100644 --- /dev/null +++ b/test/java/util/logging/bundlesearch/LoadItUp2.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2013, 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.util.MissingResourceException; +import java.util.logging.Logger; + +/* + * This class is loaded onto the call stack by LoadItUp2Invoker from a separate + * classloader. LoadItUp2Invoker was loaded by a class loader that does have + * access to the bundle, but the class loader used to load this class does not. + * Thus the logging code should not be able to see the resource bundle unless + * it has more than a single level stack crawl, which is not allowed. + * + * @author Jim Gish + */ +public class LoadItUp2 { + + private final static boolean DEBUG = false; + + public Boolean test(String rbName) throws Exception { + // we should not be able to find the resource in this directory via + // getLogger calls. The only way that would be possible given this setup + // is that if Logger.getLogger searched up the call stack + return lookupBundle(rbName); + } + + private boolean lookupBundle(String rbName) { + // See if Logger.getLogger can find the resource in this directory + try { + Logger aLogger = Logger.getLogger("NestedLogger2", rbName); + } catch (MissingResourceException re) { + if (DEBUG) { + System.out.println( + "As expected, LoadItUp2.lookupBundle() did not find the bundle " + + rbName); + } + return false; + } + System.out.println("FAILED: LoadItUp2.lookupBundle() found the bundle " + + rbName + " using a stack search."); + return true; + } +} diff --git a/test/java/util/logging/bundlesearch/LoadItUp2Invoker.java b/test/java/util/logging/bundlesearch/LoadItUp2Invoker.java new file mode 100644 --- /dev/null +++ b/test/java/util/logging/bundlesearch/LoadItUp2Invoker.java @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2013, 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.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; + +/** + * This class is loaded by a class loader that can see the resource. It creates + * a new classloader for LoadItUp2 which cannot see the resource. So, 2 levels + * up the call chain we have a class/classloader that can see the resource, but + * 1 level up the class/classloader cannot. + * + * @author Jim Gish + */ +public class LoadItUp2Invoker { + private URLClassLoader cl; + private String rbName; + private Object loadItUp2; + private Method testMethod; + + public void setup(URL[] urls, String rbName) throws + ReflectiveOperationException { + this.cl = new URLClassLoader(urls, null); + this.rbName = rbName; + // Using this new classloader, load the actual test class + // which is now two levels removed from the original caller + Class loadItUp2Clazz = Class.forName("LoadItUp2", true , cl); + this.loadItUp2 = loadItUp2Clazz.newInstance(); + this.testMethod = loadItUp2Clazz.getMethod("test", String.class); + } + + public Boolean test() throws Throwable { + try { + return (Boolean) testMethod.invoke(loadItUp2, rbName); + } catch (InvocationTargetException ex) { + throw ex.getTargetException(); + } + } +} diff --git a/test/java/util/logging/bundlesearch/ResourceBundleSearchTest.java b/test/java/util/logging/bundlesearch/ResourceBundleSearchTest.java --- a/test/java/util/logging/bundlesearch/ResourceBundleSearchTest.java +++ b/test/java/util/logging/bundlesearch/ResourceBundleSearchTest.java @@ -26,8 +26,8 @@ * @bug 8002070 * @summary Remove the stack search for a resource bundle Logger to use * @author Jim Gish - * @build ResourceBundleSearchTest IndirectlyLoadABundle LoadItUp - * @run main ResourceBundleSearchTest + * @build ResourceBundleSearchTest IndirectlyLoadABundle LoadItUp1 LoadItUp2 TwiceIndirectlyLoadABundle LoadItUp2Invoker + * @run main/othervm ResourceBundleSearchTest */ import java.net.URL; import java.net.URLClassLoader; @@ -39,6 +39,12 @@ import java.util.ResourceBundle; import java.util.logging.Logger; +/** + * This class tests various scenarios of loading resource bundles from + * java.util.logging. Since jtreg uses the logging system, it is necessary to + * run these tests using othervm mode to ensure no interference from logging + * initialization by jtreg + */ public class ResourceBundleSearchTest { private final static boolean DEBUG = false; @@ -60,15 +66,11 @@ // ensure we are using en as the default Locale so we can find the resource Locale.setDefault(Locale.ENGLISH); - String testClasses = System.getProperty("test.classes"); - System.out.println( "test.classes = " + testClasses ); - ClassLoader myClassLoader = ClassLoader.getSystemClassLoader(); // Find out where we are running from so we can setup the URLClassLoader URL String userDir = System.getProperty("user.dir"); String testDir = System.getProperty("test.src", userDir); - String sep = System.getProperty("file.separator"); URL[] urls = new URL[1]; @@ -77,30 +79,32 @@ // Test 1 - can we find a Logger bundle from doing a stack search? // We shouldn't be able to - assertFalse(testGetBundleFromStackSearch(), "testGetBundleFromStackSearch"); + assertFalse(testGetBundleFromStackSearch(), "1-testGetBundleFromStackSearch"); // Test 2 - can we find a Logger bundle off of the Thread context class // loader? We should be able to. assertTrue( testGetBundleFromTCCL(TCCL_TEST_BUNDLE, rbClassLoader), - "testGetBundleFromTCCL"); + "2-testGetBundleFromTCCL"); // Test 3 - Can we find a Logger bundle from the classpath? We should be - // able to, but .... - // We check to see if the bundle is on the classpath or not so that this - // will work standalone. In the case of jtreg/samevm, - // the resource bundles are not on the classpath. Running standalone - // (or othervm), they are + // able to. We'll first check to make sure the setup is correct and + // it actually is on the classpath before checking whether logging + // can see it there. if (isOnClassPath(PROP_RB_NAME, myClassLoader)) { debug("We should be able to see " + PROP_RB_NAME + " on the classpath"); assertTrue(testGetBundleFromSystemClassLoader(PROP_RB_NAME), - "testGetBundleFromSystemClassLoader"); + "3-testGetBundleFromSystemClassLoader"); } else { - debug("We should not be able to see " + PROP_RB_NAME + " on the classpath"); - assertFalse(testGetBundleFromSystemClassLoader(PROP_RB_NAME), - "testGetBundleFromSystemClassLoader"); + throw new Exception("TEST SETUP FAILURE: Cannot see " + PROP_RB_NAME + + " on the classpath"); } + // Test 4 - we should be able to find a bundle from the caller's + // classloader, but only one level up. + assertTrue(testGetBundlerFromCallersClassLoader(), + "4-testGetBundleFromCallersClassLoader"); + report(); } @@ -139,6 +143,13 @@ public boolean testGetBundleFromStackSearch() throws Throwable { // This should fail. This was the old functionality to search up the // caller's call stack + TwiceIndirectlyLoadABundle indirectLoader = new TwiceIndirectlyLoadABundle(); + return indirectLoader.loadAndTest(); + } + + public boolean testGetBundlerFromCallersClassLoader() throws Throwable { + // This should pass. This exercises getting the bundle using the + // class loader of the caller (one level up) IndirectlyLoadABundle indirectLoader = new IndirectlyLoadABundle(); return indirectLoader.loadAndTest(); } @@ -228,11 +239,11 @@ try { Logger aLogger = Logger.getLogger(ResourceBundleSearchTest.newLoggerName(), bundleName); - msg = "INFO: LoggingRunnable() found the bundle " + bundleName + msg = "INFO: LoggingThread.run() found the bundle " + bundleName + (setTCCL ? " with " : " without ") + "setting the TCCL"; foundBundle = true; } catch (MissingResourceException re) { - msg = "INFO: LoggingRunnable() did not find the bundle " + bundleName + msg = "INFO: LoggingThread.run() did not find the bundle " + bundleName + (setTCCL ? " with " : " without ") + "setting the TCCL"; foundBundle = false; } diff --git a/test/java/util/logging/bundlesearch/TwiceIndirectlyLoadABundle.java b/test/java/util/logging/bundlesearch/TwiceIndirectlyLoadABundle.java new file mode 100644 --- /dev/null +++ b/test/java/util/logging/bundlesearch/TwiceIndirectlyLoadABundle.java @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2013, 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.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Paths; + +/** + * This class constructs a scenario where a bundle is accessible on the call + * stack two levels up from the call to getLogger(), but not on the immediate + * caller. This tests that getLogger() isn't doing a stack crawl more than one + * level up to find a bundle. + * + * @author Jim Gish + */ +public class TwiceIndirectlyLoadABundle { + + private final static String rbName = "StackSearchableResource"; + + public boolean loadAndTest() throws Throwable { + // Find out where we are running from so we can setup the URLClassLoader URLs + // test.src and test.classes will be set if running in jtreg, but probably + // not otherwise + String testDir = System.getProperty("test.src", System.getProperty("user.dir")); + String testClassesDir = System.getProperty("test.classes", + System.getProperty("user.dir")); + URL[] urls = new URL[2]; + + // Allow for both jtreg and standalone cases here + // Unlike the 1-level test where we can get the bundle from the caller's + // class loader, for this one we don't want to expose the resource directory + // to the next class. That way we're invoking the LoadItUp2Invoker class + // from this class that does have access to the resources (two levels + // up the call stack), but the Invoker itself won't have access to resource + urls[0] = Paths.get(testDir,"resources").toUri().toURL(); + urls[1] = Paths.get(testClassesDir).toUri().toURL(); + + // Make sure we can find it via the URLClassLoader + URLClassLoader yetAnotherResourceCL = new URLClassLoader(urls, null); + Class loadItUp2InvokerClazz = Class.forName("LoadItUp2Invoker", true, + yetAnotherResourceCL); + ClassLoader actual = loadItUp2InvokerClazz.getClassLoader(); + if (actual != yetAnotherResourceCL) { + throw new Exception("LoadItUp2Invoker was loaded by an unexpected CL: " + + actual); + } + Object loadItUp2Invoker = loadItUp2InvokerClazz.newInstance(); + + Method setupMethod = loadItUp2InvokerClazz.getMethod("setup", + urls.getClass(), String.class); + try { + // For the next class loader we create, we want to leave off + // the resources. That way loadItUp2Invoker will have access to + // them, but the next class won't. + URL[] noResourceUrl = new URL[1]; + noResourceUrl[0] = urls[1]; // from above -- just the test classes + setupMethod.invoke(loadItUp2Invoker, noResourceUrl, rbName); + } catch (InvocationTargetException ex) { + throw ex.getTargetException(); + } + + Method testMethod = loadItUp2InvokerClazz.getMethod("test"); + try { + return (Boolean) testMethod.invoke(loadItUp2Invoker); + } catch (InvocationTargetException ex) { + throw ex.getTargetException(); + } + } +} diff --git a/test/java/util/logging/bundlesearch/resources/CallerSearchableResource_en.properties b/test/java/util/logging/bundlesearch/resources/CallerSearchableResource_en.properties new file mode 100644 --- /dev/null +++ b/test/java/util/logging/bundlesearch/resources/CallerSearchableResource_en.properties @@ -0,0 +1,25 @@ +# +# Copyright (c) 2013, 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. +# +sample1=translation #4 for sample1 +sample2=translation #4 for sample2 +supports-test=ResourceBundleSearchTest