Re: [RFC][BZ #14627] Make linux close errno to EINPROGRESS when interrupted in signal.

[Rich Felker <dalias at aerifal dot cx>]

On Thu, Dec 05, 2013 at 03:24:05PM -0200, Adhemerval Zanella wrote:
> I wonder if it would break applications that relies on the Linux behavior.

There are several ways in which an application could "rely on the
Linux behavior"; the two that come to mind are:

1. Assuming that when close fails with EINTR, the file descriptor has
   already been successfully closed. Per the Austin Group
   interpretation, this application behavior is now wrong, but it
   likely persists in many places. Fortunately, the proposed change
   cannot break such applications since, with the change, close will
   NEVER give a result of EINTR.

2. Assuming that errors other than EINTR are "write errors" or
   otherwise treating them as fatal. This is more problematic, as the
   proposed change could actually break such applications. An
   alternative solution which would avoid the problem is simply
   returning 0 (success) when the kernel returns EINTR, which seems to
   be a conforming option (there are no POSIX-required side effects of
   close which have not already taken place at the time EINTR is
   returned) but I'm not sure it would be desirable.

Note that, for the most part, close on Linux does not return EINTR.
The only way it can happen is with certain devices (for testing this
issue, I had someone helping me test it write a kernel module for a
fake device that always blocks in close and returns EINTR when
interrupted) and perhaps custom filesystems. So a change here will not
impact most applications.

Rich



> 
> On 05-12-2013 15:10, OndÅej BÃlka wrote:
> > Hi,
> >
> > As was recently said
> > https://www.sourceware.org/ml/libc-alpha/2013-12/msg00086.html
> >
> > linux when close is interrupted by signal closes file descriptor but
> > returns EINTR errno which lies application about restartability. As
> > kernel does not restart close on SA_RESTART only thing needed is to
> > return EINPROGRESS instead EINTR as specified by POSIX.
> >
> > I do not have actual implementation ready as close is automatically
> > generated syscall wrapper.
> >
> > Second concern is cancelability, there is a close_nocancel macro and
> > also in libc.so close is implemented as header before close_nocancel but
> > i couldn't find definition.
> >
> > Could somebody clarify?
> >
> > As actual implementation one could use something like this. As in fclose
> > we use noncancelable version should we also disable cancelability in
> > close?
> >
> > diff --git a/sysdeps/unix/sysv/linux/close.c b/sysdeps/unix/sysv/linux/close.c
> > new file mode 100644
> > index 0000000..223d6ed
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/close.c
> > @@ -0,0 +1,44 @@
> > +/* Copyright (C) 2013 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <http://www.gnu.org/licenses/>.  */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <sysdep-cancel.h>
> > +
> > +int
> > +__libc_close (int fd)
> > +{
> > +  if (SINGLE_THREAD_P)
> > +    return INLINE_SYSCALL (close, 1, fd);
> > +
> > +  int oldtype = LIBC_CANCEL_ASYNC ();
> > +
> > +  int result = INLINE_SYSCALL (close, 1, fd);
> > +
> > +  if (errno == EINTR)
> > +    errno = EINPROGRESS;
> > +
> > +  LIBC_CANCEL_RESET (oldtype);
> > +
> > +  return result;
> > +}
> > +
> > +weak_alias (__libc_close, __close)
> > +libc_hidden_weak (__close)
> > +weak_alias (__libc_close, close)
> >