[PATCH] Manual should discourage mixing TEMP_FAILURE_RETRY and close

[PATCH] Manual should discourage mixing TEMP_FAILURE_RETRY and close

[Mark Mentovai]

When the close system call is interrupted by a signal, the state of its
file descriptor argument is not specified[1]. On Linux, a close that
fails with EINTR must not be restarted because the file descriptor is
guaranteed to have been closed at that point[2]. Note that the kernel
itself never restarts a close when SA_RESTART is set or when no
user-space signal handler is present[3].

Because glibc is widely used on Linux, its documentation should state
affirmatively that the TEMP_FAILURE_RETRY, which restarts interrupted
system calls that fail with EINTR, is not to be used with close on
Linux. Examples in glibc documentation should avoid recommending
wrapping close with TEMP_FAILURE_RETRY.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html
[2] http://lkml.org/lkml/2005/9/10/129
[3] http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=ee731f4f7880b09ca147008ab46ad4e5f72cb8b

2013-12-03  Mark Mentovai  <mark@chromium.org>

	* manual/llio.texi (Opening and Closing Files): Document that
	TEMP_FAILURE_RETRY should not be used with close on Linux.
	(Duplicating Descriptors): Don't wrap close in TEMP_FAILURE_RETRY
	in example code.

---
 manual/llio.texi | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/manual/llio.texi b/manual/llio.texi
index b6c9260..f04a2cf 100644
--- a/manual/llio.texi
+++ b/manual/llio.texi
@@ -253,11 +253,9 @@ The @var{filedes} argument is not a valid file descriptor.
 @item EINTR
 The @code{close} call was interrupted by a signal.
 @xref{Interrupted Primitives}.
-Here is an example of how to handle @code{EINTR} properly:
-
-@smallexample
-TEMP_FAILURE_RETRY (close (desc));
-@end smallexample
+On Linux, @code{close} will have closed its file descriptor argument even when
+reporting @code{EINTR}.  It is not safe to retry calling @code{close} in this
+case, as would be done when wrapped in the @code{TEMP_FAILURE_RETRY} macro.
 
 @item ENOSPC
 @itemx EIO
@@ -2765,7 +2763,7 @@ if (pid == 0)
     @dots{}
     file = TEMP_FAILURE_RETRY (open (filename, O_RDONLY));
     dup2 (file, STDIN_FILENO);
-    TEMP_FAILURE_RETRY (close (file));
+    close (file);
     execv (program, NULL);
   @}
 @end smallexample
-- 
1.8.3.4

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

[Ondřej Bílka]

On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> When the close system call is interrupted by a signal, the state of its
> file descriptor argument is not specified[1]. On Linux, a close that
> fails with EINTR must not be restarted because the file descriptor is
> guaranteed to have been closed at that point[2]. Note that the kernel
> itself never restarts a close when SA_RESTART is set or when no
> user-space signal handler is present[3].
>
This is linux problem, POSIX now says

http://austingroupbugs.net/view.php?id=529 

If close( ) is interrupted by a signal that is to be caught, then it
is unspecified whether it returns -1 with errno set to [EINTR] with
fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
with fildes being closed, or returns 0 to indicate successful
completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
the option of returning -1 with errno set to [EINTR] and fildes
remaining open shall not occur. If close() returns -1 with errno set
to [EINTR], it is unspecified whether fildes can subsequently be
passed to any function except close( ) or posix_close( ) without error.
For all other error situations (except for [EBADF] where fildes was
invalid), fildes shall be closed. If fildes was closed even though
the close operation is incomplete, the close operation shall continue
asynchronously and the process shall have no further ability to track
the completion or final status of the close operation.

> Because glibc is widely used on Linux, its documentation should state
> affirmatively that the TEMP_FAILURE_RETRY, which restarts interrupted
> system calls that fail with EINTR, is not to be used with close on
> Linux. Examples in glibc documentation should avoid recommending
> wrapping close with TEMP_FAILURE_RETRY.
> 
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html
> [2] http://lkml.org/lkml/2005/9/10/129
> [3] http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=ee731f4f7880b09ca147008ab46ad4e5f72cb8b
> 
> 2013-12-03  Mark Mentovai  <mark@chromium.org>
> 
> 	* manual/llio.texi (Opening and Closing Files): Document that
> 	TEMP_FAILURE_RETRY should not be used with close on Linux.
> 	(Duplicating Descriptors): Don't wrap close in TEMP_FAILURE_RETRY
> 	in example code.
> 
> ---
>  manual/llio.texi | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/manual/llio.texi b/manual/llio.texi
> index b6c9260..f04a2cf 100644
> --- a/manual/llio.texi
> +++ b/manual/llio.texi
> @@ -253,11 +253,9 @@ The @var{filedes} argument is not a valid file descriptor.
>  @item EINTR
>  The @code{close} call was interrupted by a signal.
>  @xref{Interrupted Primitives}.
> -Here is an example of how to handle @code{EINTR} properly:
> -
> -@smallexample
> -TEMP_FAILURE_RETRY (close (desc));
> -@end smallexample
> +On Linux, @code{close} will have closed its file descriptor argument even when
> +reporting @code{EINTR}.  It is not safe to retry calling @code{close} in this
> +case, as would be done when wrapped in the @code{TEMP_FAILURE_RETRY} macro.
>  
>  @item ENOSPC
>  @itemx EIO
> @@ -2765,7 +2763,7 @@ if (pid == 0)
>      @dots{}
>      file = TEMP_FAILURE_RETRY (open (filename, O_RDONLY));
>      dup2 (file, STDIN_FILENO);
> -    TEMP_FAILURE_RETRY (close (file));
> +    close (file);
>      execv (program, NULL);
>    @}
>  @end smallexample
> -- 
> 1.8.3.4

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

[Rich Felker]

On Wed, Dec 04, 2013 at 01:08:21AM +0100, Ondřej Bílka wrote:
> On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> > When the close system call is interrupted by a signal, the state of its
> > file descriptor argument is not specified[1]. On Linux, a close that
> > fails with EINTR must not be restarted because the file descriptor is
> > guaranteed to have been closed at that point[2]. Note that the kernel
> > itself never restarts a close when SA_RESTART is set or when no
> > user-space signal handler is present[3].
> >
> This is linux problem, POSIX now says
> 
> http://austingroupbugs.net/view.php?id=529 
> 
> If close( ) is interrupted by a signal that is to be caught, then it
> is unspecified whether it returns -1 with errno set to [EINTR] with
> fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> [...]

Assuming Linux will never switch to the correct POSIX EINTR behavior
(the kernel folks are adamantly against it), would it be acceptable
for glibc to fix up the resulting errno value from EINTR to
EINPROGRESS, so that correct applications following POSIX will not
loop on EINTR and introduce (extremely dangerous) double-close bugs?
As long as the kernel folks are committed to keeping the current
behavior, getting EINTR from the kernel essentially means EINPROGRESS
and I think the replacement is justified. That's what I'm doing in
musl.

BTW if glibc ever fixes the cancellation races (bug #12683) handling
close correctly there is important too (you can't act on cancellation
when it interrupts a close syscall because the side effects have
already completed, despite the errno value from the kernel, EINTR,
making it appear that side effects did not occur).

Rich

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

[Mark Mentovai]

Ondřej Bílka wrote:
> On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
>> When the close system call is interrupted by a signal, the state of its
>> file descriptor argument is not specified[1]. On Linux, a close that
>> fails with EINTR must not be restarted because the file descriptor is
>> guaranteed to have been closed at that point[2]. Note that the kernel
>> itself never restarts a close when SA_RESTART is set or when no
>> user-space signal handler is present[3].
>>
> This is linux problem, POSIX now says
>
> http://austingroupbugs.net/view.php?id=529
>
> If close( ) is interrupted by a signal that is to be caught, then it
> is unspecified whether it returns -1 with errno set to [EINTR] with
> fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> with fildes being closed, or returns 0 to indicate successful
> completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> the option of returning -1 with errno set to [EINTR] and fildes
> remaining open shall not occur. If close() returns -1 with errno set
> to [EINTR], it is unspecified whether fildes can subsequently be
> passed to any function except close( ) or posix_close( ) without error.
> For all other error situations (except for [EBADF] where fildes was
> invalid), fildes shall be closed. If fildes was closed even though
> the close operation is incomplete, the close operation shall continue
> asynchronously and the process shall have no further ability to track
> the completion or final status of the close operation.

Linux (the kernel) doesn’t implement that behavior yet, and it may
never. In the absence of a workaround in glibc to map close EINTR
failures to EINPROGRESS, the glibc documentation currently recommends
doing something dangerous on Linux.

My proposed documentation patch doesn’t say that it’s never right to
mix TEMP_FAILURE_RETRY with close, but it does say that they shouldn’t
be combined on Linux.

>> Because glibc is widely used on Linux, its documentation should state
>> affirmatively that the TEMP_FAILURE_RETRY, which restarts interrupted
>> system calls that fail with EINTR, is not to be used with close on
>> Linux. Examples in glibc documentation should avoid recommending
>> wrapping close with TEMP_FAILURE_RETRY.
>>
>> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html
>> [2] http://lkml.org/lkml/2005/9/10/129
>> [3] http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=ee731f4f7880b09ca147008ab46ad4e5f72cb8b
>>
>> 2013-12-03  Mark Mentovai  <mark@chromium.org>
>>
>>       * manual/llio.texi (Opening and Closing Files): Document that
>>       TEMP_FAILURE_RETRY should not be used with close on Linux.
>>       (Duplicating Descriptors): Don't wrap close in TEMP_FAILURE_RETRY
>>       in example code.
>>
>> ---
>>  manual/llio.texi | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/manual/llio.texi b/manual/llio.texi
>> index b6c9260..f04a2cf 100644
>> --- a/manual/llio.texi
>> +++ b/manual/llio.texi
>> @@ -253,11 +253,9 @@ The @var{filedes} argument is not a valid file descriptor.
>>  @item EINTR
>>  The @code{close} call was interrupted by a signal.
>>  @xref{Interrupted Primitives}.
>> -Here is an example of how to handle @code{EINTR} properly:
>> -
>> -@smallexample
>> -TEMP_FAILURE_RETRY (close (desc));
>> -@end smallexample
>> +On Linux, @code{close} will have closed its file descriptor argument even when
>> +reporting @code{EINTR}.  It is not safe to retry calling @code{close} in this
>> +case, as would be done when wrapped in the @code{TEMP_FAILURE_RETRY} macro.
>>
>>  @item ENOSPC
>>  @itemx EIO
>> @@ -2765,7 +2763,7 @@ if (pid == 0)
>>      @dots{}
>>      file = TEMP_FAILURE_RETRY (open (filename, O_RDONLY));
>>      dup2 (file, STDIN_FILENO);
>> -    TEMP_FAILURE_RETRY (close (file));
>> +    close (file);
>>      execv (program, NULL);
>>    @}
>>  @end smallexample
>> --
>> 1.8.3.4

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

[Michael Kerrisk]

For what it's worth, well after the fact...

On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
> On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
>> When the close system call is interrupted by a signal, the state of its
>> file descriptor argument is not specified[1]. On Linux, a close that
>> fails with EINTR must not be restarted because the file descriptor is
>> guaranteed to have been closed at that point[2]. Note that the kernel
>> itself never restarts a close when SA_RESTART is set or when no
>> user-space signal handler is present[3].
>>
> This is linux problem, POSIX now says
>
> http://austingroupbugs.net/view.php?id=529
>
> If close( ) is interrupted by a signal that is to be caught, then it
> is unspecified whether it returns -1 with errno set to [EINTR] with
> fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> with fildes being closed, or returns 0 to indicate successful
> completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> the option of returning -1 with errno set to [EINTR] and fildes
> remaining open shall not occur. If close() returns -1 with errno set
> to [EINTR], it is unspecified whether fildes can subsequently be
> passed to any function except close( ) or posix_close( ) without error.
> For all other error situations (except for [EBADF] where fildes was
> invalid), fildes shall be closed. If fildes was closed even though
> the close operation is incomplete, the close operation shall continue
> asynchronously and the process shall have no further ability to track
> the completion or final status of the close operation.

Yes, but POSIX never said that. That was a proposal that was
discussed, that subsequently got shot down... Older POSIX.1 was wrong.
It's worth reading that entire Austin bug for the subsequent
discussion.

POSIX.1-2016 says under close()

       If  close()  is  interrupted  by a signal that is to be caught, it
       shall return −1 with errno set to [EINTR] and the state of  fildes
       is  unspecified.  If  an  I/O error occurred while reading from or
       writing to the file system during close(), it may return  −1  with
       errno set to [EIO]; if this error is returned, the state of fildes
       is unspecified.

(The only three errors mandated in POSIX.1-2016 for close() are EINTR,
EIO, and EBADF.)

My takeaway from the above POSIX text is that it's perfectly
legitimate for an implementation to say that  retrying close() after
*any* error is the wrong thing to do. Linux does this. So does
FreeBSD, where the man page says:

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

And according to a claim in the thread here:
https://news.ycombinator.com/item?id=3363884 , ancient AIX also
explicitly documented that the FD was closed in an EINTR error was
returned. This looks to be confirmed in
http://publib16.boulder.ibm.com/doc_link/en_US/a_doc_lib/libs/basetrf1/close.htm

And from the look of http://austingroupbugs.net/view.php?id=529#c1200
, the plan in the future is that POSIX will allow even more
flexibility to an implementation in the next major release (who knows
when).

>> Because glibc is widely used on Linux, its documentation should state
>> affirmatively that the TEMP_FAILURE_RETRY, which restarts interrupted
>> system calls that fail with EINTR, is not to be used with close on
>> Linux. Examples in glibc documentation should avoid recommending
>> wrapping close with TEMP_FAILURE_RETRY.
>>
>> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html
>> [2] http://lkml.org/lkml/2005/9/10/129
>> [3] http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=ee731f4f7880b09ca147008ab46ad4e5f72cb8b
>>
>> 2013-12-03  Mark Mentovai  <mark@chromium.org>
>>
>>       * manual/llio.texi (Opening and Closing Files): Document that
>>       TEMP_FAILURE_RETRY should not be used with close on Linux.
>>       (Duplicating Descriptors): Don't wrap close in TEMP_FAILURE_RETRY
>>       in example code.

This patch of Mark's really should be applied in some form, I think.
The glibc manual is currently advising users to do something actively
dangerous on at least two popular systems, and it has no justification
in current POSIX.

I'm just in the process of beefing up the Linux close man page on this
subject to say:

       Not checking the return value of close() is a common but neverthe‐
       less  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.  This can especially be observed with
       NFS and with disk quota.

       Note, however, that the return value should be used only for diag‐
       nostics.  Retrying the close() after an error is the  wrong  thing
       to  do, since this may cause a reused file descriptor from another
       thread to be closed.  (The kernel  releases  the  file  descriptor
       early in the close operation, freeing it for reuse; the steps that
       may return an error, such as flushing data  to  disk,  occur  only
       later  in the close operation.)  In particular, close() should not
       be retried after an EINTR error.  (POSIX.1 says:  "If  close()  is
       interrupted  by  a signal that is to be caught, it shall return -1
       with errno set to EINTR and the state of fildes is unspecified.")

       Careful users who want  to  know  about  I/O  errors  may  precede
       close() with a call to fsync(2).

Cheers,

Michael

>>
>> ---
>>  manual/llio.texi | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/manual/llio.texi b/manual/llio.texi
>> index b6c9260..f04a2cf 100644
>> --- a/manual/llio.texi
>> +++ b/manual/llio.texi
>> @@ -253,11 +253,9 @@ The @var{filedes} argument is not a valid file descriptor.
>>  @item EINTR
>>  The @code{close} call was interrupted by a signal.
>>  @xref{Interrupted Primitives}.
>> -Here is an example of how to handle @code{EINTR} properly:
>> -
>> -@smallexample
>> -TEMP_FAILURE_RETRY (close (desc));
>> -@end smallexample
>> +On Linux, @code{close} will have closed its file descriptor argument even when
>> +reporting @code{EINTR}.  It is not safe to retry calling @code{close} in this
>> +case, as would be done when wrapped in the @code{TEMP_FAILURE_RETRY} macro.
>>
>>  @item ENOSPC
>>  @itemx EIO
>> @@ -2765,7 +2763,7 @@ if (pid == 0)
>>      @dots{}
>>      file = TEMP_FAILURE_RETRY (open (filename, O_RDONLY));
>>      dup2 (file, STDIN_FILENO);
>> -    TEMP_FAILURE_RETRY (close (file));
>> +    close (file);
>>      execv (program, NULL);
>>    @}
>>  @end smallexample
>> --
>> 1.8.3.4
>

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

[Rich Felker]

On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
> For what it's worth, well after the fact...
> 
> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> >> When the close system call is interrupted by a signal, the state of its
> >> file descriptor argument is not specified[1]. On Linux, a close that
> >> fails with EINTR must not be restarted because the file descriptor is
> >> guaranteed to have been closed at that point[2]. Note that the kernel
> >> itself never restarts a close when SA_RESTART is set or when no
> >> user-space signal handler is present[3].
> >>
> > This is linux problem, POSIX now says
> >
> > http://austingroupbugs.net/view.php?id=529
> >
> > If close( ) is interrupted by a signal that is to be caught, then it
> > is unspecified whether it returns -1 with errno set to [EINTR] with
> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> > with fildes being closed, or returns 0 to indicate successful
> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> > the option of returning -1 with errno set to [EINTR] and fildes
> > remaining open shall not occur. If close() returns -1 with errno set
> > to [EINTR], it is unspecified whether fildes can subsequently be
> > passed to any function except close( ) or posix_close( ) without error.
> > For all other error situations (except for [EBADF] where fildes was
> > invalid), fildes shall be closed. If fildes was closed even though
> > the close operation is incomplete, the close operation shall continue
> > asynchronously and the process shall have no further ability to track
> > the completion or final status of the close operation.
> 
> Yes, but POSIX never said that. That was a proposal that was
> discussed, that subsequently got shot down...

It was not shot down. It was accepted for inclusion in the next issue
of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
cannot include non-bugfix changes.

Rich

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

[Michael Kerrisk]

Hi,

On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
> On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
>> For what it's worth, well after the fact...
>>
>> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
>> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
>> >> When the close system call is interrupted by a signal, the state of its
>> >> file descriptor argument is not specified[1]. On Linux, a close that
>> >> fails with EINTR must not be restarted because the file descriptor is
>> >> guaranteed to have been closed at that point[2]. Note that the kernel
>> >> itself never restarts a close when SA_RESTART is set or when no
>> >> user-space signal handler is present[3].
>> >>
>> > This is linux problem, POSIX now says
>> >
>> > http://austingroupbugs.net/view.php?id=529
>> >
>> > If close( ) is interrupted by a signal that is to be caught, then it
>> > is unspecified whether it returns -1 with errno set to [EINTR] with
>> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
>> > with fildes being closed, or returns 0 to indicate successful
>> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
>> > the option of returning -1 with errno set to [EINTR] and fildes
>> > remaining open shall not occur. If close() returns -1 with errno set
>> > to [EINTR], it is unspecified whether fildes can subsequently be
>> > passed to any function except close( ) or posix_close( ) without error.
>> > For all other error situations (except for [EBADF] where fildes was
>> > invalid), fildes shall be closed. If fildes was closed even though
>> > the close operation is incomplete, the close operation shall continue
>> > asynchronously and the process shall have no further ability to track
>> > the completion or final status of the close operation.
>>
>> Yes, but POSIX never said that. That was a proposal that was
>> discussed, that subsequently got shot down...
>
> It was not shot down. It was accepted for inclusion in the next issue
> of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
> cannot include non-bugfix changes.

You're right. I misread the order of entries in bug (was looking at an
older proposal that was shot down), and mismatched with what Ondrej
quoted.

But the point is still the same. Current POSIX.1 permits the Linux
behavior, and future POSIX will too. So, the glibc manual still should
get Mark's fix, I'd say.

Cheers,

Michael

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

[Rich Felker]

On Mon, Dec 05, 2016 at 05:18:28PM +0100, Michael Kerrisk wrote:
> Hi,
> 
> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
> >> For what it's worth, well after the fact...
> >>
> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> >> >> When the close system call is interrupted by a signal, the state of its
> >> >> file descriptor argument is not specified[1]. On Linux, a close that
> >> >> fails with EINTR must not be restarted because the file descriptor is
> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
> >> >> itself never restarts a close when SA_RESTART is set or when no
> >> >> user-space signal handler is present[3].
> >> >>
> >> > This is linux problem, POSIX now says
> >> >
> >> > http://austingroupbugs.net/view.php?id=529
> >> >
> >> > If close( ) is interrupted by a signal that is to be caught, then it
> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> >> > with fildes being closed, or returns 0 to indicate successful
> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> >> > the option of returning -1 with errno set to [EINTR] and fildes
> >> > remaining open shall not occur. If close() returns -1 with errno set
> >> > to [EINTR], it is unspecified whether fildes can subsequently be
> >> > passed to any function except close( ) or posix_close( ) without error..
> >> > For all other error situations (except for [EBADF] where fildes was
> >> > invalid), fildes shall be closed. If fildes was closed even though
> >> > the close operation is incomplete, the close operation shall continue
> >> > asynchronously and the process shall have no further ability to track
> >> > the completion or final status of the close operation.
> >>
> >> Yes, but POSIX never said that. That was a proposal that was
> >> discussed, that subsequently got shot down...
> >
> > It was not shot down. It was accepted for inclusion in the next issue
> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
> > cannot include non-bugfix changes.
> 
> You're right. I misread the order of entries in bug (was looking at an
> older proposal that was shot down), and mismatched with what Ondrej
> quoted.
> 
> But the point is still the same. Current POSIX.1 permits the Linux
> behavior, and future POSIX will too. So, the glibc manual still should
> get Mark's fix, I'd say.

No, POSIX-future does not permit the Linux behavior. If EINTR is
returned, the fd must still be "open" (not re-assignable, compatible
with calling close on it again, though it may be in a partially-closed
and unusable state). If the fd has been deallocated such that it's not
safe to call close on it again, close must return some value other
than EINTR; EINPROGRESS is added as the canonical error for the case
where closing is not complete and happens in the background.

Rich

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

[Michael Kerrisk]

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:
>> Hi,
>>
>> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
>> >> For what it's worth, well after the fact...
>> >>
>> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
>> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
>> >> >> When the close system call is interrupted by a signal, the state of its
>> >> >> file descriptor argument is not specified[1]. On Linux, a close that
>> >> >> fails with EINTR must not be restarted because the file descriptor is
>> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
>> >> >> itself never restarts a close when SA_RESTART is set or when no
>> >> >> user-space signal handler is present[3].
>> >> >>
>> >> > This is linux problem, POSIX now says
>> >> >
>> >> > http://austingroupbugs.net/view.php?id=529
>> >> >
>> >> > If close( ) is interrupted by a signal that is to be caught, then it
>> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
>> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
>> >> > with fildes being closed, or returns 0 to indicate successful
>> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
>> >> > the option of returning -1 with errno set to [EINTR] and fildes
>> >> > remaining open shall not occur. If close() returns -1 with errno set
>> >> > to [EINTR], it is unspecified whether fildes can subsequently be
>> >> > passed to any function except close( ) or posix_close( ) without error..
>> >> > For all other error situations (except for [EBADF] where fildes was
>> >> > invalid), fildes shall be closed. If fildes was closed even though
>> >> > the close operation is incomplete, the close operation shall continue
>> >> > asynchronously and the process shall have no further ability to track
>> >> > the completion or final status of the close operation.
>> >>
>> >> Yes, but POSIX never said that. That was a proposal that was
>> >> discussed, that subsequently got shot down...
>> >
>> > It was not shot down. It was accepted for inclusion in the next issue
>> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
>> > cannot include non-bugfix changes.
>>
>> You're right. I misread the order of entries in bug (was looking at an
>> older proposal that was shot down), and mismatched with what Ondrej
>> quoted.
>>
>> But the point is still the same. Current POSIX.1 permits the Linux
>> behavior, and future POSIX will too. So, the glibc manual still should
>> get Mark's fix, I'd say.
>
> No, POSIX-future does not permit the Linux behavior. If EINTR is
> returned, the fd must still be "open" (not re-assignable, compatible
> with calling close on it again, though it may be in a partially-closed
> and unusable state). If the fd has been deallocated such that it's not
> safe to call close on it again, close must return some value other
> than EINTR; EINPROGRESS is added as the canonical error for the case
> where closing is not complete and happens in the background.

Yes, you got me. I went away and reread this in the time you wrote the
mail, and came to the same conclusion.

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?

Cheers,

Michael

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

[Rich Felker]

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:
> >> Hi,
> >>
> >> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
> >> >> For what it's worth, well after the fact...
> >> >>
> >> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam..cz> wrote:
> >> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> >> >> >> When the close system call is interrupted by a signal, the state of its
> >> >> >> file descriptor argument is not specified[1]. On Linux, a close that
> >> >> >> fails with EINTR must not be restarted because the file descriptor is
> >> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
> >> >> >> itself never restarts a close when SA_RESTART is set or when no
> >> >> >> user-space signal handler is present[3].
> >> >> >>
> >> >> > This is linux problem, POSIX now says
> >> >> >
> >> >> > http://austingroupbugs.net/view.php?id=529
> >> >> >
> >> >> > If close( ) is interrupted by a signal that is to be caught, then it
> >> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
> >> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> >> >> > with fildes being closed, or returns 0 to indicate successful
> >> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> >> >> > the option of returning -1 with errno set to [EINTR] and fildes
> >> >> > remaining open shall not occur. If close() returns -1 with errno set
> >> >> > to [EINTR], it is unspecified whether fildes can subsequently be
> >> >> > passed to any function except close( ) or posix_close( ) without error..
> >> >> > For all other error situations (except for [EBADF] where fildes was
> >> >> > invalid), fildes shall be closed. If fildes was closed even though
> >> >> > the close operation is incomplete, the close operation shall continue
> >> >> > asynchronously and the process shall have no further ability to track
> >> >> > the completion or final status of the close operation.
> >> >>
> >> >> Yes, but POSIX never said that. That was a proposal that was
> >> >> discussed, that subsequently got shot down...
> >> >
> >> > It was not shot down. It was accepted for inclusion in the next issue
> >> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
> >> > cannot include non-bugfix changes.
> >>
> >> You're right. I misread the order of entries in bug (was looking at an
> >> older proposal that was shot down), and mismatched with what Ondrej
> >> quoted.
> >>
> >> But the point is still the same. Current POSIX.1 permits the Linux
> >> behavior, and future POSIX will too. So, the glibc manual still should
> >> get Mark's fix, I'd say.
> >
> > No, POSIX-future does not permit the Linux behavior. If EINTR is
> > returned, the fd must still be "open" (not re-assignable, compatible
> > with calling close on it again, though it may be in a partially-closed
> > and unusable state). If the fd has been deallocated such that it's not
> > safe to call close on it again, close must return some value other
> > than EINTR; EINPROGRESS is added as the canonical error for the case
> > where closing is not complete and happens in the background.
> 
> Yes, you got me. I went away and reread this in the time you wrote the
> mail, and came to the same conclusion.
> 
> 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.

Rich

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

[Michael Kerrisk (man-pages)]

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:
>> >> Hi,
>> >>
>> >> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> >> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
>> >> >> For what it's worth, well after the fact...
>> >> >>
>> >> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam..cz> wrote:
>> >> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
>> >> >> >> When the close system call is interrupted by a signal, the state of its
>> >> >> >> file descriptor argument is not specified[1]. On Linux, a close that
>> >> >> >> fails with EINTR must not be restarted because the file descriptor is
>> >> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
>> >> >> >> itself never restarts a close when SA_RESTART is set or when no
>> >> >> >> user-space signal handler is present[3].
>> >> >> >>
>> >> >> > This is linux problem, POSIX now says
>> >> >> >
>> >> >> > http://austingroupbugs.net/view.php?id=529
>> >> >> >
>> >> >> > If close( ) is interrupted by a signal that is to be caught, then it
>> >> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
>> >> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
>> >> >> > with fildes being closed, or returns 0 to indicate successful
>> >> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
>> >> >> > the option of returning -1 with errno set to [EINTR] and fildes
>> >> >> > remaining open shall not occur. If close() returns -1 with errno set
>> >> >> > to [EINTR], it is unspecified whether fildes can subsequently be
>> >> >> > passed to any function except close( ) or posix_close( ) without error..
>> >> >> > For all other error situations (except for [EBADF] where fildes was
>> >> >> > invalid), fildes shall be closed. If fildes was closed even though
>> >> >> > the close operation is incomplete, the close operation shall continue
>> >> >> > asynchronously and the process shall have no further ability to track
>> >> >> > the completion or final status of the close operation.
>> >> >>
>> >> >> Yes, but POSIX never said that. That was a proposal that was
>> >> >> discussed, that subsequently got shot down...
>> >> >
>> >> > It was not shot down. It was accepted for inclusion in the next issue
>> >> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
>> >> > cannot include non-bugfix changes.
>> >>
>> >> You're right. I misread the order of entries in bug (was looking at an
>> >> older proposal that was shot down), and mismatched with what Ondrej
>> >> quoted.
>> >>
>> >> But the point is still the same. Current POSIX.1 permits the Linux
>> >> behavior, and future POSIX will too. So, the glibc manual still should
>> >> get Mark's fix, I'd say.
>> >
>> > No, POSIX-future does not permit the Linux behavior. If EINTR is
>> > returned, the fd must still be "open" (not re-assignable, compatible
>> > with calling close on it again, though it may be in a partially-closed
>> > and unusable state). If the fd has been deallocated such that it's not
>> > safe to call close on it again, close must return some value other
>> > than EINTR; EINPROGRESS is added as the canonical error for the case
>> > where closing is not complete and happens in the background.
>>
>> Yes, you got me. I went away and reread this in the time you wrote the
>> mail, and came to the same conclusion.
>>
>> 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?

Cheers,

Michael

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

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

[Rich Felker]

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:
> >> >> Hi,
> >> >>
> >> >> On Mon, Dec 5, 2016 at 5:11 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> >> > On Mon, Dec 05, 2016 at 03:48:56PM +0100, Michael Kerrisk wrote:
> >> >> >> For what it's worth, well after the fact...
> >> >> >>
> >> >> >> On Wed, Dec 4, 2013 at 1:08 AM, Ondřej Bílka <neleai@seznam..cz> wrote:
> >> >> >> > On Tue, Dec 03, 2013 at 01:05:21PM -0500, Mark Mentovai wrote:
> >> >> >> >> When the close system call is interrupted by a signal, the state of its
> >> >> >> >> file descriptor argument is not specified[1]. On Linux, a close that
> >> >> >> >> fails with EINTR must not be restarted because the file descriptor is
> >> >> >> >> guaranteed to have been closed at that point[2]. Note that the kernel
> >> >> >> >> itself never restarts a close when SA_RESTART is set or when no
> >> >> >> >> user-space signal handler is present[3].
> >> >> >> >>
> >> >> >> > This is linux problem, POSIX now says
> >> >> >> >
> >> >> >> > http://austingroupbugs.net/view.php?id=529
> >> >> >> >
> >> >> >> > If close( ) is interrupted by a signal that is to be caught, then it
> >> >> >> > is unspecified whether it returns -1 with errno set to [EINTR] with
> >> >> >> > fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
> >> >> >> > with fildes being closed, or returns 0 to indicate successful
> >> >> >> > completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
> >> >> >> > the option of returning -1 with errno set to [EINTR] and fildes
> >> >> >> > remaining open shall not occur. If close() returns -1 with errno set
> >> >> >> > to [EINTR], it is unspecified whether fildes can subsequently be
> >> >> >> > passed to any function except close( ) or posix_close( ) without error..
> >> >> >> > For all other error situations (except for [EBADF] where fildes was
> >> >> >> > invalid), fildes shall be closed. If fildes was closed even though
> >> >> >> > the close operation is incomplete, the close operation shall continue
> >> >> >> > asynchronously and the process shall have no further ability to track
> >> >> >> > the completion or final status of the close operation.
> >> >> >>
> >> >> >> Yes, but POSIX never said that. That was a proposal that was
> >> >> >> discussed, that subsequently got shot down...
> >> >> >
> >> >> > It was not shot down. It was accepted for inclusion in the next issue
> >> >> > of POSIX. 2016 is not a new issue; it's a TC for POSIX-2008, and
> >> >> > cannot include non-bugfix changes.
> >> >>
> >> >> You're right. I misread the order of entries in bug (was looking at an
> >> >> older proposal that was shot down), and mismatched with what Ondrej
> >> >> quoted.
> >> >>
> >> >> But the point is still the same. Current POSIX.1 permits the Linux
> >> >> behavior, and future POSIX will too. So, the glibc manual still should
> >> >> get Mark's fix, I'd say.
> >> >
> >> > No, POSIX-future does not permit the Linux behavior. If EINTR is
> >> > returned, the fd must still be "open" (not re-assignable, compatible
> >> > with calling close on it again, though it may be in a partially-closed
> >> > and unusable state). If the fd has been deallocated such that it's not
> >> > safe to call close on it again, close must return some value other
> >> > than EINTR; EINPROGRESS is added as the canonical error for the case
> >> > where closing is not complete and happens in the background.
> >>
> >> Yes, you got me. I went away and reread this in the time you wrote the
> >> mail, and came to the same conclusion.
> >>
> >> 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? It was a bug that they ever
allowed ambiguity; 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.

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

Rich

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

[Michael Kerrisk (man-pages)]

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?

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

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

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

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

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.

Thanks for your explanations, Rich.

Cheers,

Michael

[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

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

Re: [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

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

[Zack Weinberg]

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

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

[Rich Felker]

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
it's inconsistent with the meaning of EINTR. Read the whole thread, as
this has already been discussed in-depth; repeating it for latecomers
is not useful.

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

Rich

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

[Michael Kerrisk (man-pages)]

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/

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

[Michael Kerrisk (man-pages)]

On 12/06/2016 11:42 PM, 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),

I believe it is "most (not all) implementations.

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

That's my opinion.

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

Exactly. This is the sense in which I'd argue that the POSIX.1
proposal (to forbid an implementation from returning EINTR
and close the FD) potentially worsens portability.

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

Yes, I still think the glibc manual needs fixing. But, by now, I've
tried to be more nuanced in a further revision to the close(2) man
page, which by now reads:

       A careful programmer will check the return value of close(), since
       it  is quite possible that errors on a previous write(2) operation
       are reported only on the final close() that releases the open file
       description.   Failing  to  check  the return value when closing a
       file may lead to silent loss of  data.   This  can  especially  be
       observed with NFS and with disk quota.

       Note, however, that a failure return should be used only for diag‐
       nostic purposes (i.e., a warning to the application that there may
       still  be  I/O pending or there may have been failed I/O) or reme‐
       dial purposes (e.g., writing the file  once  more  or  creating  a
       backup).

       Retrying  the close() after a failure return is the wrong thing to
       do, since this may cause a reused  file  descriptor  from  another
       thread  to  be  closed.   This  can occur because the Linux kernel
       always releases the file descriptor early in the close  operation,
       freeing  it for reuse; the steps that may return an error, such as
       flushing data to the filesystem or device, occur only later in the
       close operation.

       Many   other  implementations  similarly  always  close  the  file
       descriptor (except in the case of EBADF,  meaning  that  the  file
       descriptor  was invalid) even if they subsequently report an error
       on return from close().   POSIX.1  is  currently  silent  on  this
       point,  but  there  are plans to mandate this behavior in the next
       major release of the standard

       A careful programmer who wants to know about I/O errors  may  pre‐
       cede close() with a call to fsync(2).

       The  EINTR  error is a somewhat special case.  Regarding the EINTR
       error, POSIX.1-2013 says:

              If close() is interrupted by a signal that is to be caught,
              it shall return -1 with errno set to EINTR and the state of
              fildes is unspecified.

       This permits the behavior that occurs  on  Linux  and  many  other
       implementations,  where, as with other errors that may be reported
       by close(), the file descriptor is guaranteed to be closed.   How‐
       ever, it also permits another possibility: that the implementation
       returns an  EINTR  error  and  keeps  the  file  descriptor  open.
       (According  to its documentation, HP-UX's close() does this.)  The
       caller must then once more use close() to close the file  descrip‐
       tor, to avoid file descriptor leaks.  This divergence in implemen‐
       tation behaviors provides a difficult hurdle for portable applica‐
       tions,  since  on many implementations, close() must not be called
       again after an EINTR error, and on at least one, close()  must  be
       called  again.   There are plans to address this conundrum for the
       next major release of the POSIX.1 standard.

(Excepting the first paragraph, all of the text there was added by me,
and can be considered in the public domain if any of it is useful for
inclusion in the glibc manual.)

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

While the above is accurate for glibc on Linux, and is also
true for many other systems (AFAICT), I can see why there
may be resistance to accepting it for the glibc manual.
Maybe, some more nuanced text would be helpful. In any case
the current manual text should still be changed, IMO.

Cheers,

Michael


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

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

[Paul Eggert]

On 12/05/2016 08:22 AM, Rich Felker wrote:
> POSIX-future does not permit the Linux behavior.

I'm not seeing where POSIX-future (i.e., 
http://austingroupbugs.net/view.php?id=529 response 0001200 dated 
2012-04-19 15:18) prohibits the GNU/Linux behavior. If there is such a 
prohibition, isn't that more likely to be a bug in POSIX-future than in 
GNU/Linux?

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

[Rich Felker]

On Mon, Dec 05, 2016 at 08:32:37AM -0800, Paul Eggert wrote:
> On 12/05/2016 08:22 AM, Rich Felker wrote:
> >POSIX-future does not permit the Linux behavior.
> 
> I'm not seeing where POSIX-future (i.e.,
> http://austingroupbugs.net/view.php?id=529 response 0001200 dated
> 2012-04-19 15:18) prohibits the GNU/Linux behavior.

See the following text:

------------------------------------------------------------------------
At line 22871 [XSH close DESCRIPTION], change:

If close( ) is interrupted by a signal that is to be caught, it shall
return -1 with errno set to [EINTR] and the state of fildes is
unspecified. If an I/O error occurred while reading from or writing
to the file system during close( ), it may return -1 with errno set to
[EIO]; if this error is returned, the state of fildes is unspecified.

to:

If close( ) is interrupted by a signal that is to be caught, then it
is unspecified whether it returns -1 with errno set to [EINTR] with
                                                ^^^^^^^^^^^^^^^^^^^
fildes remaining open, or returns -1 with errno set to [EINPROGRESS]
^^^^^^^^^^^^^^^^^^^^^
with fildes being closed, or returns 0 to indicate successful
completion; except that if POSIX_CLOSE_RESTART is defined as 0, then
the option of returning -1 with errno set to [EINTR] and fildes
remaining open shall not occur. If close() returns -1 with errno set
to [EINTR], it is unspecified whether fildes can subsequently be
passed to any function except close( ) or posix_close( ) without
error.
For all other error situations (except for [EBADF] where fildes was
invalid), fildes shall be closed. If fildes was closed even though
the close operation is incomplete, the close operation shall continue
asynchronously and the process shall have no further ability to track
the completion or final status of the close operation.
------------------------------------------------------------------------

If close returns -1 with errno set to EINTR, filedes must remain open
(so that it is not subject to being re-assigned, in which case you
have dangerous double-close races when looping on EINTR).

> If there is such
> a prohibition, isn't that more likely to be a bug in POSIX-future
> than in GNU/Linux?

No. This whole issue has been discussed to death and the bug is in
Linux (not GNU/Linux, as it's in no way GNU's fault). Glibc is
hopefully going to fix it on the userspace side; I have an open bug
for it but it hasn't gotten a lot of attention:

https://sourceware.org/bugzilla/show_bug.cgi?id=14627

Rich

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

[Paul Eggert]

On 12/05/2016 08:38 AM, Rich Felker wrote:
> Glibc is
> hopefully going to fix it on the userspace side; I have an open bug
> for it

Ah, thanks for clarifying. I now see that I already worked around the 
problem in GNU Emacs but forgot about it afterwards. On platforms 
lacking posix_close, Emacs treats a close error with errno==EINTR as if 
the close had succeeded. Although this leaks a file descriptor on some 
non-Linux systems, the leak is unlikely and is better than closing a 
random victim.