commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <skitch...@apache.org>
Subject RE: Logging: SimpleLog not thread-safe
Date Thu, 12 Oct 2006 07:43:24 GMT
It might do; not sure what the performance of ThreadLocal is. However
the price is extra memory usage: a DateFormatter for every thread in the
system that ever logs a message.

On Wed, 2006-10-11 at 17:42 -0400, Kenneth Xu wrote:
> Hi,
> 
> I think a thread local formatter will give us better performance than
> creating one new in every log invocation.
> 
> Just my 2 cents,
> 
> Thanks,
> Ken
> 
> -----Original Message-----
> From: Simon Kitching [mailto:skitching@apache.org] 
> Sent: Wednesday, October 11, 2006 5:05 AM
> To: Jakarta Commons Developers List
> Subject: Re: Logging: SimpleLog not thread-safe
> 
> Hi Martin,
> 
> Thanks very much for letting us know about this. I think you're right
> about there being a thread-safety issue. 
> 
> SimpleLog is definitely in use, but obviously this bug will only be
> triggered under pretty rare conditions, and I expect would usually just
> result in a malformed date string which may not be noticed anyway.
> 
> But it should be fixed; I'll try to do that this weekend.
> 
> Regards,
> 
> Simon
> 
> On Fri, 2006-10-06 at 17:15 +0100, Martin Wilson wrote:
> > Hi,
> >  
> > I'm not sure if anyone else uses the SimpleLog class - anyway I've
> > noticed that SimpleLog.log is not thread-safe. The following code
> > (starting on line 282):
> >  
> >         if(showDateTime) {
> >             buf.append(dateFormatter.format(new Date()));
> >             buf.append(" ");
> >         }
> >  
> > makes an unsynchronized call to dateFormatter.format. As dateFormatter
> > is an instance variable, and DateFormat.format is not thread-safe, this
> > will cause problems if more than one thread tried to log at the same
> > time.
> >  
> > Solution: remove the dateFormatter instance variable and instantiate a
> > new DateFormat each time in the log method, e.g.
> >  
> > DateFormat dateFormatter = null;
> >             try {
> >                         dateFormatter = new
> > SimpleDateFormat(dateTimeFormat);
> >             }
> > catch(IllegalArgumentException e) {
> >                         dateFormatter = new
> > SimpleDateFormat(DEFAULT_DATE_TIME_FORMAT);
> >             }  
> >  
> > Is anyone available who could make this change?
> >  
> > Thanks, 
> > Martin
> 
> > ___________
> > 
> > Martin Wilson
> > martinw@bright-interactive.com
> > 
> > Bright Interactive: successfully delivering interactive websites and
> > business applications
> >  <http://www.bright-interactive.com/> http://www.bright-interactive.com
> > 0870 240 6520
> >  
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message