Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-09 Thread Vyom Tewari
Hi Mark, Is it OK to go ahead with the patch as it is, or you are expecting any additional modification is required ?. Thanks, Vyom On 9/8/2015 6:35 PM, Mark Sheppard wrote: that's true, the documentation is a bit nebulous on this issue. but the inference is that the file descriptors are

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-09 Thread Mark Sheppard
Hi Vyom, yes, I believe the consensus is to proceed with the changes. regards Mark On 09/09/2015 16:23, Vyom Tewari wrote: Hi Mark, Is it OK to go ahead with the patch as it is, or you are expecting any additional modification is required ?. Thanks, Vyom On 9/8/2015 6:35 PM, Mark

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-09 Thread Daniel Fuchs
On 09/09/15 17:54, Mark Sheppard wrote: Hi Vyom, yes, I believe the consensus is to proceed with the changes. OK - I will be pushing them then. best regards, -- daniel regards Mark On 09/09/2015 16:23, Vyom Tewari wrote: Hi Mark, Is it OK to go ahead with the patch as it is, or

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-08 Thread Chris Hegarty
On 7 Sep 2015, at 17:41, Mark Sheppard wrote: > a couple of other considerations in the context of this issue perhaps? > > in this s is being duped onto fd, and part of the dup2 operation is the > closing of fd, but > what's is the expected state of file descriptor

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-08 Thread Mark Sheppard
that's true, the documentation is a bit nebulous on this issue. but the inference is that the file descriptors are indeterminate state. some older man pages allude to this as per solaris man pages, close will " If close() is interrupted by a signal that is to be caught, it will return -1 with

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Ivan Gerasimov
Hi! The close() function isn't really restartable. So, I think, it's more correct to replace RESTARTABLE(close(s), res); with res = close(s); Sincerely yours, Ivan On 07.09.2015 12:26, Vyom Tewari wrote: Hi everyone, Can you please review my changes for below bug. Bug:

RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Vyom Tewari
Hi everyone, Can you please review my changes for below bug. Bug: JDK-8080402 : File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.01/ This change ensure that if close() fails we throw correct io

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Chris Hegarty
Hi Vyom, On 07/09/15 10:26, Vyom Tewari wrote: Hi everyone, Can you please review my changes for below bug. Bug: JDK-8080402 : File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.01/ This change ensure

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Alan Bateman
On 07/09/2015 10:26, Vyom Tewari wrote: Hi everyone, Can you please review my changes for below bug. Bug: JDK-8080402 : File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java Webrev: http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.01/ This change ensure that if

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Ivan Gerasimov
Thanks! It looks good to me now. Sincerely yours, Ivan On 07.09.2015 16:08, Vyom Tewari wrote: Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ Thanks, Vyom On 9/7/2015 3:48 PM, Alan Bateman wrote: On

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Vyom Tewari
Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ Thanks, Vyom On 9/7/2015 3:48 PM, Alan Bateman wrote: On 07/09/2015 10:26, Vyom Tewari wrote: Hi everyone, Can you please review my changes for below bug. Bug:

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Alan Bateman
On 07/09/2015 14:08, Vyom Tewari wrote: Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ I think this looks okay now. -Alan

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Daniel Fuchs
Oh - sorry - I hadn't seen Mark question. I will wait until those are resolved... On 07/09/15 19:24, Daniel Fuchs wrote: Hi Vyom, I will sponsor that and push it for you. best regards, -- daniel On 07/09/15 18:52, Alan Bateman wrote: On 07/09/2015 14:08, Vyom Tewari wrote: Hi All,

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Mark Sheppard
a couple of other considerations in the context of this issue perhaps? in this s is being duped onto fd, and part of the dup2 operation is the closing of fd, but what's is the expected state of file descriptor fd in the event of a dup2 failure? s is closed in any case, but what about fd,

Re: RFR 8080402: File Leak in jdk/src/java.base/share/classes/sun/net/sdp/SdpSupport.java

2015-09-07 Thread Daniel Fuchs
Hi Vyom, I will sponsor that and push it for you. best regards, -- daniel On 07/09/15 18:52, Alan Bateman wrote: On 07/09/2015 14:08, Vyom Tewari wrote: Hi All, Please find the latest diff, which contains the latest fix. http://cr.openjdk.java.net/~dfuchs/vyom/8080402/webrev.02/ I