# HG changeset patch # User dfuchs # Date 1473858254 -3600 # Node ID dba53de83476278ab711ecb19f73edc10bf85deb # Parent 46cd6d4f353fa3c30a80e1418a834833cfdf8500 6543126: Level.known can leak memory Summary: Custom level instances will now be released when their defining class loader is no longer referenced. Reviewed-by: plevart, mchung, chegar diff -r 46cd6d4f353f -r dba53de83476 jdk/src/java.base/share/classes/module-info.java --- a/jdk/src/java.base/share/classes/module-info.java Wed Sep 14 14:29:39 2016 +0200 +++ b/jdk/src/java.base/share/classes/module-info.java Wed Sep 14 14:04:14 2016 +0100 @@ -143,7 +143,8 @@ exports jdk.internal.org.objectweb.asm.signature to jdk.scripting.nashorn; exports jdk.internal.loader to - java.instrument; + java.instrument, + java.logging; exports jdk.internal.math to java.desktop; exports jdk.internal.module to diff -r 46cd6d4f353f -r dba53de83476 jdk/src/java.logging/share/classes/java/util/logging/Level.java --- a/jdk/src/java.logging/share/classes/java/util/logging/Level.java Wed Sep 14 14:29:39 2016 +0200 +++ b/jdk/src/java.logging/share/classes/java/util/logging/Level.java Wed Sep 14 14:04:14 2016 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2016, 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 @@ -24,13 +24,22 @@ */ package java.util.logging; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; import java.lang.reflect.Module; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.ResourceBundle; +import java.util.function.Function; +import jdk.internal.loader.ClassLoaderValue; /** * The Level class defines a set of standard logging levels that @@ -177,6 +186,10 @@ */ public static final Level ALL = new Level("ALL", Integer.MIN_VALUE, defaultBundle); + private static final Level[] standardLevels = { + OFF, SEVERE, WARNING, INFO, CONFIG, FINE, FINER, FINEST, ALL + }; + /** * Create a named Level with a given integer value. *

@@ -267,7 +280,8 @@ // or its defining class loader, if it's unnamed module, // of this Level instance that can be a custom Level subclass; Module module = this.getClass().getModule(); - ResourceBundle rb = ResourceBundle.getBundle(resourceBundleName, newLocale, module); + ResourceBundle rb = ResourceBundle.getBundle(resourceBundleName, + newLocale, module); final String localizedName = rb.getString(name); final boolean isDefaultBundle = defaultBundle.equals(resourceBundleName); @@ -350,12 +364,12 @@ throw new NullPointerException(); } - KnownLevel level; + Optional level; // Look for a known Level with the given non-localized name. - level = KnownLevel.findByName(name); - if (level != null) { - return level.mirroredLevel; + level = KnownLevel.findByName(name, KnownLevel::mirrored); + if (level.isPresent()) { + return level.get(); } // Now, check if the given name is an integer. If so, @@ -363,21 +377,24 @@ // if necessary create one. try { int x = Integer.parseInt(name); - level = KnownLevel.findByValue(x); - if (level == null) { + level = KnownLevel.findByValue(x, KnownLevel::mirrored); + if (!level.isPresent()) { // add new Level Level levelObject = new Level(name, x); - level = KnownLevel.findByValue(x); + // There's no need to use a reachability fence here because + // KnownLevel keeps a strong reference on the level when + // level.getClass() == Level.class. + return KnownLevel.findByValue(x, KnownLevel::mirrored).get(); } - return level.mirroredLevel; } catch (NumberFormatException ex) { // Not an integer. // Drop through. } - level = KnownLevel.findByLocalizedLevelName(name); - if (level != null) { - return level.mirroredLevel; + level = KnownLevel.findByLocalizedLevelName(name, + KnownLevel::mirrored); + if (level.isPresent()) { + return level.get(); } return null; @@ -408,15 +425,13 @@ // Serialization magic to prevent "doppelgangers". // This is a performance optimization. private Object readResolve() { - KnownLevel o = KnownLevel.matches(this); - if (o != null) { - return o.levelObject; + Optional level = KnownLevel.matches(this); + if (level.isPresent()) { + return level.get(); } - // Woops. Whoever sent us this object knows // about a new log level. Add it to our list. - Level level = new Level(this.name, this.value, this.resourceBundleName); - return level; + return new Level(this.name, this.value, this.resourceBundleName); } /** @@ -450,12 +465,12 @@ // Check that name is not null. name.length(); - KnownLevel level; + Optional level; // Look for a known Level with the given non-localized name. - level = KnownLevel.findByName(name); - if (level != null) { - return level.levelObject; + level = KnownLevel.findByName(name, KnownLevel::referent); + if (level.isPresent()) { + return level.get(); } // Now, check if the given name is an integer. If so, @@ -463,13 +478,16 @@ // if necessary create one. try { int x = Integer.parseInt(name); - level = KnownLevel.findByValue(x); - if (level == null) { - // add new Level - Level levelObject = new Level(name, x); - level = KnownLevel.findByValue(x); + level = KnownLevel.findByValue(x, KnownLevel::referent); + if (level.isPresent()) { + return level.get(); } - return level.levelObject; + // add new Level. + Level levelObject = new Level(name, x); + // There's no need to use a reachability fence here because + // KnownLevel keeps a strong reference on the level when + // level.getClass() == Level.class. + return KnownLevel.findByValue(x, KnownLevel::referent).get(); } catch (NumberFormatException ex) { // Not an integer. // Drop through. @@ -478,9 +496,9 @@ // Finally, look for a known level with the given localized name, // in the current default locale. // This is relatively expensive, but not excessively so. - level = KnownLevel.findByLocalizedLevelName(name); - if (level != null) { - return level.levelObject; + level = KnownLevel.findByLocalizedLevelName(name, KnownLevel::referent); + if (level .isPresent()) { + return level.get(); } // OK, we've tried everything and failed @@ -530,22 +548,67 @@ // If Level.getName, Level.getLocalizedName, Level.getResourceBundleName methods // were final, the following KnownLevel implementation can be removed. // Future API change should take this into consideration. - static final class KnownLevel { + static final class KnownLevel extends WeakReference { private static Map> nameToLevels = new HashMap<>(); private static Map> intToLevels = new HashMap<>(); - final Level levelObject; // instance of Level class or Level subclass + private static final ReferenceQueue QUEUE = new ReferenceQueue<>(); + + // CUSTOM_LEVEL_CLV is used to register custom level instances with + // their defining class loader, so that they are garbage collected + // if and only if their class loader is no longer strongly + // referenced. + private static final ClassLoaderValue> CUSTOM_LEVEL_CLV = + new ClassLoaderValue<>(); + final Level mirroredLevel; // mirror of the custom Level KnownLevel(Level l) { - this.levelObject = l; + super(l, QUEUE); if (l.getClass() == Level.class) { this.mirroredLevel = l; } else { // this mirrored level object is hidden - this.mirroredLevel = new Level(l.name, l.value, l.resourceBundleName, false); + this.mirroredLevel = new Level(l.name, l.value, + l.resourceBundleName, false); } } + Optional mirrored() { + return Optional.of(mirroredLevel); + } + + Optional referent() { + return Optional.ofNullable(get()); + } + + private void remove() { + Optional.ofNullable(nameToLevels.get(mirroredLevel.name)) + .ifPresent((x) -> x.remove(this)); + Optional.ofNullable(intToLevels.get(mirroredLevel.value)) + .ifPresent((x) -> x.remove(this)); + } + + // Remove all stale KnownLevel instances + static synchronized void purge() { + Reference ref; + while ((ref = QUEUE.poll()) != null) { + if (ref instanceof KnownLevel) { + ((KnownLevel)ref).remove(); + } + } + } + + private static void registerWithClassLoader(Level customLevel) { + PrivilegedAction pa = + () -> customLevel.getClass().getClassLoader(); + PrivilegedAction pn = customLevel.getClass()::getName; + final String name = AccessController.doPrivileged(pn); + final ClassLoader cl = AccessController.doPrivileged(pa); + CUSTOM_LEVEL_CLV.computeIfAbsent(cl, (c, v) -> new ArrayList<>()) + .add(customLevel); + } + static synchronized void add(Level l) { + purge(); // the mirroredLevel object is always added to the list // before the custom Level instance KnownLevel o = new KnownLevel(l); @@ -562,24 +625,36 @@ intToLevels.put(l.value, list); } list.add(o); + + // keep the custom level reachable from its class loader + // This will ensure that custom level values are not GC'ed + // until there class loader is GC'ed. + if (o.mirroredLevel != l) { + registerWithClassLoader(l); + } + } // Returns a KnownLevel with the given non-localized name. - static synchronized KnownLevel findByName(String name) { - List list = nameToLevels.get(name); - if (list != null) { - return list.get(0); - } - return null; + static synchronized Optional findByName(String name, + Function> selector) { + purge(); + return nameToLevels.getOrDefault(name, Collections.emptyList()) + .stream() + .map(selector) + .flatMap(Optional::stream) + .findFirst(); } // Returns a KnownLevel with the given value. - static synchronized KnownLevel findByValue(int value) { - List list = intToLevels.get(value); - if (list != null) { - return list.get(0); - } - return null; + static synchronized Optional findByValue(int value, + Function> selector) { + purge(); + return intToLevels.getOrDefault(value, Collections.emptyList()) + .stream() + .map(selector) + .flatMap(Optional::stream) + .findFirst(); } // Returns a KnownLevel with the given localized name matching @@ -587,32 +662,34 @@ // from the resourceBundle associated with the Level object). // This method does not call Level.getLocalizedName() that may // be overridden in a subclass implementation - static synchronized KnownLevel findByLocalizedLevelName(String name) { - for (List levels : nameToLevels.values()) { - for (KnownLevel l : levels) { - String lname = l.levelObject.getLocalizedLevelName(); - if (name.equals(lname)) { - return l; - } - } - } - return null; + static synchronized Optional findByLocalizedLevelName(String name, + Function> selector) { + purge(); + return nameToLevels.values().stream() + .flatMap(List::stream) + .map(selector) + .flatMap(Optional::stream) + .filter(l -> name.equals(l.getLocalizedLevelName())) + .findFirst(); } - static synchronized KnownLevel matches(Level l) { + static synchronized Optional matches(Level l) { + purge(); List list = nameToLevels.get(l.name); if (list != null) { - for (KnownLevel level : list) { - Level other = level.mirroredLevel; + for (KnownLevel ref : list) { + Level levelObject = ref.get(); + if (levelObject == null) continue; + Level other = ref.mirroredLevel; if (l.value == other.value && (l.resourceBundleName == other.resourceBundleName || (l.resourceBundleName != null && l.resourceBundleName.equals(other.resourceBundleName)))) { - return level; + return Optional.of(levelObject); } } } - return null; + return Optional.empty(); } } diff -r 46cd6d4f353f -r dba53de83476 jdk/test/java/util/logging/Level/CustomLevel.java --- a/jdk/test/java/util/logging/Level/CustomLevel.java Wed Sep 14 14:29:39 2016 +0200 +++ b/jdk/test/java/util/logging/Level/CustomLevel.java Wed Sep 14 14:04:14 2016 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2016, 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 @@ -22,12 +22,20 @@ */ import java.io.*; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; import java.util.*; import java.util.logging.*; /* * @test - * @bug 8026027 + * @bug 8026027 6543126 * @summary Test Level.parse to look up custom levels by name and its * localized name * @@ -41,23 +49,168 @@ private static final List levels = new ArrayList<>(); private static final String RB_NAME = "myresource"; + private static final String OTHERRB_NAME = "myresource2"; + + private static class CustomLevelReference extends WeakReference { + final String name; + final int value; + final String resourceBundleName; + public CustomLevelReference(Level level, ReferenceQueue queue) { + super(level, queue); + name = level.getName(); + value = level.intValue(); + resourceBundleName = level.getResourceBundleName(); + } + + @Override + public String toString() { + return "CustomLevelReference(\"" + name + "\", " + value + ", \"" + + resourceBundleName + "\")"; + } + + } + public static void main(String[] args) throws Exception { setupCustomLevels(); + setUpCustomLevelsOtherLoader(); // Level.parse will return the custom Level instance - ResourceBundle rb = ResourceBundle.getBundle(RB_NAME); for (Level level : levels) { + final ResourceBundle rb = getResourceBundle(level); String name = level.getName(); - if (!name.equals("WARNING") && !name.equals("INFO")) { + Level l = Level.parse(name); + if (!name.equals("WARNING") && !name.equals("INFO") + && !name.equals("SEVERE")) { // custom level whose name doesn't conflict with any standard one - checkCustomLevel(Level.parse(name), level); + checkCustomLevel(l, level); + } else if (l != Level.WARNING && l != Level.INFO && l != Level.SEVERE + || !name.equals(l.getName())) { + throw new RuntimeException("Unexpected level " + formatLevel(l)); } + System.out.println("Level.parse found expected level: " + + formatLevel(l)); String localizedName = rb.getString(level.getName()); - Level l = Level.parse(localizedName); + l = Level.parse(localizedName); if (l != level) { - throw new RuntimeException("Unexpected level " + l + " " + l.getClass()); + throw new RuntimeException("Unexpected level " + l + " " + + l.getClass() + " for " + localizedName + + " in " + rb.getBaseBundleName()); + } + } + + final long otherLevelCount = levels.stream() + .filter(CustomLevel::isCustomLoader) + .count(); + + // Now verify that custom level instances are correctly + // garbage collected when no longer referenced + ReferenceQueue queue = new ReferenceQueue<>(); + List refs = new ArrayList<>(); + List customRefs = new ArrayList<>(); + int otherLevels = 0; + while (!levels.isEmpty()) { + Level l = levels.stream().findAny().get(); + boolean isCustomLoader = isCustomLoader(l); + if (isCustomLoader) otherLevels++; + + CustomLevelReference ref = new CustomLevelReference(l, queue); + if (isCustomLoader) { + customRefs.add(ref); + } else { + refs.add(ref); + } + + // remove strong references to l + levels.remove(l); + l = null; + + // Run gc and wait for garbage collection + if (otherLevels == otherLevelCount) { + if (customRefs.size() != otherLevelCount) { + throw new RuntimeException("Test bug: customRefs.size() != " + + otherLevelCount); + } + waitForGC(customRefs, queue); } } + if (otherLevelCount != otherLevels || otherLevelCount == 0) { + throw new RuntimeException("Test bug: " + + "no or wrong count of levels loaded from custom loader"); + } + if (!customRefs.isEmpty()) { + throw new RuntimeException( + "Test bug: customRefs.size() should be empty!"); + } + while (!refs.isEmpty()) { + final Reference ref = refs.remove(0); + if (ref.get() == null) { + throw new RuntimeException("Unexpected garbage collection for " + + ref); + } + } + } + + private static void waitForGC(List customRefs, + ReferenceQueue queue) + throws InterruptedException + { + while (!customRefs.isEmpty()) { + Reference ref2; + do { + System.gc(); + Thread.sleep(100); + } while ((ref2 = queue.poll()) == null); + + // Check garbage collected reference + if (!customRefs.contains(ref2)) { + throw new RuntimeException("Unexpected reference: " + ref2); + } + CustomLevelReference ref = customRefs.remove(customRefs.indexOf(ref2)); + System.out.println(ref2 + " garbage collected"); + final String name = ref.name; + Level l; + try { + l = Level.parse(name); + if (!name.equals("SEVERE") + && !name.equals("INFO") + || !name.equals(l.getName())) { + throw new RuntimeException("Unexpected level " + + formatLevel(l)); + } else { + if (l == Level.WARNING || l == Level.INFO + || l == Level.SEVERE) { + System.out.println("Level.parse found expected level: " + + formatLevel(l)); + } else { + throw new RuntimeException("Unexpected level " + + formatLevel(l)); + } + } + } catch (IllegalArgumentException iae) { + if (!name.equals("WARNING") + && !name.equals("INFO") + && !name.equals("SEVERE")) { + System.out.println("Level.parse fired expected exception: " + + iae); + } else { + throw iae; + } + } + } + } + + private static boolean isCustomLoader(Level level) { + final ClassLoader cl = level.getClass().getClassLoader(); + return cl != null + && cl != ClassLoader.getPlatformClassLoader() + && cl != ClassLoader.getSystemClassLoader(); + } + + static ResourceBundle getResourceBundle(Level level) { + return isCustomLoader(level) + ? ResourceBundle.getBundle(OTHERRB_NAME, Locale.getDefault(), + level.getClass().getClassLoader()) + : ResourceBundle.getBundle(RB_NAME); } private static void setupCustomLevels() throws IOException { @@ -67,22 +220,53 @@ levels.add(new CustomLevel("WARNING", 1010, RB_NAME)); levels.add(new CustomLevel("INFO", 1000, RB_NAME)); } + + static void setUpCustomLevelsOtherLoader() + throws MalformedURLException, + ClassNotFoundException, NoSuchMethodException, + IllegalAccessException, InvocationTargetException + { + final String classes = System.getProperty("test.classes", + "build/classes"); + final String sources = System.getProperty("test.src", + "src"); + final URL curl = new File(classes).toURI().toURL(); + final URL surl = new File(sources).toURI().toURL(); + URLClassLoader loader = new URLClassLoader(new URL[] {curl, surl}, + ClassLoader.getPlatformClassLoader()); + Class customLevelClass = Class.forName("CustomLevel", false, loader); + Method m = customLevelClass.getMethod("setUpCustomLevelsOtherLoader", + List.class); + m.invoke(null, levels); + } + + public static void setUpCustomLevelsOtherLoader(List levels) { + levels.add(new CustomLevel("OTHEREMERGENCY", 1091, OTHERRB_NAME)); + levels.add(new CustomLevel("OTHERALERT", 1061, OTHERRB_NAME)); + levels.add(new CustomLevel("OTHERCRITICAL", 1031, OTHERRB_NAME)); + levels.add(new CustomLevel("SEVERE", 1011, OTHERRB_NAME)); + levels.add(new CustomLevel("INFO", 1000, OTHERRB_NAME)); + } + static void checkCustomLevel(Level level, Level expected) { // Level value must be the same if (!level.equals(expected)) { - throw new RuntimeException(formatLevel(level) + " != " + formatLevel(expected)); + throw new RuntimeException(formatLevel(level) + " != " + + formatLevel(expected)); } if (!level.getName().equals(expected.getName())) { - throw new RuntimeException(formatLevel(level) + " != " + formatLevel(expected)); + throw new RuntimeException(formatLevel(level) + " != " + + formatLevel(expected)); } // Level.parse is expected to return the custom Level if (level != expected) { - throw new RuntimeException(formatLevel(level) + " != " + formatLevel(expected)); + throw new RuntimeException(formatLevel(level) + " != " + + formatLevel(expected)); } - ResourceBundle rb = ResourceBundle.getBundle(RB_NAME); + final ResourceBundle rb = getResourceBundle(level); String name = rb.getString(level.getName()); if (!level.getLocalizedName().equals(name)) { // must have the same localized name diff -r 46cd6d4f353f -r dba53de83476 jdk/test/java/util/logging/Level/myresource2.properties --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/java/util/logging/Level/myresource2.properties Wed Sep 14 14:04:14 2016 +0100 @@ -0,0 +1,5 @@ +OTHEREMERGENCY=localized.otheremergency +OTHERALERT=localized.otheralert +OTHERCRITICAL=localized.othercritical +SEVERE=localized.severe +INFO=localized.info.2