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

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