2015-07-09 12:06 GMT+03:00 <violet...@apache.org>:
> Author: violetagg
> Date: Thu Jul 9 09:06:25 2015
> New Revision: 1690035
>
> URL: http://svn.apache.org/r1690035
> Log:
> Merged revisions 1690011, 1690021 from tomcat/trunk:
> Fix possible resource leaks by closing streams properly. Issues reported by
> Coverity Scan.
>
> Modified:
> tomcat/tc7.0.x/trunk/ (props changed)
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/DeployTask.java
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/ValidatorTask.java
>
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
>
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties
> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/DeployTask.java
> URL:
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/DeployTask.java?rev=1690035&r1=1690034&r2=1690035&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/DeployTask.java
> (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/ant/DeployTask.java Thu Jul
> 9 09:06:25 2015
> @@ -199,6 +199,13 @@ public class DeployTask extends Abstract
> sb.append(URLEncoder.encode(tag, getCharset()));
> }
> } catch (UnsupportedEncodingException e) {
> + if (stream != null) {
> + try {
> + stream.close();
> + } catch (IOException ioe) {
> + // Ignore
> + }
> + }
> throw new BuildException("Invalid 'charset' attribute: " +
> getCharset());
> }
The above is OK (it fixes an obvious error of non closing the stream
when BuildException is thrown), but I think that it would be slightly
better to close the stream in a finally{}. I mean:
a) move "execute(sb.toString(), stream, contentType, contentLength);"
line that follows the above lines into main try/catch block
b) close the stream in a finally{}
The good side that it will take care if there is an unexpected RuntimeException.
It will change operation slightly that under normal conditions it will
close the stream twice (once in execute() and second in finally()),
but closing the same stream twice is allowed in Java.
http://docs.oracle.com/javase/6/docs/api/java/io/Closeable.html#close%28%29
"If the stream is already closed then invoking this method has no effect."
The rest of this commit is OK, no comments.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
