RFR 7167293:FtpURLConnection connection leak on FileNotFoundException

vyom vyom.tewari at oracle.com
Tue Mar 29 13:03:39 UTC 2016


please find that updated 
webrev(http://cr.openjdk.java.net/~vtewari/7167293/webrev0.3 
<http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.3>), I 
incorporated Roger's comments.
Thanks,
Vyom


On Wednesday 23 March 2016 10:04 PM, Roger Riggs wrote:
> 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.
>>>
>>
>



More information about the net-dev mailing list