Re: posix/tst-waitid.c possible race condition on Linux

[Olivier Langlois <olivier at trillion01 dot com>]

On Wed, 2013-06-12 at 19:12 -0700, Roland McGrath wrote:
> > Wouldn't it make more sense to use sigtimedwait or sigwaitinfo to get
> > the signal? This would avoid the race condition without adding a long
> > sleep.
> 
> Yeah, the test could probably be changed to use sigwait instead of delays.
> Feel free to work it out and send a patch.


Here is my attempt:


2013-06-14  Olivier Langlois  <olivier@trillion01.com>

       * posix/tst-waitid.c (do_test): Improve Roland McGrath change by
       replacing some sleep() calls with sigtimedwait() calls making the
       test faster.

--- glibc-2.17/posix/tst-waitid.c.orig	2013-06-14 00:28:01.675669424 -0400
+++ glibc-2.17/posix/tst-waitid.c	2013-06-14 00:55:53.165450947 -0400
@@ -28,10 +28,6 @@
 static void
 test_child (void)
 {
-  /* Wait a second to be sure the parent set his variables before we
-     produce a SIGCHLD.  */
-  sleep (1);
-
   /* First thing, we stop ourselves.  */
   raise (SIGSTOP);
 
@@ -122,6 +118,16 @@ do_test (int argc, char *argv[])
       printf ("setting SIGCHLD handler: %m\n");
       return EXIT_FAILURE;
     }
+
+  sigset_t chldmask, oldmask;
+  sigemptyset (&chldmask);
+  sigaddset (&chldmask, SIGCHLD);
+
+  if (sigprocmask (SIG_BLOCK, &chldmask, &oldmask) < 0)
+    {
+      printf ("blocking SIGCHLD: %m\n");
+      return EXIT_FAILURE;
+    }
 #endif
 
   expecting_sigchld = 1;
@@ -139,16 +145,41 @@ do_test (int argc, char *argv[])
     }
 
   int status = EXIT_SUCCESS;
+  siginfo_t info;
 #define RETURN(ok) \
     do { if (status == EXIT_SUCCESS) status = (ok); goto out; } while (0)
 
   /* Give the child a chance to stop.  */
-  sleep (3);
+#ifdef SA_SIGINFO
+  struct timespec rqt;
+  rqt.tv_sec = 3;
+  rqt.tv_nsec = 0;
+
+  info.si_signo = 0;            /* A successful call sets it to SIGCHLD.  */
+  info.si_pid = -1;
+  info.si_status = -1;
+  int sign = sigtimedwait (&chldmask, &info, &rqt);
+  if (sign < 0)
+    {
+      printf ("wait SIGCHLD: %m\n");
+      RETURN (EXIT_FAILURE);
+    }
+  else
+    {
+      sigchld (sign, &info, NULL);
+    }
 
-  CHECK_SIGCHLD ("stopped", CLD_STOPPED, SIGSTOP);
+  if (sigprocmask (SIG_SETMASK, &oldmask, NULL) < 0)
+    {
+      printf ("setting oldmask: %m\n");
+      RETURN (EXIT_FAILURE);
+    }
+#else
+  sleep (3);
+#endif
+  CHECK_SIGCHLD ("stopped (before waitid)", CLD_STOPPED, SIGSTOP);
 
   /* Now try a wait that should not succeed.  */
-  siginfo_t info;
   info.si_signo = 0;		/* A successful call sets it to SIGCHLD.  */
   int fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
   switch (fail)
@@ -227,7 +258,7 @@ do_test (int argc, char *argv[])
       expecting_sigchld = 0;
     }
   else
-    CHECK_SIGCHLD ("continued", CLD_CONTINUED, SIGCONT);
+    CHECK_SIGCHLD ("continued (before waitid)", CLD_CONTINUED, SIGCONT);
 
   info.si_signo = 0;		/* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
@@ -330,12 +361,46 @@ do_test (int argc, char *argv[])
     }
 
   /* Now stop him again and test waitpid with WCONTINUED.  */
+#ifdef SA_SIGINFO
+  if (sigprocmask (SIG_BLOCK, &chldmask, &oldmask) < 0)
+    {
+      printf ("blocking SIGCHLD: %m\n");
+      RETURN (EXIT_FAILURE);
+    }
+#endif
   expecting_sigchld = 1;
   if (kill (pid, SIGSTOP) != 0)
     {
       printf ("kill (%d, SIGSTOP): %m\n", pid);
       RETURN (EXIT_FAILURE);
     }
+
+#ifdef SA_SIGINFO
+  /* Give the child a chance to stop.  */
+  rqt.tv_sec = 3;
+  rqt.tv_nsec = 0;
+
+  info.si_signo = 0;            /* A successful call sets it to SIGCHLD.  */
+  info.si_pid = -1;
+  info.si_status = -1;
+  sign = sigtimedwait (&chldmask, &info, &rqt);
+  if (sign < 0)
+    {
+      printf ("wait SIGCHLD: %m\n");
+      RETURN (EXIT_FAILURE);
+    }
+  else
+    {
+      sigchld (sign, &info, NULL);
+    }
+
+  if (sigprocmask (SIG_SETMASK, &oldmask, NULL) < 0)
+    {
+      printf ("setting oldmask: %m\n");
+      RETURN (EXIT_FAILURE);
+    }
+#endif
+
   pid_t wpid = waitpid (pid, &fail, WUNTRACED);
   if (wpid < 0)
     {
@@ -354,7 +419,7 @@ do_test (int argc, char *argv[])
       printf ("waitpid WUNTRACED on stopped: status %x\n", fail);
       RETURN (EXIT_FAILURE);
     }
-  CHECK_SIGCHLD ("stopped", CLD_STOPPED, SIGSTOP);
+  CHECK_SIGCHLD ("stopped (after waitpid)", CLD_STOPPED, SIGSTOP);
 
   expecting_sigchld = 1;
   if (kill (pid, SIGCONT) != 0)
@@ -372,7 +437,7 @@ do_test (int argc, char *argv[])
       expecting_sigchld = 0;
     }
   else
-    CHECK_SIGCHLD ("continued", CLD_CONTINUED, SIGCONT);
+    CHECK_SIGCHLD ("continued (before waitpid)", CLD_CONTINUED, SIGCONT);
 
   wpid = waitpid (pid, &fail, WCONTINUED);
   if (wpid < 0)