[PATCH] Manual should discourage mixing TEMP_FAILURE_RETRY and close

[Rich Felker]

On Tue, Dec 06, 2016 at 10:57:16AM +0100, Michael Kerrisk (man-pages) wrote:
> Hello Rich,
> 
> On 5 December 2016 at 17:50, Rich Felker <dalias@aerifal.cx> wrote:
> > On Mon, Dec 05, 2016 at 05:40:48PM +0100, Michael Kerrisk (man-pages) wrote:
> >> Hello Rich
> >>
> >> On 5 December 2016 at 17:34, Rich Felker <dalias@aerifal.cx> wrote:
> >> > On Mon, Dec 05, 2016 at 05:27:23PM +0100, Michael Kerrisk wrote:
> >> >> On Mon, Dec 5, 2016 at 5:22 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> >> > On Mon, Dec 05, 2016 at 05:18:28PM +0100, Michael Kerrisk wrote:
> 
> [...]
> 
> >> >> So, the summary looks like this, AFAICT. Current POSIX permits the
> >> >> Linux behavior. Future POSIX proposes to make a change that
> >> >> contradicts the behavior of at least three existing implementations
> >> >> (Linux, FreeBSD, and AIX). That doesn't seem right. What have I
> >> >> missed?
> >> >
> >> > The fact that these existing implementations were inconsistent with
> >> > what EINTR means everywhere else ("no significant side effect has
> >> > occurred yet from the call; if there are significant side effects,
> >> > e.g. partial write, semaphore decremented, etc. the call returns the
> >> > proper success value") and incompatible with the requirement that the
> >> > side effects of a cancellation point on thread cancellation are
> >> > required to be the same (almost-nothing) as the side effects on EINTR.
> >> > In order to have adopted the Linux kernel behavior of close, POSIX
> >> > would have had to special-case cancellation behavior of close() in a
> >> > way that differs from every other cancellation point, and would have
> >> > had to contradict the historical behavior of other implementations
> >> > that actually got this (close/EINTR) right.
> >>
> >> I didn't miss that ;-). I understand that in most other places, EINTR
> >> means something else. But already, POSIX is special casing close().
> >> This is not "Linux kernel behavior"; this is "Linux kernel and free
> >> BSD kernel and AIX kernel behavior. Surely, POSIX.1 won't just by fiat
> >> say that the existing implementations are broken?
> >
> > Do you have another proposed way out?
> 
> Perhaps: leave the current close() specification as is, and mandate
> only the new posix_close() with the desired behavior?

I don't think leaving a serious bug in an interface that everyone uses
and forcing programs to abandon the interface and use a new
barely-known one in its place is a viable solution.

> > It was a bug that they ever
> > allowed ambiguity;
> 
> Well, inasmuch as POSIX.1 generally standardizes existing behavior,
> the initial bug was the failure to account for existing implementation
> behavior of close()...

There are many precedents for POSIX imposing new requirements where
traditional/existing behavior had critical bugs. I can dig some up if
you want. I see this as a very good thing.

> > before POSIX had threads it didn't matter (well,
> > not too much; it could still be an issue with signal handlers!), but
> > after threads were added, it became unsafe to have any ambiguity
> > whether a file descriptor remains open, because closing it again
> > results in an extremely-dangerous double-close race -- these can lead
> > to file corruption, file contents disclosure, and even arbitrary code
> > execution. The only way to avoid that would be to live with possible
> > fd leaks, which result in eventual fd exhaustion.
> 
> Well, the other way to do it would be to do what Linux and FreeBSD do:
> always close the FD, even when returning an error. But, I take it you
> mean what the standard should provide as a guarantee to portable
> applications. Ambiguity there is obviously not helpful.

I was speaking from a portable application perspective, not from an
implementation perspective. With the ambiguous spec, the application
must accept the leak, since the double-close would be much worse.

> > Fortunately nobody is asking Linux, FreeBSD, and AIX to change a
> > feature that people actually want, or in an incompatible way. EINTR in
> > close is very rare (only possible with tape drives, other weird
> > devices, and perhaps some NFS configurations), and when it does
> > happen, it's unwanted anyway. Changing in an incompatible way --
> > returning EINTR with the fd still open -- would be one option, but
> > it's not a good one. Instead they should just return EINPROGRESS or
> > better yet 0.
> 
> Of the options you list, 0 does seem the best bet.
> 
> > As discussed on the glibc bug tracker issue, 0 is
> > probably best to avoid affecting applications that might treat unknown
> > results as a write error at close time.
> >
> > There is perhaps one other course of action that POSIX could have
> > taken: defining a macro, or a sysconf() value, to query whether the
> > implementation has left the fd open when close returns with EINTR. I
> > think this is a much worse solution, but at least it would have made
> > it possible to write safe applications.
> 
> Not so pretty, I agree.
> 
> So, from what I understand, there's a future path for implementations
> and applications. But, some existing applications (ones developed on
> Linux/FreeBSD/AIX that know not to retry the close() on EINTR) will be
> declared broken. That doesn't seem good. Or do I still miss something?

They're only broken insomuch as they leak file descriptors on
conforming implementations, which is exactly the situation now when
these applications are run on implementations with the other (now
POSIX-adopted) behavior. There is no regression. These applications
were already making a non-portable assumption (because the standard
did not offer them any way to avoid doign so).

> Also, there is a wider problem here, it seems to me. It is not just
> EINTR that is being special cased in close(). Linux and FreeBSD [1],
> and to some extent, AIX [2] special case *all* errors from close().
> For Linux and FreeBSD, the FD must be considered closed for any error
> that is returned. AIX has the following text:
> 
>     If the FileDescriptor parameter refers to a device and the
>     close subroutine actually results in a device close, and the
>     device close routine returns an error, the error is returned
>     to the application. However, the FileDescriptor parameter is
>     considered closed and it may not be used in any subsequent
>     calls.
> 
> (The AIX man page documents only the errors EBADF and EINTR, but that
> text hints to me that other errors are also possible.)

POSIX-future imposes this requirement too. The accepted text contains:

    For all other error situations (except for [EBADF] where fildes
    was invalid), fildes shall be closed.

Previously it was ambiguous for other errors too, which was a big
problem.

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

Rich