You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by gl...@apache.org on 2002/08/26 14:15:59 UTC
cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core StandardContext.java
glenn 2002/08/26 05:15:59
Modified: catalina/src/share/org/apache/catalina/connector
RequestBase.java
catalina/src/share/org/apache/catalina/core
StandardContext.java
Log:
Fix for BUG 11947. Change where startCapture/stopCapture is invoked to
prevent memory leaks from the SystemLogHandler.
Revision Changes Path
1.21 +4 -10 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/connector/RequestBase.java
Index: RequestBase.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/connector/RequestBase.java,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -r1.20 -r1.21
--- RequestBase.java 27 May 2002 13:34:34 -0000 1.20
+++ RequestBase.java 26 Aug 2002 12:15:58 -0000 1.21
@@ -91,7 +91,6 @@
import org.apache.catalina.util.Enumerator;
import org.apache.catalina.util.RequestUtil;
import org.apache.catalina.util.StringManager;
-import org.apache.tomcat.util.log.SystemLogHandler;
/**
@@ -340,7 +339,6 @@
this.context = context;
- SystemLogHandler.startCapture();
}
@@ -559,10 +557,6 @@
*/
public void recycle() {
- String log = SystemLogHandler.stopCapture();
- if (log != null && log.length() > 0) {
- context.getServletContext().log(log);
- }
attributes.clear();
authorization = null;
characterEncoding = null;
1.110 +15 -5 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core/StandardContext.java
Index: StandardContext.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core/StandardContext.java,v
retrieving revision 1.109
retrieving revision 1.110
diff -u -r1.109 -r1.110
--- StandardContext.java 28 Jun 2002 16:09:57 -0000 1.109
+++ StandardContext.java 26 Aug 2002 12:15:59 -0000 1.110
@@ -138,6 +138,7 @@
import org.apache.catalina.session.StandardManager;
import org.apache.catalina.util.CharsetMapper;
import org.apache.catalina.util.RequestUtil;
+import org.apache.tomcat.util.log.SystemLogHandler;
/**
@@ -2347,7 +2348,16 @@
}
// Normal request processing
- super.invoke(request, response);
+ try {
+ SystemLogHandler.startCapture();
+ super.invoke(request, response);
+ } finally {
+ String log = SystemLogHandler.stopCapture();
+ if (log != null && log.length() > 0) {
+ log(log);
+ }
+
+ }
}
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
Glenn Nielsen wrote:
> Remy Maucherat wrote:
>
>> Glenn Nielsen wrote:
>>
>>>> So I'll add support for log capture on shutdown (I think it's as
>>>> useful to have it as on startup). My personal preference is to leave
>>>> the capture switched off by default for the critical path, as it
>>>> would confuse developers (IMO).
>>>>
>>>
>>> Thanks
>>>
>>> The number of people using Tomcat for development is probably much
>>> larger than
>>> those using it in production. So I am removing my previous -1 for
>>> the default
>>> setting of swallowOutput.
>>
>>
>>
>> I was thinking maybe it could be useful to add that flag to the
>> DefaultContext. I'll do that after tagging 4.1.10.
>>
>
>
> I forgot about the DefaultContext, yes, swallowOutput must be configurable
> in the DefaultContext. It shouldn't take much to add this, why not add it
> before tagging 4.1.10 since it may be a good candidate for release.
> I know that I will require the ability to set swallowOutput=true in the
> DefaultContext.
Ok. Do you have time to add it ?
I don't have time to do it today, so I would only do it tomorrow before
tagging.
Thanks,
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Remy Maucherat wrote:
> Glenn Nielsen wrote:
>
>>> So I'll add support for log capture on shutdown (I think it's as
>>> useful to have it as on startup). My personal preference is to leave
>>> the capture switched off by default for the critical path, as it
>>> would confuse developers (IMO).
>>>
>>
>> Thanks
>>
>> The number of people using Tomcat for development is probably much
>> larger than
>> those using it in production. So I am removing my previous -1 for the
>> default
>> setting of swallowOutput.
>
>
> I was thinking maybe it could be useful to add that flag to the
> DefaultContext. I'll do that after tagging 4.1.10.
>
I forgot about the DefaultContext, yes, swallowOutput must be configurable
in the DefaultContext. It shouldn't take much to add this, why not add it
before tagging 4.1.10 since it may be a good candidate for release.
I know that I will require the ability to set swallowOutput=true in the
DefaultContext.
Regards,
Glenn
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
Glenn Nielsen wrote:
>> So I'll add support for log capture on shutdown (I think it's as
>> useful to have it as on startup). My personal preference is to leave
>> the capture switched off by default for the critical path, as it would
>> confuse developers (IMO).
>>
>
> Thanks
>
> The number of people using Tomcat for development is probably much
> larger than
> those using it in production. So I am removing my previous -1 for the
> default
> setting of swallowOutput.
I was thinking maybe it could be useful to add that flag to the
DefaultContext. I'll do that after tagging 4.1.10.
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Remy Maucherat wrote:
> Glenn Nielsen wrote:
>
>> Remy Maucherat wrote:
>>
>>> Glenn Nielsen wrote:
>>>
>>>> Capturing of stdout/stderr has been enabled in Tomcat 4.1 for 2-3
>>>> months now.
>>>
>>>
>>>
>>>
>>> I don't think it was, as RequestBase is deprecated, and is not used
>>> anymore (except in the old connectors).
>>>
>>>> What I committed was a bug fix. If you revert it, capturing
>>>> stderr/stdout code
>>>> will still exist, you would just put back in place a memory leak.
>>>
>>>
>>>
>>>
>>> Ok.
>>>
>>>> There is another place where you will need to implement your
>>>> SwallowOutput flag,
>>>> in StandardWrapper.java. It captures output from a load on startup
>>>> servlet.
>>>
>>>
>>>
>>>
>>> I don't have many problems with capturing during servlet init. It
>>> makes more sense to me to capture that data (no rational explanation,
>>> just a feeling).
>>> It's not done during shutdown, BTW.
>>>
>>
>> I can add that. :-)
>>
>>
>>> I can also make that optional using the same flag.
>>>
>>>> The thread sync issue is a good point, but this can be improved by
>>>> changing
>>>> the SystemLogHandler to use Thread Local variables.
>>>
>>>
>>>
>>>
>>> Thread local is quite slow, from what I saw with OptimizeIt, so I
>>> don't want to use it.
>>>
>>
>> From reading the section on Thread Local Performance here:
>>
>> http://www-106.ibm.com/developerworks/java/library/j-threads3.html#h15292
>>
>> Thread local performance differs based on the JVM.
>>
>> 1.2 -> poor
>> 1.3 -> better than normal sync _if_ you have alot of thread contention.
>> In production where performance is an issue Tomcat can spawn alot
>> of threads, so Thread Local could be of benefit.
>> 1.4 -> much faster than normal sync
>>
>> Based on this, for the long run, I think its better to switch to
>> Thread Local.
>
>
> Thanks for the information :)
>
>> In the long run it may be better to add support for CaptureLog to the
>> base class
>> for a Processor so that the thread sync issues can be avoided altogether.
>
>
> So I'll add support for log capture on shutdown (I think it's as useful
> to have it as on startup). My personal preference is to leave the
> capture switched off by default for the critical path, as it would
> confuse developers (IMO).
>
Thanks
The number of people using Tomcat for development is probably much larger than
those using it in production. So I am removing my previous -1 for the default
setting of swallowOutput.
Glenn
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
Glenn Nielsen wrote:
> Remy Maucherat wrote:
>
>> Glenn Nielsen wrote:
>>
>>> Capturing of stdout/stderr has been enabled in Tomcat 4.1 for 2-3
>>> months now.
>>
>>
>>
>> I don't think it was, as RequestBase is deprecated, and is not used
>> anymore (except in the old connectors).
>>
>>> What I committed was a bug fix. If you revert it, capturing
>>> stderr/stdout code
>>> will still exist, you would just put back in place a memory leak.
>>
>>
>>
>> Ok.
>>
>>> There is another place where you will need to implement your
>>> SwallowOutput flag,
>>> in StandardWrapper.java. It captures output from a load on startup
>>> servlet.
>>
>>
>>
>> I don't have many problems with capturing during servlet init. It
>> makes more sense to me to capture that data (no rational explanation,
>> just a feeling).
>> It's not done during shutdown, BTW.
>>
>
> I can add that. :-)
>
>
>> I can also make that optional using the same flag.
>>
>>> The thread sync issue is a good point, but this can be improved by
>>> changing
>>> the SystemLogHandler to use Thread Local variables.
>>
>>
>>
>> Thread local is quite slow, from what I saw with OptimizeIt, so I
>> don't want to use it.
>>
>
> From reading the section on Thread Local Performance here:
>
> http://www-106.ibm.com/developerworks/java/library/j-threads3.html#h15292
>
> Thread local performance differs based on the JVM.
>
> 1.2 -> poor
> 1.3 -> better than normal sync _if_ you have alot of thread contention.
> In production where performance is an issue Tomcat can spawn alot
> of threads, so Thread Local could be of benefit.
> 1.4 -> much faster than normal sync
>
> Based on this, for the long run, I think its better to switch to Thread
> Local.
Thanks for the information :)
> In the long run it may be better to add support for CaptureLog to the
> base class
> for a Processor so that the thread sync issues can be avoided altogether.
So I'll add support for log capture on shutdown (I think it's as useful
to have it as on startup). My personal preference is to leave the
capture switched off by default for the critical path, as it would
confuse developers (IMO).
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Remy Maucherat wrote:
> Glenn Nielsen wrote:
>
>> Capturing of stdout/stderr has been enabled in Tomcat 4.1 for 2-3
>> months now.
>
>
> I don't think it was, as RequestBase is deprecated, and is not used
> anymore (except in the old connectors).
>
>> What I committed was a bug fix. If you revert it, capturing
>> stderr/stdout code
>> will still exist, you would just put back in place a memory leak.
>
>
> Ok.
>
>> There is another place where you will need to implement your
>> SwallowOutput flag,
>> in StandardWrapper.java. It captures output from a load on startup
>> servlet.
>
>
> I don't have many problems with capturing during servlet init. It makes
> more sense to me to capture that data (no rational explanation, just a
> feeling).
> It's not done during shutdown, BTW.
>
I can add that. :-)
> I can also make that optional using the same flag.
>
>> The thread sync issue is a good point, but this can be improved by
>> changing
>> the SystemLogHandler to use Thread Local variables.
>
>
> Thread local is quite slow, from what I saw with OptimizeIt, so I don't
> want to use it.
>
From reading the section on Thread Local Performance here:
http://www-106.ibm.com/developerworks/java/library/j-threads3.html#h15292
Thread local performance differs based on the JVM.
1.2 -> poor
1.3 -> better than normal sync _if_ you have alot of thread contention.
In production where performance is an issue Tomcat can spawn alot
of threads, so Thread Local could be of benefit.
1.4 -> much faster than normal sync
Based on this, for the long run, I think its better to switch to Thread Local.
In the long run it may be better to add support for CaptureLog to the base class
for a Processor so that the thread sync issues can be avoided altogether.
Regards,
Glenn
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
Glenn Nielsen wrote:
> Capturing of stdout/stderr has been enabled in Tomcat 4.1 for 2-3 months
> now.
I don't think it was, as RequestBase is deprecated, and is not used
anymore (except in the old connectors).
> What I committed was a bug fix. If you revert it, capturing
> stderr/stdout code
> will still exist, you would just put back in place a memory leak.
Ok.
> There is another place where you will need to implement your
> SwallowOutput flag,
> in StandardWrapper.java. It captures output from a load on startup
> servlet.
I don't have many problems with capturing during servlet init. It makes
more sense to me to capture that data (no rational explanation, just a
feeling).
It's not done during shutdown, BTW.
I can also make that optional using the same flag.
> The thread sync issue is a good point, but this can be improved by changing
> the SystemLogHandler to use Thread Local variables.
Thread local is quite slow, from what I saw with OptimizeIt, so I don't
want to use it.
> Why don't we wait and see what other tomcat developers think of this.
>
> Releasing 4.1.10 can wait 24-36 hours.
It will have to as there's a big problem in Jasper.
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Remy Maucherat wrote:
> Glenn Nielsen wrote:
>
>> Remy Maucherat wrote:
>>
>>> glenn@apache.org wrote:
>>>
>>>> glenn 2002/08/26 05:15:59
>>>>
>>>> Modified: catalina/src/share/org/apache/catalina/connector
>>>> RequestBase.java
>>>> catalina/src/share/org/apache/catalina/core
>>>> StandardContext.java
>>>> Log:
>>>> Fix for BUG 11947. Change where startCapture/stopCapture is
>>>> invoked to
>>>> prevent memory leaks from the SystemLogHandler.
>>>
>>>
>>>
>>>
>>> I'd like log swallowing to be optional and disabled by default. It
>>> will confuse users too much otherwise (they're used to using Sys.out
>>> to debug).
>>> It is also not a good idea to pay the performance cost when the
>>> feature is not needed IMO.
>>>
>>> I plan to add a "swallowOutput" flag to StandardContext, with its
>>> default to false.
>>>
>>> Remy
>>>
>>
>> I do think the feature is needed, especially for production systems.
>>
>> Many API's used by a web application don't know they are being used in
>> a web
>> application and have no access to the servlet context log, they use
>> stderr
>> and stdout. It is very confusing during production if errors
>> generated by
>> these API's are put in catalina.out and other errors are in the
>> Context log.
>> Putting all the stdout/stderr output for a web app into one log makes
>> it easier
>> to debug a problem. Also, when the output goes to catalina.out there
>> are no
>> timestamps which makes it difficult to correlate output in
>> catalina.out with
>> the context log.
>>
>> For me, debugging is easier if everything related to a webapp is in
>> the same log,
>> and with timestamps.
>>
>> This is just a change in behaviour, I don't see a problem with having
>> it enabled
>> by default as long as it is documented.
>>
>> -0 for adding the flag
>> -1 for setting default behaviour to disable putting stderr/stdout in
>> the context log.
>
>
> Most Tomcat users are developers, and do not care about your
> "production" ready feature. IMO system administrators will have to tweak
> their Tomcat installation, no matter what we do. OTOH, casual developers
> are going to be deeply confused and annoyed by the feature.
>
> I'm not happy with the patch you committed yesterday, and which forced
> redirection (at the cost of adding two synced hashtable operations in
> the critical path).
>
> So -1 for the patch you committed yesterday.
>
> (I think the code I just committed is an acceptable compromise, so I'd
> appreciate it if you retracted your veto; if you don't, we'll have to
> revert the last 2 StandardContext commits)
>
> Remy
>
>
Capturing of stdout/stderr has been enabled in Tomcat 4.1 for 2-3 months now.
What I committed was a bug fix. If you revert it, capturing stderr/stdout code
will still exist, you would just put back in place a memory leak.
There is another place where you will need to implement your SwallowOutput flag,
in StandardWrapper.java. It captures output from a load on startup servlet.
The thread sync issue is a good point, but this can be improved by changing
the SystemLogHandler to use Thread Local variables.
Why don't we wait and see what other tomcat developers think of this.
Releasing 4.1.10 can wait 24-36 hours.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
Glenn Nielsen wrote:
> Remy Maucherat wrote:
>
>> glenn@apache.org wrote:
>>
>>> glenn 2002/08/26 05:15:59
>>>
>>> Modified: catalina/src/share/org/apache/catalina/connector
>>> RequestBase.java
>>> catalina/src/share/org/apache/catalina/core
>>> StandardContext.java
>>> Log:
>>> Fix for BUG 11947. Change where startCapture/stopCapture is
>>> invoked to
>>> prevent memory leaks from the SystemLogHandler.
>>
>>
>>
>> I'd like log swallowing to be optional and disabled by default. It
>> will confuse users too much otherwise (they're used to using Sys.out
>> to debug).
>> It is also not a good idea to pay the performance cost when the
>> feature is not needed IMO.
>>
>> I plan to add a "swallowOutput" flag to StandardContext, with its
>> default to false.
>>
>> Remy
>>
>
> I do think the feature is needed, especially for production systems.
>
> Many API's used by a web application don't know they are being used in a
> web
> application and have no access to the servlet context log, they use stderr
> and stdout. It is very confusing during production if errors generated by
> these API's are put in catalina.out and other errors are in the Context
> log.
> Putting all the stdout/stderr output for a web app into one log makes it
> easier
> to debug a problem. Also, when the output goes to catalina.out there
> are no
> timestamps which makes it difficult to correlate output in catalina.out
> with
> the context log.
>
> For me, debugging is easier if everything related to a webapp is in the
> same log,
> and with timestamps.
>
> This is just a change in behaviour, I don't see a problem with having it
> enabled
> by default as long as it is documented.
>
> -0 for adding the flag
> -1 for setting default behaviour to disable putting stderr/stdout in the
> context log.
Most Tomcat users are developers, and do not care about your
"production" ready feature. IMO system administrators will have to tweak
their Tomcat installation, no matter what we do. OTOH, casual developers
are going to be deeply confused and annoyed by the feature.
I'm not happy with the patch you committed yesterday, and which forced
redirection (at the cost of adding two synced hashtable operations in
the critical path).
So -1 for the patch you committed yesterday.
(I think the code I just committed is an acceptable compromise, so I'd
appreciate it if you retracted your veto; if you don't, we'll have to
revert the last 2 StandardContext commits)
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Glenn Nielsen <gl...@mail.more.net>.
Remy Maucherat wrote:
> glenn@apache.org wrote:
>
>> glenn 2002/08/26 05:15:59
>>
>> Modified: catalina/src/share/org/apache/catalina/connector
>> RequestBase.java
>> catalina/src/share/org/apache/catalina/core
>> StandardContext.java
>> Log:
>> Fix for BUG 11947. Change where startCapture/stopCapture is invoked to
>> prevent memory leaks from the SystemLogHandler.
>
>
> I'd like log swallowing to be optional and disabled by default. It will
> confuse users too much otherwise (they're used to using Sys.out to debug).
> It is also not a good idea to pay the performance cost when the feature
> is not needed IMO.
>
> I plan to add a "swallowOutput" flag to StandardContext, with its
> default to false.
>
> Remy
>
I do think the feature is needed, especially for production systems.
Many API's used by a web application don't know they are being used in a web
application and have no access to the servlet context log, they use stderr
and stdout. It is very confusing during production if errors generated by
these API's are put in catalina.out and other errors are in the Context log.
Putting all the stdout/stderr output for a web app into one log makes it easier
to debug a problem. Also, when the output goes to catalina.out there are no
timestamps which makes it difficult to correlate output in catalina.out with
the context log.
For me, debugging is easier if everything related to a webapp is in the same log,
and with timestamps.
This is just a change in behaviour, I don't see a problem with having it enabled
by default as long as it is documented.
-0 for adding the flag
-1 for setting default behaviour to disable putting stderr/stdout in the context log.
Regards,
Glenn
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/core
StandardContext.java
Posted by Remy Maucherat <re...@apache.org>.
glenn@apache.org wrote:
> glenn 2002/08/26 05:15:59
>
> Modified: catalina/src/share/org/apache/catalina/connector
> RequestBase.java
> catalina/src/share/org/apache/catalina/core
> StandardContext.java
> Log:
> Fix for BUG 11947. Change where startCapture/stopCapture is invoked to
> prevent memory leaks from the SystemLogHandler.
I'd like log swallowing to be optional and disabled by default. It will
confuse users too much otherwise (they're used to using Sys.out to debug).
It is also not a good idea to pay the performance cost when the feature
is not needed IMO.
I plan to add a "swallowOutput" flag to StandardContext, with its
default to false.
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>