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

[Ivan Gerasimov]

Thanks Alan!

On 25.08.2014 15:25, Alan Bateman wrote:
> On 24/08/2014 21:54, Ivan Gerasimov wrote:
>> 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.
> The webrev looks good and I agree that we should deal with close 
> separately (as that will necessitate touching a number of areas). 
> Martin's suggestion to put add a comment to 
> UnixNativeDispatcher.fclose that it ignores EINTR and doesn't fflush 
> would be useful.
>
Here's the comment I've added to UnixNativeDispatcher.fclose:

    /* NOTE: fclose() wrapper is only used with read-only streams.
      * If it ever is used with write streams, it might be better to add
      * RESTARTABLE(fflush(fp)) before closing, to make sure the stream
      * is completely written even if fclose() failed.
      */

> I found this article useful and the issue of close vs. EINTR 
> discussion has been going for years:
>     http://lwn.net/Articles/576478/
>
One conclusion from this article is that it's not generally safe to 
retry close() upon a failure, as we can accidentally close some other 
file been just opened by another thread. So, shouldn't we at least 
remove RESTARTABLE macro around close(fd)?

Sincerely yours,
Ivan