Re: [PATCH] Manual should discourage mixing TEMP_FAILURE_RETRY and close

[Zack Weinberg <zackw at panix dot com>]

On 12/06/2016 10:51 AM, Rich Felker wrote:
>> Also, I took a look at the illumos code base, which presumably does
>> not differ greatly from historical Solaris on this point. It looks to
>> me (closef(), closeandsetf()) that also on that system there are paths
>> in close() where errors are returned but the FD is nevertheless
>> closed. So it really does seem that POSIX.1 was specifying (and plans
>> to again specify) behavior that is inconsistent with actual
>> implementations.
> 
> I think you're assuming POSIX is imposing something opposite of what
> it's doing. The new text has filedes left open _only_ in the case of
> EINTR. For all other errors it's closed and available for reassignment
> on subsequent open, etc.

But that's still wrong!  `fildes` MUST be closed even when EINTR is
returned.  That's what all existing implementations do (if I understood
Michael correctly), and even if I've misunderstood that part, because
*some* implementations do it, portable code has to assume that it's
unsafe to retry the operation.  So the only sane specification for POSIX
to make is that the descriptor is closed.

(No, new programs cannot assume that it's safe to retry, because new
programs get run on old operating systems.)

(Yes, this means delayed failures can get lost.  Code that cares
should've called `fsync` anyway.  Also, the whole thing is moot if you
use SA_RESTART on all your signals, which is the Right Thing for tons of
other reasons.)

----

Regardless of what the specification does or doesn't say or will be
corrected to say, what the *glibc manual* should say is clear, IMNSHO:
don't retry close().  Proposed new text follows - it's not as emphatic
as what Michael wrote for the manpages but I think it's sufficient.

zw


@comment unistd.h
@comment POSIX.1
@deftypefun int close (int @var{filedes})
@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{@acsfd{}}}
The function @code{close} closes the file descriptor @var{filedes}.
Closing a file has the following consequences:

@itemize @bullet
@item
The file descriptor is deallocated.

@item
Any record locks owned by the process on the file are unlocked.

@item
When all file descriptors associated with a pipe or FIFO have been closed,
any unread data is discarded.
@end itemize

Closing a file does @emph{not} guarantee that all data has been
successfully written to disk.  If you need to make sure of this, you
must call @code{fsync} before @code{close} (@pxref{Synchronizing I/O
operations}).

@code{close} never @emph{fails}; after any call to @code{close}
returns, the file descriptor @var{filedes} has been closed and must
not be used again.  However, it may still report an error.  It returns
0 if there was no error, and @math{-1} if an error occurred.  The
following @code{errno} error conditions are defined for this function:

@table @code
@item ENOSPC
@itemx EIO
@itemx EDQUOT
These codes indicate an I/O error in an earlier call to @code{write}
that appeared to succeed.  @xref{I/O Primitives}, for details on their
meaning.  Because of these ``delayed failures,'' it is important to
check for errors in @code{close}.

@item EINTR
The @code{close} call was interrupted by a signal.  The only
consequence of this is that a ``delayed failure'' may have been
lost---the file descriptor has still been closed.  You should
@emph{not} retry the system call.

@item EBADF
The @var{filedes} argument was not an open file descriptor to begin
with (and still isn't).
@end table

This function is a cancellation point in multi-threaded programs.  This
is a problem if the thread allocates some resources (like memory, file
descriptors, semaphores or whatever) at the time @code{close} is
called.  If the thread gets canceled these resources stay allocated
until the program ends.  To avoid this, calls to @code{close} should be
protected using cancellation handlers.
@c ref pthread_cleanup_push / pthread_cleanup_pop

@code{close} can close any type of file descriptor, no matter how it
was opened.  Therefore, there is no @code{close64}, as it is
unnecessary.  However, @code{FILE} objects must be closed using
@code{fclose} instead (@pxref{Closing Streams}).
@end deftypefun