Am 26.01.2015 um 09:28 schrieb Mark Thomas:
On 25/01/2015 22:57, Christopher Schultz wrote:
Felix,

On 1/24/15 2:33 PM, Felix Schumacher wrote:
Am 24.01.2015 um 17:13 schrieb Christopher Schultz:
Felix,

On 1/24/15 9:42 AM, fschumac...@apache.org wrote:
Author: fschumacher
Date: Sat Jan 24 14:42:27 2015
New Revision: 1654524

URL: http://svn.apache.org/r1654524
Log:
Close input and output streams in expandCGIScript to
avoid resource leaks. Issue reported by Coverity Scan.

Modified:
      tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java

Modified: tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java?rev=1654524&r1=1654523&r2=1654524&view=diff

==============================================================================

--- tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
Sat Jan 24 14:42:27 2015
@@ -1133,6 +1133,10 @@ public final class CGIServlet extends Ht
                 File f = new File(destPath.toString());
               if (f.exists()) {
+                try {
+                    is.close();
+                } catch (IOException ignore) {
+                }
Should this be logged? It should very rarely happen, but it would be
good to know if there was a problem (which might represent a resource
leak).
I looked for other examples in the source code before and the first few
examples I found where ignoring the exception while closing, too. So I
thought it would be ok, do ignore this exception.

If we don't want this exception ignored, at what level should the
information be logged? I would go for debug or info.
I think I'm in a minority when it comes to wanting to log these
exceptions. I think it might even be appropriate to log them at the WARN
level. If they happen -- which should be very rare -- it could indicate
a serious problem with the system or the JVM.
Ideally, the only time we would ignore exceptions is when we genuinely
don't care. That could include:
- something has already gone wrong (which we logged) and we are just
trying to clean up the mess, exceptions are expected and aren't going to
tell us anything useful
- we know that the exception is impossible but we have to catch it

I think the main thing is not to fill the logs with exception messages
the user can do nothing about.

In this case WARN seems reasonable.
I have added the logging of the exceptions.

Felix

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to