svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-24 Thread markt
Author: markt Date: Wed Jun 24 23:53:11 2009 New Revision: 788214 URL: http://svn.apache.org/viewvc?rev=788214&view=rev Log: Add thread safety patch. I haven't voted for it as I need to review it thoroughly first and it is getting late here. I'll review and vote tomorrow. Modified: tomcat/c

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-26 Thread Konstantin Kolinko
2009/6/25 : > Author: markt > Date: Wed Jun 24 23:53:11 2009 > New Revision: 788214 > > URL: http://svn.apache.org/viewvc?rev=788214&view=rev > Log: > Add thread safety patch. I haven't voted for it as I need to review it > thoroughly first and it is getting late here. I'll review and vote tomorr

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-26 Thread Mark Thomas
Konstantin Kolinko wrote: > 1. There is a bug, that sneaked in rev.781753 > http://svn.apache.org/viewvc?rev=781753&view=rev > That is: > the timeZoneNoDST field is never assigned and remains null > It was ok in 5.5.27. That is an error in my backport. That line should never have been deleted. I'l

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-27 Thread Konstantin Kolinko
Mark Thomas wrote: >Konstantin Kolinko wrote: >> 2. struct.currentDateString is calculated in two different places >> I propose to encapsulate that code into AccessDateStruct >> >> 3. Invocation of getDate() in invoke() (the only place where getDate() >> is called): >> I think that getDate() also c

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-27 Thread Xie Xiaodong
But how could ThreadLocal be shared among all AccessLogValve instances on the same server? After all, each thread access ThreadLocal has its own. Another point, based on the java doc of ThreadLocal, maybe "private ThreadLocal currentDateStruct" should be declared as static? 2009/6/27 Konstantin

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-27 Thread sebb
On 27/06/2009, Xie Xiaodong wrote: > But how could ThreadLocal be shared among all AccessLogValve instances on > the same server? After all, each thread access ThreadLocal has its own. > > Another point, based on the java doc of ThreadLocal, maybe "private > ThreadLocal currentDateStruct" shoul

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-27 Thread Xie Xiaodong
Yes, you're right. Both static and final. :) 2009/6/27 sebb > On 27/06/2009, Xie Xiaodong wrote: > > But how could ThreadLocal be shared among all AccessLogValve instances on > > the same server? After all, each thread access ThreadLocal has its own. > > > > Another point, based on the java d

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-28 Thread Konstantin Kolinko
Regarding FastCommonAccessLogValve and ExtendedAccessLogValve: FastCommonAccessLogValve: >> There is a bug, that sneaked in rev.781753 >> http://svn.apache.org/viewvc?rev=781753&view=rev >> That is: >> the timeZoneNoDST field is never assigned and remains null >> It was ok in 5.5.27. > > That is

Re: svn commit: r788214 - /tomcat/current/tc5.5.x/STATUS.txt

2009-06-30 Thread Mark Thomas
Konstantin Kolinko wrote: > Regarding FastCommonAccessLogValve and ExtendedAccessLogValve: > > FastCommonAccessLogValve: > >>> There is a bug, that sneaked in rev.781753 >>> http://svn.apache.org/viewvc?rev=781753&view=rev >>> That is: >>> the timeZoneNoDST field is never assigned and remains nul