RFR [8055421]: (fs) bad error handling in java.base/unix/native/libnio/fs/UnixNativeDispatcher.c

[Ivan Gerasimov]

Thank you Alan for the input!

I've updated the webrev as you suggested:
http://cr.openjdk.java.net/~igerasim/8055421/2/webrev/

With the wrapper for close we currently ignore any error that might have 
happened.
Don't we want to revise it with this change?

For example, note the caution on the Linux man page:
Not  checking  the  return  value of close() is a common but 
nevertheless serious programming error.  It is quite possible that 
errors on a previous write(2) operation are first reported at the final 
close().  Not checking the return value when closing the file may lead 
to silent loss of data.


Sincerely yours,
Ivan

On 24.08.2014 17:13, Alan Bateman wrote:
> On 22/08/2014 21:59, Ivan Gerasimov wrote:
>> How about another approach?
>> If we face EINTR, we can try to fallback to restartable close():
> I was away for a few days and just catching up on this thread now.
>
> I agree with Florian's original comment as there is something fishy 
> about the original bug report and it would be useful to know if there 
> is really a case where we are calling this native method with errno 
> set. In any case, the long standing rule with close is that you never 
> retry it after EINTR because the behavior is undefined. It's an 
> oversight in the original implementation to have used a restart loop 
> in fclose and to have used the RESTARTABLE macro in closedir. 
> Apologies about those. The proposed change to dup looks okay, that 
> would be a problem if dup were called when there aren't any file 
> descriptors available.
>
> On fclose then one thing to note is that it is only used on Solaris 
> when reading /etc/mnttab, I don't think it is used elsewhere. So write 
> buffering isn't an issue with the current usage. If there was writing 
> then Martin's suggestion to use a restartable fflush would be good to 
> do. It also means that having fclose ignore EINTR as per your 1/webrev 
> is okay too. For completeness then we could add a new method fflush 
> method that might get used someday.
>
> For closedir then dropping the use of RESTARTABLE and ignoring EINTR 
> should be okay.
>
> -Alan.
>
>
>
>