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

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