RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

[Roger Riggs]

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.
>>
>