Re: [PATCH v3] Remove signal handling for nanosleep (bug 16364)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 10-11-2015 14:41, Adhemerval Zanella wrote:
> From: Adhemerval Zanella <adhemerval.zanella@linaro.com>
> 
> Linux 2.6.32 and forward do not show the issue regarding SysV SIGCHLD
> vs. SIG_IGN for nanosleep which make it feasible to use it for sleep
> implementation without requiring any hacking to handle the spurious
> wake up.  The issue is likely being fixed before 2.6 and git
> history [1] [2].
> 
> This patch simplifies the sleep code to call nanosleep directly by
> using the posix default version.  It also removes the early cancellation
> tests for zero argument, since nanosleep will handle cancellation
> in this case.
> 
> [1] https://lkml.org/lkml/2004/11/25/5
> [2] https://lkml.org/lkml/2003/11/8/50
> 
> Checked on x86_64, ppc64le, and aarch64.
> 
> 	[BZ #16364]
> 	* sysdeps/unix/sysv/linux/sleep.c: Remove file
> 	* sysdeps/posix/sleep.c (__sleep): Simplify cancellation handling.
> 
> Changes from previous version:
> 
>  - Dropped racy test.
> 

Ping.

> ---
>  sysdeps/posix/sleep.c           |   9 ---
>  sysdeps/unix/sysv/linux/sleep.c | 149 ----------------------------------------
>  3 files changed, 6 insertions(+), 158 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/sleep.c
> 
> diff --git a/sysdeps/posix/sleep.c b/sysdeps/posix/sleep.c
> index 75d9c18..80beb2a 100644
> --- a/sysdeps/posix/sleep.c
> +++ b/sysdeps/posix/sleep.c
> @@ -32,15 +32,6 @@
>  unsigned int
>  __sleep (unsigned int seconds)
>  {
> -  /* This is not necessary but some buggy programs depend on it.  */
> -  if (__glibc_unlikely (seconds == 0))
> -    {
> -#ifdef CANCELLATION_P
> -      CANCELLATION_P (THREAD_SELF);
> -#endif
> -      return 0;
> -    }
> -
>    int save_errno = errno;
>  
>    const unsigned int max
> diff --git a/sysdeps/unix/sysv/linux/sleep.c b/sysdeps/unix/sysv/linux/sleep.c
> deleted file mode 100644
> index 2a06d5a..0000000
> --- a/sysdeps/unix/sysv/linux/sleep.c
> +++ /dev/null
> @@ -1,149 +0,0 @@
> -/* Implementation of the POSIX sleep function using nanosleep.
> -   Copyright (C) 1996-2015 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -   Contributed by Ulrich Drepper <drepper@cygnus.com>, 1996.
> -
> -   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 <time.h>
> -#include <signal.h>
> -#include <string.h>	/* For the real memset prototype.  */
> -#include <unistd.h>
> -#include <sys/param.h>
> -#include <nptl/pthreadP.h>
> -
> -
> -#if 0
> -static void
> -cl (void *arg)
> -{
> -  (void) __sigprocmask (SIG_SETMASK, arg, (sigset_t *) NULL);
> -}
> -#endif
> -
> -
> -/* We are going to use the `nanosleep' syscall of the kernel.  But the
> -   kernel does not implement the stupid SysV SIGCHLD vs. SIG_IGN
> -   behaviour for this syscall.  Therefore we have to emulate it here.  */
> -unsigned int
> -__sleep (unsigned int seconds)
> -{
> -  const unsigned int max
> -    = (unsigned int) (((unsigned long int) (~((time_t) 0))) >> 1);
> -  struct timespec ts;
> -  sigset_t set, oset;
> -  unsigned int result;
> -
> -  /* This is not necessary but some buggy programs depend on this.  */
> -  if (__glibc_unlikely (seconds == 0))
> -    {
> -#ifdef CANCELLATION_P
> -      CANCELLATION_P (THREAD_SELF);
> -#endif
> -      return 0;
> -    }
> -
> -  ts.tv_sec = 0;
> -  ts.tv_nsec = 0;
> - again:
> -  if (sizeof (ts.tv_sec) <= sizeof (seconds))
> -    {
> -      /* Since SECONDS is unsigned assigning the value to .tv_sec can
> -	 overflow it.  In this case we have to wait in steps.  */
> -      ts.tv_sec += MIN (seconds, max);
> -      seconds -= (unsigned int) ts.tv_sec;
> -    }
> -  else
> -    {
> -      ts.tv_sec = (time_t) seconds;
> -      seconds = 0;
> -    }
> -
> -  /* Linux will wake up the system call, nanosleep, when SIGCHLD
> -     arrives even if SIGCHLD is ignored.  We have to deal with it
> -     in libc.  We block SIGCHLD first.  */
> -  __sigemptyset (&set);
> -  __sigaddset (&set, SIGCHLD);
> -  if (__sigprocmask (SIG_BLOCK, &set, &oset))
> -    return -1;
> -
> -  /* If SIGCHLD is already blocked, we don't have to do anything.  */
> -  if (!__sigismember (&oset, SIGCHLD))
> -    {
> -      int saved_errno;
> -      struct sigaction oact;
> -
> -      __sigemptyset (&set);
> -      __sigaddset (&set, SIGCHLD);
> -
> -      /* We get the signal handler for SIGCHLD.  */
> -      if (__sigaction (SIGCHLD, (struct sigaction *) NULL, &oact) < 0)
> -	{
> -	  saved_errno = errno;
> -	  /* Restore the original signal mask.  */
> -	  (void) __sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL);
> -	  __set_errno (saved_errno);
> -	  return -1;
> -	}
> -
> -      /* Note the sleep() is a cancellation point.  But since we call
> -	 nanosleep() which itself is a cancellation point we do not
> -	 have to do anything here.  */
> -      if (oact.sa_handler == SIG_IGN)
> -	{
> -	  //__libc_cleanup_push (cl, &oset);
> -
> -	  /* We should leave SIGCHLD blocked.  */
> -	  while (1)
> -	    {
> -	      result = __nanosleep (&ts, &ts);
> -
> -	      if (result != 0 || seconds == 0)
> -		break;
> -
> -	      if (sizeof (ts.tv_sec) <= sizeof (seconds))
> -		{
> -		  ts.tv_sec = MIN (seconds, max);
> -		  seconds -= (unsigned int) ts.tv_nsec;
> -		}
> -	    }
> -
> -	  //__libc_cleanup_pop (0);
> -
> -	  saved_errno = errno;
> -	  /* Restore the original signal mask.  */
> -	  (void) __sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL);
> -	  __set_errno (saved_errno);
> -
> -	  goto out;
> -	}
> -
> -      /* We should unblock SIGCHLD.  Restore the original signal mask.  */
> -      (void) __sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL);
> -    }
> -
> -  result = __nanosleep (&ts, &ts);
> -  if (result == 0 && seconds != 0)
> -    goto again;
> -
> - out:
> -  if (result != 0)
> -    /* Round remaining time.  */
> -    result = seconds + (unsigned int) ts.tv_sec + (ts.tv_nsec >= 500000000L);
> -
> -  return result;
> -}
> -weak_alias (__sleep, sleep)
>