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
--- 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
--- 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.
* <p>
@@ -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> 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> 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> 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<Level> {
private static Map<String, List<KnownLevel>> nameToLevels = new HashMap<>();
private static Map<Integer, List<KnownLevel>> intToLevels = new HashMap<>();
- final Level levelObject; // instance of Level class or Level subclass
+ private static final ReferenceQueue<Level> 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<List<Level>> 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<Level> mirrored() {
+ return Optional.of(mirroredLevel);
+ }
+
+ Optional<Level> 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<? extends Level> ref;
+ while ((ref = QUEUE.poll()) != null) {
+ if (ref instanceof KnownLevel) {
+ ((KnownLevel)ref).remove();
+ }
+ }
+ }
+
+ private static void registerWithClassLoader(Level customLevel) {
+ PrivilegedAction<ClassLoader> pa =
+ () -> customLevel.getClass().getClassLoader();
+ PrivilegedAction<String> 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<KnownLevel> list = nameToLevels.get(name);
- if (list != null) {
- return list.get(0);
- }
- return null;
+ static synchronized Optional<Level> findByName(String name,
+ Function<KnownLevel, Optional<Level>> 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<KnownLevel> list = intToLevels.get(value);
- if (list != null) {
- return list.get(0);
- }
- return null;
+ static synchronized Optional<Level> findByValue(int value,
+ Function<KnownLevel, Optional<Level>> 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<KnownLevel> levels : nameToLevels.values()) {
- for (KnownLevel l : levels) {
- String lname = l.levelObject.getLocalizedLevelName();
- if (name.equals(lname)) {
- return l;
- }
- }
- }
- return null;
+ static synchronized Optional<Level> findByLocalizedLevelName(String name,
+ Function<KnownLevel, Optional<Level>> 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<Level> matches(Level l) {
+ purge();
List<KnownLevel> 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();
}
}
--- 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<Level> levels = new ArrayList<>();
private static final String RB_NAME = "myresource";
+ private static final String OTHERRB_NAME = "myresource2";
+
+ private static class CustomLevelReference extends WeakReference<Level> {
+ final String name;
+ final int value;
+ final String resourceBundleName;
+ public CustomLevelReference(Level level, ReferenceQueue<Level> 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<Level> queue = new ReferenceQueue<>();
+ List<CustomLevelReference> refs = new ArrayList<>();
+ List<CustomLevelReference> 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<CustomLevelReference> customRefs,
+ ReferenceQueue<Level> queue)
+ throws InterruptedException
+ {
+ while (!customRefs.isEmpty()) {
+ Reference<? extends Level> 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<Level> 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
--- /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