Hi Vyom,
I think it should be the case that the same exception (FrpProtocol,
IOException) should
be rethrown instead of the exception that resulted from the call to close.
In many case, the exceptions from a call to a cleanup close() are simply
ignored
but the alternative is to add them as suppressed exceptions to the
original fe or ex.
I would code the new blocks as ignoring the IOException on close and
falling through
to throw the same exception as previously.
For example,
} catch (FtpProtocolException fe) {
+ if (ftp != null) {
+ try {
+ ftp.close();
+ } catch (IOException ioe) {
+ // ignore; alternatively fe.addSuppressed(ioe);
+ }
+ }
throw new IOException(fe);
Roger
On 3/21/2016 5:19 AM, vyom wrote:
Hi,
Please find the updated webrev, got some off line comment from Chris.
http://cr.openjdk.java.net/~vtewari/7167293/webrev0.2/index.html
<http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.2/index.html>
Thanks,
Vyom
On Thursday 17 March 2016 12:30 PM, vyom wrote:
please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/7167293/webrev0.1/index.html
<http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.1/index.html>).
Thanks,
Vyom
On Tuesday 15 March 2016 05:11 PM, Chris Hegarty wrote:
Vyom,
On 15/03/16 10:00, vyom wrote:
Hi,
Please review the below fix.
Bug: JDK-7167293 : FtpURLConnection connection leak on
FileNotFoundException
Webrev: http://cr.openjdk.java.net/~rgoel/~vyom/7167293/webrev0.0/
<http://cr.openjdk.java.net/%7Ergoel/%7Evyom/7167293/webrev0.0/>
You have the same lines of code in a number of places. Does
a private static helper method make sense for this?
Test tries to connect to an external resource, which is not
reachable on my, and many, systems. Can the test setup a simple
ServerSocket to do something minimal?
-Chris.
