6543126: Level.known can leak memory

Custom level instances will now be released when their defining class loader is no longer referenced.

Reviewed-by: plevart, mchung, chegar

jdk/src/java.base/share/classes/module-info.java

@@ -143,7 +143,8 @@ module java.base {
     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

jdk/src/java.logging/share/classes/java/util/logging/Level.java

@@ -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 class Level implements java.io.Serializable {
      */
     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 @@ public class Level implements java.io.Serializable {
         // 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 @@ public class Level implements java.io.Serializable {
             throw new NullPointerException();
         }
 
-        KnownLevel level;
+        Optional<Level> level;
 
         // Look for a known Level with the given non-localized name.
-        level = KnownLevel.findByName(name);
+        level = KnownLevel.findByName(name, KnownLevel::mirrored);
-        if (level != null) {
+        if (level.isPresent()) {
-            return level.mirroredLevel;
+            return level.get();
         }
 
         // Now, check if the given name is an integer.  If so,
@@ -363,21 +377,24 @@ public class Level implements java.io.Serializable {
         // if necessary create one.
         try {
             int x = Integer.parseInt(name);
-            level = KnownLevel.findByValue(x);
+            level = KnownLevel.findByValue(x, KnownLevel::mirrored);
-            if (level == null) {
+            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);
+        level = KnownLevel.findByLocalizedLevelName(name,
-        if (level != null) {
+                KnownLevel::mirrored);
-            return level.mirroredLevel;
+        if (level.isPresent()) {
+            return level.get();
         }
 
         return null;
@@ -408,15 +425,13 @@ public class Level implements java.io.Serializable {
     // Serialization magic to prevent "doppelgangers".
     // This is a performance optimization.
     private Object readResolve() {
-        KnownLevel o = KnownLevel.matches(this);
+        Optional<Level> level = KnownLevel.matches(this);
-        if (o != null) {
+        if (level.isPresent()) {
-            return o.levelObject;
+            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 new Level(this.name, this.value, this.resourceBundleName);
-        return level;
     }
 
     /**
@@ -450,12 +465,12 @@ public class Level implements java.io.Serializable {
         // 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);
+        level = KnownLevel.findByName(name, KnownLevel::referent);
-        if (level != null) {
+        if (level.isPresent()) {
-            return level.levelObject;
+            return level.get();
         }
 
         // Now, check if the given name is an integer.  If so,
@@ -463,13 +478,16 @@ public class Level implements java.io.Serializable {
         // if necessary create one.
         try {
             int x = Integer.parseInt(name);
-            level = KnownLevel.findByValue(x);
+            level = KnownLevel.findByValue(x, KnownLevel::referent);
-            if (level == null) {
+            if (level.isPresent()) {
-                // add new Level
+                return level.get();
-                Level levelObject = new Level(name, x);
-                level = KnownLevel.findByValue(x);
             }
-            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 @@ public class Level implements java.io.Serializable {
         // 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);
+        level = KnownLevel.findByLocalizedLevelName(name, KnownLevel::referent);
-        if (level != null) {
+        if (level .isPresent()) {
-            return level.levelObject;
+            return level.get();
         }
 
         // OK, we've tried everything and failed
@@ -530,22 +548,67 @@ public class Level implements java.io.Serializable {
     // 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 @@ public class Level implements java.io.Serializable {
                 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) {
+        static synchronized Optional<Level> findByName(String name,
-            List<KnownLevel> list = nameToLevels.get(name);
+                Function<KnownLevel, Optional<Level>> selector) {
-            if (list != null) {
+            purge();
-                return list.get(0);
+            return nameToLevels.getOrDefault(name, Collections.emptyList())
-            }
+                        .stream()
-            return null;
+                        .map(selector)
+                        .flatMap(Optional::stream)
+                        .findFirst();
         }
 
         // Returns a KnownLevel with the given value.
-        static synchronized KnownLevel findByValue(int value) {
+        static synchronized Optional<Level> findByValue(int value,
-            List<KnownLevel> list = intToLevels.get(value);
+                Function<KnownLevel, Optional<Level>> selector) {
-            if (list != null) {
+            purge();
-                return list.get(0);
+            return intToLevels.getOrDefault(value, Collections.emptyList())
-            }
+                        .stream()
-            return null;
+                        .map(selector)
+                        .flatMap(Optional::stream)
+                        .findFirst();
         }
 
         // Returns a KnownLevel with the given localized name matching
@@ -587,32 +662,34 @@ public class Level implements java.io.Serializable {
         // 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) {
+        static synchronized Optional<Level> findByLocalizedLevelName(String name,
-            for (List<KnownLevel> levels : nameToLevels.values()) {
+                Function<KnownLevel, Optional<Level>> selector) {
-                for (KnownLevel l : levels) {
+            purge();
-                    String lname = l.levelObject.getLocalizedLevelName();
+            return nameToLevels.values().stream()
-                    if (name.equals(lname)) {
+                         .flatMap(List::stream)
-                        return l;
+                         .map(selector)
-                    }
+                         .flatMap(Optional::stream)
-                }
+                         .filter(l -> name.equals(l.getLocalizedLevelName()))
-            }
+                         .findFirst();
-            return null;
         }
 
-        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) {
+                for (KnownLevel ref : list) {
-                    Level other = level.mirroredLevel;
+                    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();
         }
     }
 

jdk/test/java/util/logging/Level/CustomLevel.java

@@ -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 @@ public class CustomLevel extends Level {
 
     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 @@ public class CustomLevel extends Level {
         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

jdk/test/java/util/logging/Level/myresource2.properties

@@ -0,0 +1,5 @@
+OTHEREMERGENCY=localized.otheremergency
+OTHERALERT=localized.otheralert
+OTHERCRITICAL=localized.othercritical
+SEVERE=localized.severe
+INFO=localized.info.2