On Tuesday 29 March 2016 09:10 PM, Chris Hegarty wrote:
On 29 Mar 2016, at 16:20, Roger Riggs <roger.ri...@oracle.com> wrote:
Looks good,
+1
Does the test need to be run in othervm mode ?
I don't think othervm mode required, do you wants to me to generate
another webrev(0.5) ?
-Chris.
Thanks, Roger
On 3/29/2016 11:00 AM, vyom wrote:
Hi,
Please find the updated webrev.
http://cr.openjdk.java.net/~vtewari/7167293/webrev0.4/index.html
<http://cr.openjdk.java.net/%7Evtewari/7167293/webrev0.4/index.html>
Thanks,
Vyom
On Tuesday 29 March 2016 07:49 PM, Roger Riggs wrote:
Hi Vyom,
A minor cleanup to reduce the size of the little used code. Create the FNFE
before checking and closing.
Then the form of the cleanup code will be consistent and there will be less
code.
For example,
+ FileNotFoundException fnfe = new
FileNotFoundException(fullpath);
+ if (ftp != null) {
+ try {
+ ftp.close();
+ } catch (IOException ioe) {
+ fnfe.addSuppressed(ioe);
+ }
+ }
+ throw fnfe;
Thanks, Roger
On 3/29/2016 9:03 AM, vyom wrote:
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.
