Remove synchronization around logging of messages in order to avoid potential deadlock - see PR 45194

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@669464 13f79535-47bb-0310-9956-ffa450edef68

WHATSNEW

@@ -38,6 +38,12 @@ Changes that could break older environments:
   to do so however using a new parameter.
   Bugzilla report 33969.
   
+* A lock in Project ensured that a BuildListener's messageLogged
+  method was only ever executed by a single thread at a time, while
+  all other methods could be invoked by multiple threads
+  simultaniously (while within <parallel>, for example).  This lock is
+  no longer in place, messageLogged should be made thread-safe now.
+
 Fixed bugs:
 -----------
 
@@ -63,6 +69,10 @@ Fixed bugs:
   characters in them.
   Bugzilla report 45190
 
+ * A deadlock could occur if a BuildListener tried to access an Ant property
+   within messageLogged while a different thread also accessed one.
+   Bugzilla report 45194
+
 Other changes:
 --------------
 

docs/manual/develop.html

@@ -498,6 +498,11 @@ not access System.out and System.err directly. It must use the streams with whic
 been configured.
 </p>
 
+<p><b>Note2:</b> All methods of a BuildListener except for the "Build
+  Started" and "Build Finished" events may occur on several threads
+  simultaneously - for example while Ant is executing
+  a <code>&lt;parallel&gt;</code> task.</p>
+
 <hr>
 <h2><a name="integration">Source code integration</a></h2>
 

src/main/org/apache/tools/ant/NoBannerLogger.java

@@ -48,7 +48,7 @@ public class NoBannerLogger extends DefaultLogger {
      * @param event A BuildEvent containing target information.
      *              Must not be <code>null</code>.
      */
-    public void targetStarted(BuildEvent event) {
+    public synchronized void targetStarted(BuildEvent event) {
         targetName = extractTargetName(event);
     }
 
@@ -67,7 +67,7 @@ public class NoBannerLogger extends DefaultLogger {
      *
      * @param event Ignored in this implementation.
      */
-    public void targetFinished(BuildEvent event) {
+    public synchronized void targetFinished(BuildEvent event) {
         targetName = null;
     }
 
@@ -88,9 +88,11 @@ public class NoBannerLogger extends DefaultLogger {
                 return;
         }
 
-        if (null != targetName) {
-            out.println(StringUtils.LINE_SEP + targetName + ":");
-            targetName = null;
+        synchronized (this) {
+            if (null != targetName) {
+                out.println(StringUtils.LINE_SEP + targetName + ":");
+                targetName = null;
+            }
         }
 
         super.messageLogged(event);

src/main/org/apache/tools/ant/Project.java

@@ -172,6 +172,14 @@ public class Project implements ResourceFactory {
     /** List of listeners to notify of build events. */
     private Vector listeners = new Vector();
 
+    /** for each thread, record whether it is currently executing
+        messageLogged */
+    private final ThreadLocal isLoggingMessage = new ThreadLocal() {
+            protected Object initialValue() {
+                return Boolean.FALSE;
+            }
+        };
+
     /**
      * The Ant core classloader--may be <code>null</code> if using
      * parent classloader.
@@ -201,11 +209,6 @@ public class Project implements ResourceFactory {
      */
     private boolean keepGoingMode = false;
 
-    /**
-     * Flag which catches Listeners which try to use System.out or System.err .
-     */
-    private boolean loggingMessage = false;
-
     /**
      * Set the input handler.
      *
@@ -2144,8 +2147,7 @@ public class Project implements ResourceFactory {
         } else {
             event.setMessage(message, priority);
         }
-        synchronized (this) {
-            if (loggingMessage) {
+            if (isLoggingMessage.get() != Boolean.FALSE) {
                 /*
                  * One of the Listeners has attempted to access
                  * System.err or System.out.
@@ -2162,16 +2164,15 @@ public class Project implements ResourceFactory {
                 return;
             }
             try {
-                loggingMessage = true;
+                isLoggingMessage.set(Boolean.TRUE);
                 Iterator iter = listeners.iterator();
                 while (iter.hasNext()) {
                     BuildListener listener = (BuildListener) iter.next();
                     listener.messageLogged(event);
                 }
             } finally {
-                loggingMessage = false;
+                isLoggingMessage.set(Boolean.FALSE);
             }
-        }
     }
 
     /**

src/main/org/apache/tools/ant/listener/Log4jListener.java

@@ -34,7 +34,7 @@ import org.apache.tools.ant.Task;
 public class Log4jListener implements BuildListener {
 
     /** Indicates if the listener was initialized. */
-    private boolean initialized = false;
+    private final boolean initialized;
 
     /**
      * log category we log into