United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6274920 JDK logger holds strong reference to java.util.logging.Logger instances
JDK-6274920 : JDK logger holds strong reference to java.util.logging.Logger instances

Details
Type:
Bug
Submit Date:
2005-05-23
Status:
Closed
Updated Date:
2012-10-01
Project Name:
JDK
Resolved Date:
2011-04-19
Component:
core-libs
OS:
windows_2003,linux,generic,solaris_10,windows_xp,windows_2000
Sub-Component:
java.util.logging
CPU:
x86,generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
1.4.1,1.4.2_06,6,6u18,7
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:
Duplicate:
Duplicate:
Duplicate:
Duplicate:
Relates:
Relates:
Relates:
Relates:
Relates:
Relates:

Sub Tasks

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
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);
  		}
  	    }
  	}
                                     
2006-08-30
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.
                                     
2006-08-30
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.
                                     
2009-07-07
SUGGESTED FIX

<moved this note to the evaluation>
                                     
2010-05-05
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
                                     
2010-05-05



Hardware and Software, Engineered to Work Together