JDK-6274920 : JDK logger holds strong reference to java.util.logging.Logger instances
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.logging
  • Affected Version: 1.4.1,1.4.2_06,6,6u18,7
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS:
    generic,linux,solaris_10,windows_2000,windows_2003,windows_xp generic,linux,solaris_10,windows_2000,windows_2003,windows_xp
  • CPU: generic,x86
  • Submitted: 2005-05-23
  • Updated: 2012-10-01
  • Resolved: 2011-04-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7
6u16-revFixed 7 b03Fixed
Related Reports
Duplicate :  
JDK-6717784 - Memory leak in LogManager when used with subclassed resource bundle
Duplicate :  
JDK-4811930 - RFE: Cannot remove Loggers from LogManager
Duplicate :  
JDK-2180192 - nsk/logging/Logger/getParent/getparent001, ../LogManager/getLoggerNames/getlogrn001 fail in b03 PIT
Duplicate :  
JDK-4952316 - RFE: Can't "release" logger instances
Relates :  
JDK-7067691 - java/lang/management/PlatformLoggingMXBean/LoggingMXBeanTest.java failing intermittently
Relates :  
JDK-6498300 - nsk/logging/Logger/getParent/getparent001, ../LogManager/getLoggerNames/getlogrn001 fail in b03 PIT
Relates :  
JDK-6543126 - Level.known can leak memory
Relates :  
JDK-6947908 - JAVA Logging interface is not threadsafe since 1.6.0_18
Relates :  
JDK-6949710 - the GC'able nature of Logging objects needs to be made brutally clear
Relates :  
JDK-6942989 - Memory leak of java.lang.ref.WeakReference objects
Description
In the package java.util.logging, an instance of Logger is obtained using the Logger.getLogger() method, the returned instance is held permanently for the life of the JVM by a strong reference inside the log manager. This is causing some problems in the JBI (JSR208) Reference Implementation. One of the API methods we must provide is a method to allow a JBI plug-in component to request a logger. We in turn use the standard Logger implementation to request a logger on behalf of the component and then return that instance to the component. We support an optional resource bundle provided by the component to be used by that logger.
The problem is that during the lifetime of a single execution of JBI, which is a Lifecycle Module running inside the AppServer, a JBI plug-in component could be installed, started, shut down, and uninstalled any number of times. This could be an upgrade of the JBI plug-in component, which could include updates to its resource bundle. However, because the J2SE log manager never releases the Logger instance, when the JBI plug-in component requests a logger when it is being restarted, it still gets the original Logger instance which has the out-of-date resource bundle.
It seems there should be a way to force the log manager to release a Logger instance that is no longer being used so that situations like this would not occur. We've tried forcing a GC run after releasing all of our references to the logger (when the plug-in component is shut down), but this does not help.
Refer to CR #6263011 for more information.
###@###.### 2005-05-23 23:42:48 GMT
Comments
EVALUATION <moved this note here from the suggested fix section> The fix for this bug has correctly made Logger objects GC'able. However, the JavaDoc could be more clear about the GC'able nature of Logging objects. See: 6949710 3/3 the GC'able nature of Logging objects needs to be made brutally clear
05-05-2010
SUGGESTED FIX <moved this note to the evaluation>
05-05-2010
SUGGESTED FIX The fix for this bug (6274920) introduced a regression that was fixed the following bug: 6498300 2/2 nsk/logging/Logger/getParent/getparent001, ../LogManager/getLoggerNames/getlogrn001 fail in b03 PIT 6498300 was fixed in JDK7-B12.
07-07-2009
SUGGESTED FIX Please, see a suggested fix below: % sccs sccsdiff -r1.51 -r1.53 -c LogManager.java ------- LogManager.java ------- *** /tmp/geta10520 Mon Sep 4 09:27:22 2006 --- /tmp/getb10520 Mon Sep 4 09:27:22 2006 *************** *** 11,16 **** --- 11,17 ---- import java.io.*; import java.util.*; import java.security.*; + import java.lang.ref.WeakReference; import java.beans.PropertyChangeListener; import java.beans.PropertyChangeSupport; import java.net.URL; *************** *** 137,143 **** private final static Level defaultLevel = Level.INFO; // Table of known loggers. Maps names to Loggers. ! private Hashtable<String,Logger> loggers = new Hashtable<String,Logger>(); // Tree of known loggers private LogNode root = new LogNode(null); private Logger rootLogger; --- 138,145 ---- private final static Level defaultLevel = Level.INFO; // Table of known loggers. Maps names to Loggers. ! private Hashtable<String,WeakReference<Logger>> loggers = ! new Hashtable<String,WeakReference<Logger>>(); // Tree of known loggers private LogNode root = new LogNode(null); private Logger rootLogger; *************** *** 302,307 **** --- 304,355 ---- changes.removePropertyChangeListener(l); } + // Add new per logger handlers. + // We need to raise privilege here. All our decisions will + // be made based on the logging configuration, which can + // only be modified by trusted code. + private void loadLoggerHandlers(final Logger logger, final String handlersPropertyName, + final String name, final boolean addingHandlers) { + AccessController.doPrivileged(new PrivilegedAction() { + public Object run() { + String names[] = parseClassNames(handlersPropertyName); + for (int i = 0; i < names.length; i++) { + String word = names[i]; + try { + Class clz = ClassLoader.getSystemClassLoader().loadClass(word); + Handler h = (Handler) clz.newInstance(); + try { + // Check if there is a property defining the + // this handler's level. + String levs = getProperty(word + ".level"); + if (levs != null) { + h.setLevel(Level.parse(levs)); + } + } catch (Exception ex) { + System.err.println("Can't set level for " + word); + // Probably a bad level. Drop through. + } + // Add this Handler to the logger + if (addingHandlers) { + logger.addHandler(h); + } + if (logger != rootLogger) { + boolean useParent = getBooleanProperty(name + + ".useParentHandlers", true); + if (!useParent) { + logger.setUseParentHandlers(false); + } + } + } catch (Exception ex) { + System.err.println("Can't load log handler \"" + word + "\""); + System.err.println("" + ex); + ex.printStackTrace(); + } + } + return null; + }}); + } + /** * Add a named logger. This does nothing and returns false if a logger * with the same name is already registered. *************** *** 324,339 **** throw new NullPointerException(); } ! Logger old = loggers.get(name); ! if (old != null) { ! // We already have a registered logger with the given name. ! return false; } // We're adding a new logger. ! // Note that we are creating a strong reference here that will ! // keep the Logger in existence indefinitely. ! loggers.put(name, logger); // Apply any initial level defined for the new logger. Level level = getLevelProperty(name+".level", null); --- 372,393 ---- throw new NullPointerException(); } ! WeakReference<Logger> ref = loggers.get(name); ! if (ref != null) { ! if (ref.get() == null) { ! // Hashtable holds stale weak reference ! // to a logger which has been GC-ed. ! // Allow to register new one. ! loggers.remove(name); ! } else { ! // We already have a registered logger with the given name. ! return false; ! } } // We're adding a new logger. ! // Note that we are creating a weak reference here. ! loggers.put(name, new WeakReference(logger)); // Apply any initial level defined for the new logger. Level level = getLevelProperty(name+".level", null); *************** *** 344,384 **** // Do we have a per logger handler too? // Note: this will add a 200ms penalty if (getProperty(name+".handlers") != null) { ! // This code is taken from the root handler initialization ! AccessController.doPrivileged(new PrivilegedAction() { ! public Object run() { ! // Add new per logger handlers. ! String names[] = parseClassNames(name+".handlers"); ! for (int i = 0; i < names.length; i++) { ! String word = names[i]; ! try { ! Class clz = ClassLoader.getSystemClassLoader().loadClass(word); ! Handler h = (Handler) clz.newInstance(); ! try { ! // Check if there is a property defining the ! // this handler's level. ! String levs = getProperty(word + ".level"); ! if (levs != null) { ! h.setLevel(Level.parse(levs)); ! } ! boolean useParent = getBooleanProperty(name + ".useParentHandlers", true); ! if (!useParent) { ! getLogger(name).setUseParentHandlers(false); ! } ! } catch (Exception ex) { ! System.err.println("Can't set level for " + word); ! // Probably a bad level. Drop through. ! } ! // Add this Handler to the logger ! getLogger(name).addHandler(h); ! } catch (Exception ex) { ! System.err.println("Can't load log handler \"" + word + "\""); ! System.err.println("" + ex); ! ex.printStackTrace(); ! } ! } ! return null; ! }}); } // do we have per logger handlers // If any of the logger's parents have levels defined, --- 398,404 ---- // Do we have a per logger handler too? // Note: this will add a 200ms penalty if (getProperty(name+".handlers") != null) { ! loadLoggerHandlers(logger, name+".handlers", name, true); } // do we have per logger handlers // If any of the logger's parents have levels defined, *************** *** 390,441 **** break; } String pname = name.substring(0,ix2); ! if (getProperty(pname+".level") != null) { ! // This pname has a level definition. Make sure it exists. ! Logger plogger = Logger.getLogger(pname); ! } // While we are walking up the tree I can check for our // own root logger and get its handlers initialized too with ! // the same code if (getProperty(pname+".handlers") != null) { ! final String nname=pname; ! ! AccessController.doPrivileged(new PrivilegedAction() { ! public Object run() { ! String names[] = parseClassNames(nname+".handlers"); ! ! for (int i = 0; i < names.length; i++) { ! String word = names[i]; ! try { ! Class clz = ClassLoader.getSystemClassLoader().loadClass(word); ! Handler h = (Handler) clz.newInstance(); ! try { ! // Check if there is a property defining the ! // handler's level. ! String levs = getProperty(word + ".level"); ! if (levs != null) { ! h.setLevel(Level.parse(levs)); ! } ! } catch (Exception ex) { ! System.err.println("Can't set level for " + word); ! // Probably a bad level. Drop through. ! } ! if (getLogger(nname) == null ) { ! Logger nplogger=Logger.getLogger(nname); ! addLogger(nplogger); ! } ! boolean useParent = getBooleanProperty(nname + ".useParentHandlers", true); ! if (!useParent) { ! getLogger(nname).setUseParentHandlers(false); ! } ! } catch (Exception ex) { ! System.err.println("Can't load log handler \"" + word + "\""); ! System.err.println("" + ex); ! ex.printStackTrace(); ! } ! } ! return null; ! }}); } //found a parent handler ix = ix2+1; --- 410,422 ---- break; } String pname = name.substring(0,ix2); ! // While we are walking up the tree I can check for our // own root logger and get its handlers initialized too with ! // the same code. if (getProperty(pname+".handlers") != null) { ! loadLoggerHandlers(Logger.getLogger(pname), ! pname+".handlers", pname, false); } //found a parent handler ix = ix2+1; *************** *** 443,455 **** // Find the new node and its parent. LogNode node = findNode(name); ! node.logger = logger; Logger parent = null; LogNode nodep = node.parent; while (nodep != null) { ! if (nodep.logger != null) { ! parent = nodep.logger; ! break; } nodep = nodep.parent; } --- 424,439 ---- // Find the new node and its parent. LogNode node = findNode(name); ! node.loggerRef = new WeakReference(logger); Logger parent = null; LogNode nodep = node.parent; while (nodep != null) { ! WeakReference<Logger> nodeRef = nodep.loggerRef; ! if (nodeRef != null) { ! parent = nodeRef.get(); ! if (parent != null) { ! break; ! } } nodep = nodep.parent; } *************** *** 543,549 **** * @return matching logger or null if none is found */ public synchronized Logger getLogger(String name) { ! return loggers.get(name); } /** --- 527,543 ---- * @return matching logger or null if none is found */ public synchronized Logger getLogger(String name) { ! WeakReference<Logger> ref = loggers.get(name); ! if (ref == null) { ! return null; ! } ! Logger logger = ref.get(); ! if (logger == null) { ! // Hashtable holds stale weak reference ! // to a logger which has been GC-ed. ! loggers.remove(name); ! } ! return logger; } /** *************** *** 555,560 **** --- 549,567 ---- * @return enumeration of logger name strings */ public synchronized Enumeration<String> getLoggerNames() { + Enumeration<String> keys = loggers.keys(); + + // This loop is needed for consistency. Some of the loggers + // can be GC-ed, so we usually delete them in such a case. + // However, we want this function to return the corresponding state. + while (keys.hasMoreElements()) { + String key = (String) keys.nextElement(); + WeakReference ref = loggers.get(key); + if (loggers.get(key).get() == null) { + // Stale reference - delete the entry + loggers.remove(key); + } + } return loggers.keys(); } *************** *** 876,914 **** // Avoid allocating global handlers. return; } ! ! // We need to raise privilege here. All our decisions will ! // be made based on the logging configuration, which can ! // only be modified by trusted code. ! AccessController.doPrivileged(new PrivilegedAction() { ! public Object run() { ! // Add new global handlers. ! String names[] = parseClassNames("handlers"); ! for (int i = 0; i < names.length; i++) { ! String word = names[i]; ! try { ! Class clz = ClassLoader.getSystemClassLoader().loadClass(word); ! Handler h = (Handler) clz.newInstance(); ! try { ! // Check if there is a property defining the ! // handler's level. ! String levs = getProperty(word + ".level"); ! if (levs != null) { ! h.setLevel(Level.parse(levs)); ! } ! } catch (Exception ex) { ! System.err.println("Can't set level for " + word); ! // Probably a bad level. Drop through. ! } ! rootLogger.addHandler(h); ! } catch (Exception ex) { ! System.err.println("Can't load log handler \"" + word + "\""); ! System.err.println("" + ex); ! ex.printStackTrace(); ! } ! } ! return null; ! }}); } --- 883,889 ---- // Avoid allocating global handlers. return; } ! loadLoggerHandlers(rootLogger, "handlers", null, true); } *************** *** 935,941 **** // Nested class to represent a node in our tree of named loggers. private static class LogNode { HashMap<Object,Object> children; ! Logger logger; LogNode parent; LogNode(LogNode parent) { --- 910,916 ---- // Nested class to represent a node in our tree of named loggers. private static class LogNode { HashMap<Object,Object> children; ! WeakReference<Logger> loggerRef; LogNode parent; LogNode(LogNode parent) { *************** *** 951,960 **** Iterator values = children.values().iterator(); while (values.hasNext()) { LogNode node = (LogNode) values.next(); ! if (node.logger == null) { node.walkAndSetParent(parent); } else { ! doSetParent(node.logger, parent); } } } --- 926,936 ---- Iterator values = children.values().iterator(); while (values.hasNext()) { LogNode node = (LogNode) values.next(); ! WeakReference<Logger> ref = node.loggerRef; ! if (ref == null || ref.get() == null) { node.walkAndSetParent(parent); } else { ! doSetParent(ref.get(), parent); } } }
30-08-2006
EVALUATION Yes, the LogManager saves the strong references to the Logger objects in the loggers hashtable. Please, see the code fragment from LogManager.java below: // We're adding a new logger. // Note that we are creating a strong reference here that will // keep the Logger in existence indefinitely. loggers.put(name, logger); This is is against the spec and has to be fixed.
30-08-2006