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

["Michael Kerrisk (man-pages)" <mtk dot manpages at gmail dot com>]

Rich,

On 12/07/2016 12:58 AM, Rich Felker wrote:
> On Tue, Dec 06, 2016 at 05:42:57PM -0500, Zack Weinberg wrote:
>> 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),
> 
> It's not. It's what some (roughly half?) of implementations do, and

Where do you get this number "half" from? That's far from what I've
managed to divine so far. From my reading of various source code,
and manual pages (for the benefit of others, see pasted message that
I sent to the Austin-l mailing list yesterday; Zack, I'll loop you
into that thread on my next reply), the situation is as follows.

1. Always close the FD, regardless of the error (including EINTR):

    Linux (verified from source)
    AIX (documented, and AIX 4.1.3 source)
    FreeBSD (from documentation and source)
    Historical System V R4 (from source)
    Solaris (from source)

   I've since taken a look at the NetBSD and OpenBSD code bases,
   and it appears that they do likewise. I assert this with
   less certainty though, since I've not spent so much time
   looking at these implementations.

   One of the other things that is clear, by the way, is that many
   of these systems did/do not accurately document what they do. 
   And those that do, did so only recently: FreeBSD in 2012 and 
   Linux (man-pages) in 2013.

2. Can return an EINTR while leaving the FD open:

    HP-UX (from documentation)

3. Unclear:

    Irix 6.5.5 (I looked at the source code. The Irix code is
    structured rather differently from other systems, and I
    couldn't really work out what goes on.)

Of course, there are many other systems (now mostly irrelevant)
that are not listed above, but from the above, "half" seems
quite wide of the mark.

> it's inconsistent with the meaning of EINTR. 

I don't really consider this statement to have much weight.
The general argument that "EINTR is being treated inconsistently
for close() (by contrast with other interfaces that return EINTR)"
has little weight in the face of the following:

1. It appears that majority of implementations (including the 
   majority of the currently relevant implementations) always
   close the FD, even in the case that EINTR returns.
   Furthermore, one could equally say that those implementations 
   are treating EINTR consistently with all other errors from 
   close().

2. Future POSIX.1 (http://austingroupbugs.net/view.php?id=529#c1200)
   proposes to special case the meaning of failure return from
   close(). According to the proposal, even on errors (other 
   than EINTR), close will close the FD (i.e., it will "succeed").
   This is consistent (except for the EINTR detail) with the majority
   of implementations. But, if consistency is the argument,then it's
   worth noting that by this definition, the meaning of "failure" 
   for close() is inconsistent with the meaning of "failure" for 
   other interfaces, where "failure" generally means that the 
   interface did not successfully complete the requested action.
   And then, in the middle of this special casing of the meaning 
   of "failure" for close(), the POSIX.1 proposal is to special 
   case one failure (EINTR) to mean something different from the 
   other failures from close().

> Read the whole thread, as
> this has already been discussed in-depth; repeating it for latecomers
> is not useful.

Yes, but a critical piece is absent from the Austin bug and Austin
mailing list discussion, as far as I can see: namely, the rationale
for changing the close() spec in one direction (forbidding that
close() returns EINTR *and* closes the file descriptor) rather than 
another (forbidding the possibility that close() returns EINTR and 
leaves the FD open) that would seem more consistent with the 
majority of implementations, and would not render those
implementations nonconforming according to future POSIX.1
(and would not render applications that depend on existing
implementation behavior "broken").

> In any case, the practical path forward that avoids the issue entirely
> is never generating EINTR at all, so that applications don't have to
> deal with it. See https://sourceware.org/bugzilla/show_bug.cgi?id=14627

Yes, but this ignores the fact that a burden is placed on (most)
implementations to fix a non-issue on those implementations and a
burden is placed on application developers to modify code that works
on the majority of implementations. And there is no clear advantage
to this change versus the less burdensome requirement of forbidding
the possibility that close() returns EINTR and leaves the FD open,
which requires changes to a minority(?) of implementations,
and no changes to applications.

Cheers,

Michael

===
Here's my message from yesterday to the austin-l list, for reference.
I'm continuing that discussion later today, elaborating on some of
the points made in this mail.

-------- Forwarded Message --------
Subject: Some questions on bug 529 (fildes unspecified on close()'s [EINTR])
Date: Tue, 6 Dec 2016 14:50:23 +0100
From: Michael Kerrisk <mtk.manpages@gmail.com>
To: austin-group-l@opengroup.org <austin-group-l@opengroup.org>
CC: Rich Felker <dalias@aerifal.cx>, Michael Kerrisk-manpages <mtk.manpages@gmail.com>, Eric Blake <eblake@redhat.com>, Geoff Clare <gwc@opengroup.org>

Hello all,

Following a discussion [4][5] (and a bit of education for me, thanks
to Rich Felker) on the GNU C Library (libc-alpha) mailing list, I have
some questions about bug 529 [0].

1. If I understand correctly, the proposed changes for Issue 8 will in
effect declare to be broken those existing applications that treat
EINTR from close() as "the file is closed". (I.e., per the proposed
changes [7], implementations will not be allowed to have close() fail
with EINTR while also closing the file.) However, those existing
applications are behaving correctly in terms of what the
implementations do. Is my understanding of the intentions of the Issue
8 changes correct? If yes, I presume the path is that such
applications would be conformant to older POSIX, but won't be
conformant to Issue 8. (Right?)

2. From the bug thread, I've missed something. I understand the
argument that EINTR was being treated inconsistently for close() on
Linux and other systems, but the thing is that implementations with
the close() "EINTR means the file descriptor is closed" behavior
exist, and seem even to be common. Why (given the variation in
existing implementations) was the decision taken to change the
specification for close() , instead of just adding a new posix_close()
that addresses the problem?

3. [An observation, not a question] As far as I can tell, the Linux
behavior for close() EINTR semantics is not an isolated case. One can
see this in the FreeBSD close(2) manual page [1] and in the AIX [2]
manual page. It seems also to be the norm for other implementations
(see my next point). HP-UX claims [6] in the documentation to leave
the file open on EINTR, but unfortunately, the source code isn't
publicly available to verify this

4. The interpretation for bug 529 doesn't address a wider issue. On
several implementations, the FD is always closed *for any error* that
close() may return. That is what Linux does (the FD is released very
early in the "close" processing, and errors may be reported
afterward). It's also what FreeBSD does, as documented in its close(2)
man page:

    In case of any error except EBADF, the supplied file descriptor
    is deallocated and therefore is no longer valid.

(By the way, that text seems to have been added in FreeBSD 9.1, in
early 2012, I presume to document existing behavior after someone read
this Austin bug report.)

For info, FreeBSD documents the following errors for close(): EBADF,
EINTR, ENOSPC, and ECONNRESET.

Looking at some historical source code (mostly from [3]) suggests that
the "close() always closes regardless of error return" behavior has a
long history, predating even POSIX.1-1990.

For example, in SVR4 for x86 (from the file sysvr4.tar.bz2 at [3]), we
see the following:

===
int
close(uap, rvp)
        register struct closea *uap;
        rval_t *rvp;
{
        file_t *fp;
        register int error;

        if (error = getf(uap->fdes, &fp))
                return error;
        error = closef(fp);
        setf(uap->fdes, NULLFP);
        return error;
}
===

In the above, getf() can return EBADF. The other errors are returned
by closef(), but the file descriptor is deallocated regardless of
errors by setf().

A similar pattern seems to have been preserved into at least late
OpenSolaris days (verified from looking at the initial commit of the
illumos source code). There we find the following in closeandsetf()
(called by close())

        error = closef(fp);

        setf(fd, newfp);

        return (error);

Looking at the code of closef() in AIX 4.1.3 suggests that, as on on
Linux and FreeBSD, the open file is always released, regardless of
errors.

For Irix, 6.5.5, I'm not sure (the code is not so easy to quickly
read); it may be that it does return errors while leaving the FD open.

So, my summary here is that many (perhaps most?) implementations are
similar to Linux and FreeBSD, but this isn't currently addressed in
POSIX, so far as I can tell. Should it be?

Cheers,

Michael

[0] http://austingroupbugs.net/view.php?id=529
[1] https://www.freebsd.org/cgi/man.cgi?query=close&apropos=0&sektion=0&manpath=FreeBSD+10.3-RELEASE+and+Ports&arch=default&format=html#ERRORS
[2] http://publib16.boulder.ibm.com/doc_link/en_US/a_doc_lib/libs/basetrf1/close.htm
[3] https://archive.org/download/various_operating_system_source_code
[4] https://sourceware.org/ml/libc-alpha/2016-12/msg00110.html
[5] https://sourceware.org/ml/libc-alpha/2016-12/msg00136.html
[6] http://www.unix.com/man-page/hpux/2/close/
[7] http://austingroupbugs.net/view.php?id=529#c1200


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/