You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/09/29 18:30:35 UTC
svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: markt
Date: Mon Sep 29 09:30:34 2008
New Revision: 700170
URL: http://svn.apache.org/viewvc?rev=700170&view=rev
Log:
Propose mew fix for date format thread safety
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=700170&r1=700169&r2=700170&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Mon Sep 29 09:30:34 2008
@@ -236,3 +236,10 @@
Note: Deleted getETag(boolean) method to remain but deprecated
+1: markt, remm (I was -1 originally, but it may not be so bad)
-1:
+
+* Fix some thread safety issues.
+ Deprecate (rather than delete) any deleted code that isn't already deprecated
+ http://svn.apache.org/viewvc?rev=699714&view=rev (previous patch)
+ http://svn.apache.org/viewvc?rev=700167&view=rev (additional changes)
+ +1: mark
+ -1:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
ExtensionValidator problem
Posted by Christian Pich <cm...@cs.uoregon.edu>.
Hi,
I found a bizarre behavior in the startup, i.e. in the
ExtensionValidator.validateManifestResources() method.
when an extension library is not found then this method logs a message
on 'info' level and returns a 'false'.
The return value is the right thing to do but since this validation
failure leads to a failure in starting up the context
the logging level should be 'error'. I encountered the problem and the
only message I got is an inconspicuous
'Error getConfigured' in the StandardContext. People typically do not
put the Tomcat classes into 'info' mode and they shouldn't.
Shall I file a bug?
Also, I'd be happy to fix this. How do I become a committer?
Christian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by sebb <se...@gmail.com>.
On 29/09/2008, Mark Thomas <ma...@apache.org> wrote:
> sebb wrote:
> > On 29/09/2008, Mark Thomas <ma...@apache.org> wrote:
>
> >> ThreadLocals and container classloader environments need careful handling
> >> to avoid memory leaks.
> >
> > I would have thought that was a good reason to keep the class - one
> > only needs to get the code right once?
>
> If you don't use ThreadLocal, the code is much simpler so there is little
> to be gained by using a utility class. In this case of the two formats that
> were still used, one class was using each. It makes more sense to me to put
> the code specific to a class in that class and do away with what was a
> broken, deprecated and little used utility class.
>
>
> >> In this case the pain to make sure there wasn't a memory leak wasn't worth the gain.
> >
> > OK, point taken w.r.t. using synchronisation - the code should be
> > reasonably quick.
> >
> > But why not use an instance variable rather than a static class variable?
> > If there is more than 1 instance, each instance will have its own lock.
> > Which may share the load better.
>
> Creating the object is likely to be more expensive than using it. True I
> haven't tested it and if you have performance figures that suggest
> otherwise then it may be worth changing.
I don't know how the classes are used in Tomcat, but if each instance
is used many times, then the object creation cost needs to be compared
against the cost of multiple synchronisations.
If each instance is used only once - or only by a single thread - then
the synchronisation can be eliminated entirely.
But I have not done any tests to show which approach performs better.
>
> Mark
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by Mark Thomas <ma...@apache.org>.
sebb wrote:
> On 29/09/2008, Mark Thomas <ma...@apache.org> wrote:
>> ThreadLocals and container classloader environments need careful handling
>> to avoid memory leaks.
>
> I would have thought that was a good reason to keep the class - one
> only needs to get the code right once?
If you don't use ThreadLocal, the code is much simpler so there is little
to be gained by using a utility class. In this case of the two formats that
were still used, one class was using each. It makes more sense to me to put
the code specific to a class in that class and do away with what was a
broken, deprecated and little used utility class.
>> In this case the pain to make sure there wasn't a memory leak wasn't worth the gain.
>
> OK, point taken w.r.t. using synchronisation - the code should be
> reasonably quick.
>
> But why not use an instance variable rather than a static class variable?
> If there is more than 1 instance, each instance will have its own lock.
> Which may share the load better.
Creating the object is likely to be more expensive than using it. True I
haven't tested it and if you have performance figures that suggest
otherwise then it may be worth changing.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by sebb <se...@gmail.com>.
On 29/09/2008, Mark Thomas <ma...@apache.org> wrote:
> sebb wrote:
> > BTW, Commons LANG has a thread-safe FastDateUtils which can be used
> > for formatting (but not parsing) dates.
>
> A whole jar for one method is somewhat overkill.
Indeed, but there's lots of other useful stuff in LANG.
I just mentioned it in case of interest.
> > Not sure why you did not just fix DateTool - e.g. by using ThreadLocal
> > (or indeed synchronizing the use of DateFormats) - rather than fixing
> > all calls to DateTool.
> > Or maybe I've misunderstood something here.
>
> The class served no purpose. There aren't lots of places using this code.
>
> ThreadLocals and container classloader environments need careful handling
> to avoid memory leaks.
I would have thought that was a good reason to keep the class - one
only needs to get the code right once?
> In this case the pain to make sure there wasn't a memory leak wasn't worth the gain.
OK, point taken w.r.t. using synchronisation - the code should be
reasonably quick.
But why not use an instance variable rather than a static class variable?
If there is more than 1 instance, each instance will have its own lock.
Which may share the load better.
> Mark
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by Mark Thomas <ma...@apache.org>.
sebb wrote:
> BTW, Commons LANG has a thread-safe FastDateUtils which can be used
> for formatting (but not parsing) dates.
A whole jar for one method is somewhat overkill.
> Not sure why you did not just fix DateTool - e.g. by using ThreadLocal
> (or indeed synchronizing the use of DateFormats) - rather than fixing
> all calls to DateTool.
> Or maybe I've misunderstood something here.
The class served no purpose. There aren't lots of places using this code.
ThreadLocals and container classloader environments need careful handling
to avoid memory leaks. In this case the pain to make sure there wasn't a
memory leak wasn't worth the gain.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r700170 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by sebb <se...@gmail.com>.
BTW, Commons LANG has a thread-safe FastDateUtils which can be used
for formatting (but not parsing) dates.
Not sure why you did not just fix DateTool - e.g. by using ThreadLocal
(or indeed synchronizing the use of DateFormats) - rather than fixing
all calls to DateTool.
Or maybe I've misunderstood something here.
It would be useful to add a comment to the declaration of
private static final DateFormat format
to say that all access to the format must be synchronized.
On 29/09/2008, markt@apache.org <ma...@apache.org> wrote:
> Author: markt
> Date: Mon Sep 29 09:30:34 2008
> New Revision: 700170
>
> URL: http://svn.apache.org/viewvc?rev=700170&view=rev
> Log:
> Propose mew fix for date format thread safety
>
> Modified:
> tomcat/tc6.0.x/trunk/STATUS.txt
>
> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=700170&r1=700169&r2=700170&view=diff
> ==============================================================================
> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
> +++ tomcat/tc6.0.x/trunk/STATUS.txt Mon Sep 29 09:30:34 2008
> @@ -236,3 +236,10 @@
> Note: Deleted getETag(boolean) method to remain but deprecated
> +1: markt, remm (I was -1 originally, but it may not be so bad)
> -1:
> +
> +* Fix some thread safety issues.
> + Deprecate (rather than delete) any deleted code that isn't already deprecated
> + http://svn.apache.org/viewvc?rev=699714&view=rev (previous patch)
> + http://svn.apache.org/viewvc?rev=700167&view=rev (additional changes)
> + +1: mark
> + -1:
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org