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

[Mark Mentovai <mark at chromium dot org>]

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