diff mbox

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

Message ID 1447173662-5776-1-git-send-email-adhemerval.zanella@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto Nov. 10, 2015, 4:41 p.m. UTC
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.

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

Comments

Adhemerval Zanella Netto Nov. 17, 2015, 2:09 p.m. UTC | #1
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)
>
diff mbox

Patch

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)