Google engnieers found concurrency problem in logger and

received the following note from them:
------------------------------------------------------------------------

Description:
The removal of unnecessary optimization below
(replacing synchronized with j.u.c. or volatile replacements)
made a big difference in one of our local applications with
many concurrently logging threads.

As well, making levelObject volatile slightly improves the thread-safety
of Logger.

Fix written by Jeremy Manson and reviewed by Bill Pugh
(both of JSR-133 fame), and by myself.

There are further concurrency improvements that can be made to this code,
but we are not that ambitious.

# HG changeset patch
# User martin
# Date 1232071338 28800
# Node ID 7ac6b44ba1709d554282cc16b5b8b2c613bc1079
# Parent 7f6969c090750e1d389a93c3a657b60426d3d593
6666666: Remove synchronization bottlenecks in Logger
Reviewed-by: xxxxx
Contributed-by: jeremymanson

diff --git a/src/share/classes/java/util/logging/Logger.java
b/src/share/classes/java/util/logging/Logger.java
--- a/src/share/classes/java/util/logging/Logger.java
+++ b/src/share/classes/java/util/logging/Logger.java
@@ -27,6 +27,7 @@
 package java.util.logging;

 import java.util.*;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.security.*;
 import java.lang.ref.WeakReference;

@@ -165,10 +166,11 @@
     private static final int offValue = Level.OFF.intValue();
     private LogManager manager;
     private String name;
- private ArrayList<Handler> handlers;
+ private final CopyOnWriteArrayList<Handler> handlers =
+ new CopyOnWriteArrayList<Handler>();
     private String resourceBundleName;
- private boolean useParentHandlers = true;
- private Filter filter;
+ private volatile boolean useParentHandlers = true;
+ private volatile Filter filter;
     private boolean anonymous;

     private ResourceBundle catalog; // Cached resource bundle
@@ -180,9 +182,9 @@
     private static Object treeLock = new Object();
     // We keep weak references from parents to children, but strong
     // references from children to parents.
- private Logger parent; // our nearest parent.
+ private volatile Logger parent; // our nearest parent.
     private ArrayList<WeakReference<Logger>> kids; //
WeakReferences to loggers that have us as parent
- private Level levelObject;
+ private volatile Level levelObject;
     private volatile int levelValue; // current effective level value

     /**
@@ -438,7 +440,7 @@
      * @exception SecurityException if a security manager exists and if
      * the caller does not have LoggingPermission("control").
      */
- public synchronized void setFilter(Filter newFilter) throws
SecurityException {
+ public void setFilter(Filter newFilter) throws SecurityException {
         checkAccess();
         filter = newFilter;
     }
@@ -448,7 +450,7 @@
      *
      * @return a filter object (may be null)
      */
- public synchronized Filter getFilter() {
+ public Filter getFilter() {
         return filter;
     }

@@ -465,10 +467,9 @@
         if (record.getLevel().intValue() < levelValue || levelValue
== offValue) {
             return;
         }
- synchronized (this) {
- if (filter != null && !filter.isLoggable(record)) {
- return;
- }
+ Filter theFilter = filter;
+ if (theFilter != null && !theFilter.isLoggable(record)) {
+ return;
         }

         // Post the LogRecord to all our Handlers, and then to
@@ -1182,13 +1183,10 @@
      * @exception SecurityException if a security manager exists and if
      * the caller does not have LoggingPermission("control").
      */
- public synchronized void addHandler(Handler handler) throws
SecurityException {
+ public void addHandler(Handler handler) throws SecurityException {
         // Check for null handler
         handler.getClass();
         checkAccess();
- if (handlers == null) {
- handlers = new ArrayList<Handler>();
- }
         handlers.add(handler);
     }

@@ -1201,12 +1199,9 @@
      * @exception SecurityException if a security manager exists and if
      * the caller does not have LoggingPermission("control").
      */
- public synchronized void removeHandler(Handler handler) throws
SecurityException {
+ public void removeHandler(Handler handler) throws SecurityException {
         checkAccess();
         if (handler == null) {
- return;
- }
- if (handlers == null) {
             return;
         }
         handlers.remove(handler);
@@ -1217,11 +1212,8 @@
      * <p>
      * @return an array of all registered Handlers
      */
- public synchronized Handler[] getHandlers() {
- if (handlers == null) {
- return emptyHandlers;
- }
- return handlers.toArray(new Handler[handlers.size()]);
+ public Handler[] getHandlers() {
+ return handlers.toArray(emptyHandlers);
     }

     /**
@@ -1235,7 +1227,7 @@
      * @exception SecurityException if a security manager exists and if
      * the caller does not have LoggingPermission("control").
      */
- public synchronized void setUseParentHandlers(boolean useParentHandlers) {
+ public void setUseParentHandlers(boolean useParentHandlers) {
         checkAccess();
         this.useParentHandlers = useParentHandlers;
     }
@@ -1246,7 +1238,7 @@
      *
      * @return true if output is to be sent to the logger's parent
      */
- public synchronized boolean getUseParentHandlers() {
+ public boolean getUseParentHandlers() {
         return useParentHandlers;
     }

@@ -1354,9 +1346,12 @@
      * @return nearest existing parent Logger
      */
     public Logger getParent() {
- synchronized (treeLock) {
- return parent;
- }
+ // Note: this used to be synchronized on treeLock. However, this only
+ // provided memory semantics, as there was no guarantee that the caller
+ // would synchronize on treeLock (in fact, there is no way for external
+ // callers to so synchronize. Therefore, we have made parent volatile
+ // instead.
+ return parent;
     }

     /**