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. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
