You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Maarten Coene <ma...@yahoo.com> on 2014/03/25 13:27:08 UTC
Re: svn commit: r1553704 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
Maybe a bit late, but what was the reason to use a ThreadLocal here?
This gives issues when the configuration of Ivy (for instance setting a custom logger) is done from a different thread than the actual resolve.
Maarten
________________________________
Van: "cduffy@apache.org" <cd...@apache.org>
Aan: notifications@ant.apache.org
Verzonden: vrijdag 27 december 18:40 2013
Onderwerp: svn commit: r1553704 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
Author: cduffy
Date: Fri Dec 27 17:40:20 2013
New Revision: 1553704
URL: http://svn.apache.org/r1553704
Log:
Move non-root MessageLogger instances into a thread-local stack
Logger context is inherently thread-local.
Modified:
ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
Modified: ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
URL: http://svn.apache.org/viewvc/ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java?rev=1553704&r1=1553703&r2=1553704&view=diff
==============================================================================
--- ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java (original)
+++ ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java Fri Dec 27 17:40:20 2013
@@ -34,7 +34,7 @@ import java.util.Stack;
* </p>
*/
public class MessageLoggerEngine implements MessageLogger {
- private final Stack/*<MessageLogger>*/ loggerStack = new Stack();
+ private final ThreadLocal/*<Stack<MessageLogger>>*/ loggerStacks = new ThreadLocal();
private MessageLogger defaultLogger = null;
@@ -44,6 +44,15 @@ public class MessageLoggerEngine impleme
private List errors = new ArrayList();
+ private Stack getLoggerStack() {
+ Stack stack = (Stack) loggerStacks.get();
+ if (stack == null) {
+ stack = new Stack();
+ loggerStacks.set(stack);
+ }
+ return stack;
+ }
+
public MessageLoggerEngine() {
}
@@ -66,7 +75,7 @@ public class MessageLoggerEngine impleme
*/
public void pushLogger(MessageLogger logger) {
Checks.checkNotNull(logger, "logger");
- loggerStack.push(logger);
+ getLoggerStack().push(logger);
}
/**
@@ -76,8 +85,8 @@ public class MessageLoggerEngine impleme
* </p>
*/
public void popLogger() {
- if (!loggerStack.isEmpty()) {
- loggerStack.pop();
+ if (!getLoggerStack().isEmpty()) {
+ getLoggerStack().pop();
}
}
@@ -86,10 +95,10 @@ public class MessageLoggerEngine impleme
* @return the current logger, or the default one if there is no logger in the stack
*/
public MessageLogger peekLogger() {
- if (loggerStack.isEmpty()) {
+ if (getLoggerStack().isEmpty()) {
return getDefaultLogger();
}
- return (MessageLogger) loggerStack.peek();
+ return (MessageLogger) getLoggerStack().peek();
}
private MessageLogger getDefaultLogger() {
@@ -130,7 +139,7 @@ public class MessageLoggerEngine impleme
public void clearProblems() {
getDefaultLogger().clearProblems();
- for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
+ for (Iterator iter = getLoggerStack().iterator(); iter.hasNext();) {
MessageLogger l = (MessageLogger) iter.next();
l.clearProblems();
}
@@ -142,7 +151,7 @@ public class MessageLoggerEngine impleme
public void setShowProgress(boolean progress) {
getDefaultLogger().setShowProgress(progress);
// updates all loggers in the stack
- for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
+ for (Iterator iter = getLoggerStack().iterator(); iter.hasNext();) {
MessageLogger l = (MessageLogger) iter.next();
l.setShowProgress(progress);
}
Re: svn commit: r1553704 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
Posted by Matt Sicker <bo...@gmail.com>.
Oh man those logger call stacks are notoriously difficult to deal with.
On 1 April 2014 01:24, Charles Duffy <ch...@dyfis.net> wrote:
> The amount of "fun" otherwise involved when two separate threads called
> into Ivy code in parallel was considerable, and was regularly resulting in
> a logger stack with no relationship with any individual thread's call
> stack.
> On Mar 25, 2014 7:27 AM, "Maarten Coene" <ma...@yahoo.com> wrote:
>
> > Maybe a bit late, but what was the reason to use a ThreadLocal here?
> > This gives issues when the configuration of Ivy (for instance setting a
> > custom logger) is done from a different thread than the actual resolve.
> >
> > Maarten
> >
> >
> >
> > ________________________________
> > Van: "cduffy@apache.org" <cd...@apache.org>
> > Aan: notifications@ant.apache.org
> > Verzonden: vrijdag 27 december 18:40 2013
> > Onderwerp: svn commit: r1553704 -
> > /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> >
> >
> > Author: cduffy
> > Date: Fri Dec 27 17:40:20 2013
> > New Revision: 1553704
> >
> > URL: http://svn.apache.org/r1553704
> > Log:
> > Move non-root MessageLogger instances into a thread-local stack
> >
> > Logger context is inherently thread-local.
> >
> > Modified:
> >
> > ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> >
> > Modified:
> > ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> > URL:
> >
> http://svn.apache.org/viewvc/ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java?rev=1553704&r1=1553703&r2=1553704&view=diff
> >
> >
> ==============================================================================
> > ---
> > ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> > (original)
> > +++
> > ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> > Fri Dec 27 17:40:20 2013
> > @@ -34,7 +34,7 @@ import java.util.Stack;
> > * </p>
> > */
> > public class MessageLoggerEngine implements MessageLogger {
> > - private final Stack/*<MessageLogger>*/ loggerStack = new Stack();
> > + private final ThreadLocal/*<Stack<MessageLogger>>*/ loggerStacks =
> > new ThreadLocal();
> >
> > private MessageLogger defaultLogger = null;
> >
> > @@ -44,6 +44,15 @@ public class MessageLoggerEngine impleme
> >
> > private List errors = new ArrayList();
> >
> > + private Stack getLoggerStack() {
> > + Stack stack = (Stack) loggerStacks.get();
> > + if (stack == null) {
> > + stack = new Stack();
> > + loggerStacks.set(stack);
> > + }
> > + return stack;
> > + }
> > +
> > public MessageLoggerEngine() {
> > }
> >
> > @@ -66,7 +75,7 @@ public class MessageLoggerEngine impleme
> > */
> > public void pushLogger(MessageLogger logger) {
> > Checks.checkNotNull(logger, "logger");
> > - loggerStack.push(logger);
> > + getLoggerStack().push(logger);
> > }
> >
> > /**
> > @@ -76,8 +85,8 @@ public class MessageLoggerEngine impleme
> > * </p>
> > */
> > public void popLogger() {
> > - if (!loggerStack.isEmpty()) {
> > - loggerStack.pop();
> > + if (!getLoggerStack().isEmpty()) {
> > + getLoggerStack().pop();
> > }
> > }
> >
> > @@ -86,10 +95,10 @@ public class MessageLoggerEngine impleme
> > * @return the current logger, or the default one if there is no
> > logger in the stack
> > */
> > public MessageLogger peekLogger() {
> > - if (loggerStack.isEmpty()) {
> > + if (getLoggerStack().isEmpty()) {
> > return getDefaultLogger();
> > }
> > - return (MessageLogger) loggerStack.peek();
> > + return (MessageLogger) getLoggerStack().peek();
> > }
> >
> > private MessageLogger getDefaultLogger() {
> > @@ -130,7 +139,7 @@ public class MessageLoggerEngine impleme
> >
> > public void clearProblems() {
> > getDefaultLogger().clearProblems();
> > - for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
> > + for (Iterator iter = getLoggerStack().iterator();
> > iter.hasNext();) {
> > MessageLogger l = (MessageLogger) iter.next();
> > l.clearProblems();
> > }
> > @@ -142,7 +151,7 @@ public class MessageLoggerEngine impleme
> > public void setShowProgress(boolean progress) {
> > getDefaultLogger().setShowProgress(progress);
> > // updates all loggers in the stack
> > - for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
> > + for (Iterator iter = getLoggerStack().iterator();
> > iter.hasNext();) {
> > MessageLogger l = (MessageLogger) iter.next();
> > l.setShowProgress(progress);
> > }
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1553704 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
Posted by Charles Duffy <ch...@dyfis.net>.
The amount of "fun" otherwise involved when two separate threads called
into Ivy code in parallel was considerable, and was regularly resulting in
a logger stack with no relationship with any individual thread's call stack.
On Mar 25, 2014 7:27 AM, "Maarten Coene" <ma...@yahoo.com> wrote:
> Maybe a bit late, but what was the reason to use a ThreadLocal here?
> This gives issues when the configuration of Ivy (for instance setting a
> custom logger) is done from a different thread than the actual resolve.
>
> Maarten
>
>
>
> ________________________________
> Van: "cduffy@apache.org" <cd...@apache.org>
> Aan: notifications@ant.apache.org
> Verzonden: vrijdag 27 december 18:40 2013
> Onderwerp: svn commit: r1553704 -
> /ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
>
>
> Author: cduffy
> Date: Fri Dec 27 17:40:20 2013
> New Revision: 1553704
>
> URL: http://svn.apache.org/r1553704
> Log:
> Move non-root MessageLogger instances into a thread-local stack
>
> Logger context is inherently thread-local.
>
> Modified:
>
> ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
>
> Modified:
> ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> URL:
> http://svn.apache.org/viewvc/ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java?rev=1553704&r1=1553703&r2=1553704&view=diff
>
> ==============================================================================
> ---
> ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> (original)
> +++
> ant/ivy/core/trunk/src/java/org/apache/ivy/util/MessageLoggerEngine.java
> Fri Dec 27 17:40:20 2013
> @@ -34,7 +34,7 @@ import java.util.Stack;
> * </p>
> */
> public class MessageLoggerEngine implements MessageLogger {
> - private final Stack/*<MessageLogger>*/ loggerStack = new Stack();
> + private final ThreadLocal/*<Stack<MessageLogger>>*/ loggerStacks =
> new ThreadLocal();
>
> private MessageLogger defaultLogger = null;
>
> @@ -44,6 +44,15 @@ public class MessageLoggerEngine impleme
>
> private List errors = new ArrayList();
>
> + private Stack getLoggerStack() {
> + Stack stack = (Stack) loggerStacks.get();
> + if (stack == null) {
> + stack = new Stack();
> + loggerStacks.set(stack);
> + }
> + return stack;
> + }
> +
> public MessageLoggerEngine() {
> }
>
> @@ -66,7 +75,7 @@ public class MessageLoggerEngine impleme
> */
> public void pushLogger(MessageLogger logger) {
> Checks.checkNotNull(logger, "logger");
> - loggerStack.push(logger);
> + getLoggerStack().push(logger);
> }
>
> /**
> @@ -76,8 +85,8 @@ public class MessageLoggerEngine impleme
> * </p>
> */
> public void popLogger() {
> - if (!loggerStack.isEmpty()) {
> - loggerStack.pop();
> + if (!getLoggerStack().isEmpty()) {
> + getLoggerStack().pop();
> }
> }
>
> @@ -86,10 +95,10 @@ public class MessageLoggerEngine impleme
> * @return the current logger, or the default one if there is no
> logger in the stack
> */
> public MessageLogger peekLogger() {
> - if (loggerStack.isEmpty()) {
> + if (getLoggerStack().isEmpty()) {
> return getDefaultLogger();
> }
> - return (MessageLogger) loggerStack.peek();
> + return (MessageLogger) getLoggerStack().peek();
> }
>
> private MessageLogger getDefaultLogger() {
> @@ -130,7 +139,7 @@ public class MessageLoggerEngine impleme
>
> public void clearProblems() {
> getDefaultLogger().clearProblems();
> - for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
> + for (Iterator iter = getLoggerStack().iterator();
> iter.hasNext();) {
> MessageLogger l = (MessageLogger) iter.next();
> l.clearProblems();
> }
> @@ -142,7 +151,7 @@ public class MessageLoggerEngine impleme
> public void setShowProgress(boolean progress) {
> getDefaultLogger().setShowProgress(progress);
> // updates all loggers in the stack
> - for (Iterator iter = loggerStack.iterator(); iter.hasNext();) {
> + for (Iterator iter = getLoggerStack().iterator();
> iter.hasNext();) {
> MessageLogger l = (MessageLogger) iter.next();
> l.setShowProgress(progress);
> }