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

[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 errno set to EINTR and the state of fildes is 
unspecified"

if dup2(s, fd) is viewed as a combination of close(fd) and fcntl (s, 
F_DUP2FD, fd), and close is not restartable
then similar semantics could be inferred for dup2 in a EINTR scenario?
presume that subsequent call in the RESTARTABLE loop  will return 
another error.




On 08/09/2015 09:28, Chris Hegarty wrote:
> On 7 Sep 2015, at 17:41, Mark Sheppard <mark.sheppard at oracle.com> 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 fd in the event of a dup2 failure?
> I’m not sure that the documentation for dup2 gives us enough detail here. The only situation I can see where fd would not be closed is when EBADF is returned. Should not be an issue here.
>
>> s is closed in any case, but what about fd, should it be attended to if dup2 < 0
>> and closed ? is it still considered a valid fd?
>>
>> what can be said about the state of the encapsulating FileDescriptor associated with fd?
>> at this stage can it still be considered valid?
>> should valid() return false?
> Probably, but may not be worth bothering with unless there are operations that can access it after this method throws.
>
> -Chris.
>
>> regards
>> Mark
>>
>> On 07/09/2015 14:29, Ivan Gerasimov wrote:
>>> 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 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 close() fails we throw correct io exception and there is no file leak.
>>>>> close isn't restartable so I think we need to look at that while we are here.
>>>>>
>>>>> -Alan.
>>>>