RFR : (fs) bad error handling in java.base/unix/native/libnio/fs/UnixNativeDispatcher.c
martinrb at google.com
Mon Aug 25 05:30:57 UTC 2014
On Sun, Aug 24, 2014 at 1:54 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com>
> Thank you Alan for the input!
> I've updated the webrev as you suggested:
Ivan, this change looks good!
(if UnixNativeDispatcher.fclose is only designed for reading, not writing,
there should be at the very least a comment. It's natural for future
readers to assume this is a general wrapper for fclose of either kind of
With the wrapper for close we currently ignore any error that might have
> Don't we want to revise it with this change?
I agree that we should add error checking for close as you suggest, but
let's do that in another 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,
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the nio-dev