From a7e2a93ee49eb3d1cdd090d3909aa115959d55b5 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Thu, 19 Jun 2008 12:20:12 +0000 Subject: [PATCH] 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 | 10 +++++++++ docs/manual/develop.html | 5 +++++ .../org/apache/tools/ant/NoBannerLogger.java | 12 ++++++----- src/main/org/apache/tools/ant/Project.java | 21 ++++++++++--------- .../tools/ant/listener/Log4jListener.java | 2 +- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/WHATSNEW b/WHATSNEW index 1d18d3d66..45cec34d9 100644 --- a/WHATSNEW +++ b/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 , 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: -------------- diff --git a/docs/manual/develop.html b/docs/manual/develop.html index 88ff90e8b..1544aaeb8 100644 --- a/docs/manual/develop.html +++ b/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.

+

Note2: 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 <parallel> task.

+

Source code integration

diff --git a/src/main/org/apache/tools/ant/NoBannerLogger.java b/src/main/org/apache/tools/ant/NoBannerLogger.java index 956e3c58c..a086bf5e0 100644 --- a/src/main/org/apache/tools/ant/NoBannerLogger.java +++ b/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 null. */ - 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); diff --git a/src/main/org/apache/tools/ant/Project.java b/src/main/org/apache/tools/ant/Project.java index c64d6a781..4ab8ee301 100644 --- a/src/main/org/apache/tools/ant/Project.java +++ b/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 null 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); } - } } /** diff --git a/src/main/org/apache/tools/ant/listener/Log4jListener.java b/src/main/org/apache/tools/ant/listener/Log4jListener.java index 059d1e56c..bf8d73390 100644 --- a/src/main/org/apache/tools/ant/listener/Log4jListener.java +++ b/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