]> Zhao Yanbai Git Server - minix.git/commitdiff
PM: rework signal handling
authorDavid van Moolenbroek <david@minix3.org>
Sun, 27 Oct 2013 12:47:45 +0000 (13:47 +0100)
committerLionel Sambuc <lionel@minix3.org>
Sat, 1 Mar 2014 08:04:59 +0000 (09:04 +0100)
- introduce PROC_STOPPED flag, which tracks whether the process is
  stopped on PROC_STOP in the kernel, rather than implicitly deriving
  this from PM_SIG_PENDING;
- make the process resumption test based on current state rather than
  state transitions;
- add and clarify several flag checks in the signal handling code;
- add test79 to test signal handling robustness.

Change-Id: Ic8c7527095035b300b56f2ab1b9dd190bd4bf001

12 files changed:
distrib/sets/lists/minix/mi
servers/is/dmp_pm.c
servers/pm/forkexit.c
servers/pm/main.c
servers/pm/mproc.h
servers/pm/proto.h
servers/pm/signal.c
servers/pm/trace.c
servers/procfs/pid.c
test/Makefile
test/run
test/test79.c [new file with mode: 0644]

index fd3c9a12201b98fd2a263917bb955d09f46713d3..dc49573327dfa7773d933e76aeeb9b3d944a5444 100644 (file)
 ./usr/tests/minix-posix/test76         minix-sys
 ./usr/tests/minix-posix/test77         minix-sys
 ./usr/tests/minix-posix/test78         minix-sys
+./usr/tests/minix-posix/test79         minix-sys
 ./usr/tests/minix-posix/test8          minix-sys
 ./usr/tests/minix-posix/test9          minix-sys
 ./usr/tests/minix-posix/testinterp     minix-sys
index bf55f1e000346c6f5368498fc75f2b81d9fcdbec..e6a46eaa5778654117aecfec38dd4ece6caae39d 100644 (file)
@@ -25,11 +25,11 @@ static char *flags_str(int flags)
        str[1] = (flags & ZOMBIE)  ? 'Z' : '-';
        str[2] = (flags & ALARM_ON)  ? 'A' : '-';
        str[3] = (flags & EXITING) ? 'E' : '-';
-       str[4] = (flags & STOPPED)  ? 'S' : '-';
+       str[4] = (flags & TRACE_STOPPED)  ? 'T' : '-';
        str[5] = (flags & SIGSUSPENDED)  ? 'U' : '-';
        str[6] = (flags & REPLY)  ? 'R' : '-';
        str[7] = (flags & VFS_CALL) ? 'F' : '-';
-       str[8] = (flags & PM_SIG_PENDING) ? 's' : '-';
+       str[8] = (flags & PROC_STOPPED) ? 's' : '-';
        str[9] = (flags & PRIV_PROC)  ? 'p' : '-';
        str[10] = (flags & PARTIAL_EXEC) ? 'x' : '-';
        str[11] = (flags & DELAY_CALL) ? 'd' : '-';
index eafd37b6c71fcfa0c5e60a4806c422db37910260..68ef6df3c62721831b273badb0a5aaa491ae13e3 100644 (file)
@@ -296,8 +296,15 @@ int dump_core;                     /* flag indicating whether to dump core */
    * This order is important so that VFS can tell drivers to cancel requests
    * such as copying to/ from the exiting process, before it is gone.
    */
-  if ((r = sys_stop(proc_nr_e)) != OK)         /* stop the process */
-       panic("sys_stop failed: %d", r);
+  /* If the process is not yet stopped, we force a stop here. This means that
+   * the process may still have a delay call pending. For this reason, the main
+   * message loop discards requests from exiting processes.
+   */
+  if (!(rmp->mp_flags & PROC_STOPPED)) {
+       if ((r = sys_stop(proc_nr_e)) != OK)            /* stop the process */
+               panic("sys_stop failed: %d", r);
+       rmp->mp_flags |= PROC_STOPPED;
+  }
 
   if((r=vm_willexit(proc_nr_e)) != OK) {
        panic("exit_proc: vm_willexit failed: %d", r);
@@ -337,7 +344,7 @@ int dump_core;                      /* flag indicating whether to dump core */
   /* Clean up most of the flags describing the process's state before the exit,
    * and mark it as exiting.
    */
-  rmp->mp_flags &= (IN_USE|VFS_CALL|PRIV_PROC|TRACE_EXIT);
+  rmp->mp_flags &= (IN_USE|VFS_CALL|PRIV_PROC|TRACE_EXIT|PROC_STOPPED);
   rmp->mp_flags |= EXITING;
 
   /* Keep the process around until VFS is finished with it. */
@@ -475,7 +482,7 @@ int do_waitpid()
                        check_parent(rp, TRUE /*try_cleanup*/);
                        return(SUSPEND);
                }
-               if (rp->mp_flags & STOPPED) {
+               if (rp->mp_flags & TRACE_STOPPED) {
                        /* This child meets the pid test and is being traced.
                         * Deliver a signal to the tracer, if any.
                         */
index abfc1360ab4a5d188c25289ee7a0aabdb874f7ba..96d94787b2040f609c6efe4967c7db68f6cd9ce5 100644 (file)
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <machine/archtypes.h>
 #include <env.h>
+#include <assert.h>
 #include "mproc.h"
 #include "param.h"
 
@@ -484,6 +485,11 @@ static void handle_vfs_reply()
        break;
 
   case PM_UNPAUSE_REPLY:
+       /* The target process must always be stopped while unpausing; otherwise
+        * it could just end up pausing itself on a new call afterwards.
+        */
+       assert(rmp->mp_flags & PROC_STOPPED);
+
        /* Process is now unpaused */
        rmp->mp_flags |= UNPAUSED;
 
index b6d010fe93c6e096b4452caf9e2d5e3dce9ced44..26f5dc1c71b3fa93a4095298449fb8df0b25bcf9 100644 (file)
@@ -76,15 +76,15 @@ EXTERN struct mproc {
 #define IN_USE         0x00001 /* set when 'mproc' slot in use */
 #define WAITING                0x00002 /* set by WAIT system call */
 #define ZOMBIE         0x00004 /* waiting for parent to issue WAIT call */
+#define PROC_STOPPED   0x00008 /* process is stopped in the kernel */
 #define ALARM_ON       0x00010 /* set when SIGALRM timer started */
 #define EXITING                0x00020 /* set by EXIT, process is now exiting */
 #define TOLD_PARENT    0x00040 /* parent wait() completed, ZOMBIE off */
-#define STOPPED                0x00080 /* set if process stopped for tracing */
+#define TRACE_STOPPED  0x00080 /* set if process stopped for tracing */
 #define SIGSUSPENDED   0x00100 /* set by SIGSUSPEND system call */
 #define REPLY          0x00200 /* set if a reply message is pending */
 #define VFS_CALL               0x00400 /* set if waiting for VFS (normal calls) */
-#define PM_SIG_PENDING 0x00800 /* process got a signal while waiting for VFS */
-#define UNPAUSED       0x01000 /* process is not in a blocking call */
+#define UNPAUSED       0x01000 /* VFS has replied to unpause request */
 #define PRIV_PROC      0x02000 /* system process, special privileges */
 #define PARTIAL_EXEC   0x04000 /* process got a new map but no content */
 #define TRACE_EXIT     0x08000 /* tracer is forcing this process to exit */
index bf38049f6a0d4e88019cadf001a71b3b361ba894..c1840aa1de042e87caeff1c773ada4fc057aa767 100644 (file)
@@ -87,7 +87,7 @@ int do_settime(void);
 
 /* trace.c */
 int do_trace(void);
-void stop_proc(struct mproc *rmp, int sig_nr);
+void trace_stop(struct mproc *rmp, int sig_nr);
 
 /* utility.c */
 pid_t get_free_pid(void);
index 7771573b68ba6557684b05d3a3051797dcd9f64b..d3d7ed145b7a1331b184116684ae48fa6b8d3862 100644 (file)
 #include <signal.h>
 #include <sys/resource.h>
 #include <string.h>
+#include <assert.h>
 #include "mproc.h"
 #include "param.h"
 
-static void unpause(struct mproc *rmp);
+static int unpause(struct mproc *rmp);
 static int sig_send(struct mproc *rmp, int signo);
 static void sig_proc_exit(struct mproc *rmp, int signo);
 
 /*===========================================================================*
  *                             do_sigaction                                 *
  *===========================================================================*/
-int do_sigaction()
+int do_sigaction(void)
 {
   int r;
   struct sigaction svec;
   struct sigaction *svp;
 
+  assert(!(mp->mp_flags & (PROC_STOPPED | VFS_CALL | UNPAUSED)));
+
   if (m_in.sig_nr == SIGKILL) return(OK);
   if (m_in.sig_nr < 1 || m_in.sig_nr >= _NSIG) return(EINVAL);
 
@@ -86,8 +89,10 @@ int do_sigaction()
 /*===========================================================================*
  *                             do_sigpending                                *
  *===========================================================================*/
-int do_sigpending()
+int do_sigpending(void)
 {
+  assert(!(mp->mp_flags & (PROC_STOPPED | VFS_CALL | UNPAUSED)));
+
   mp->mp_reply.reply_mask = (long) mp->mp_sigpending;
   return OK;
 }
@@ -95,7 +100,7 @@ int do_sigpending()
 /*===========================================================================*
  *                             do_sigprocmask                               *
  *===========================================================================*/
-int do_sigprocmask()
+int do_sigprocmask(void)
 {
 /* Note that the library interface passes the actual mask in sigmask_set,
  * not a pointer to the mask, in order to save a copy.  Similarly,
@@ -107,9 +112,10 @@ int do_sigprocmask()
  *
  * KILL and STOP can't be masked.
  */
-
   int i;
 
+  assert(!(mp->mp_flags & (PROC_STOPPED | VFS_CALL | UNPAUSED)));
+
   mp->mp_reply.reply_mask = (long) mp->mp_sigmask;
 
   switch (m_in.sig_how) {
@@ -150,8 +156,10 @@ int do_sigprocmask()
 /*===========================================================================*
  *                             do_sigsuspend                                *
  *===========================================================================*/
-int do_sigsuspend()
+int do_sigsuspend(void)
 {
+  assert(!(mp->mp_flags & (PROC_STOPPED | VFS_CALL | UNPAUSED)));
+
   mp->mp_sigmask2 = mp->mp_sigmask;    /* save the old mask */
   mp->mp_sigmask = (sigset_t) m_in.sig_set;
   sigdelset(&mp->mp_sigmask, SIGKILL);
@@ -164,14 +172,15 @@ int do_sigsuspend()
 /*===========================================================================*
  *                             do_sigreturn                                 *
  *===========================================================================*/
-int do_sigreturn()
+int do_sigreturn(void)
 {
 /* A user signal handler is done.  Restore context and check for
  * pending unblocked signals.
  */
-
   int r;
 
+  assert(!(mp->mp_flags & (PROC_STOPPED | VFS_CALL | UNPAUSED)));
+
   mp->mp_sigmask = (sigset_t) m_in.sig_set;
   sigdelset(&mp->mp_sigmask, SIGKILL);
   sigdelset(&mp->mp_sigmask, SIGSTOP);
@@ -184,7 +193,7 @@ int do_sigreturn()
 /*===========================================================================*
  *                             do_kill                                      *
  *===========================================================================*/
-int do_kill()
+int do_kill(void)
 {
 /* Perform the kill(pid, signo) system call. */
 
@@ -194,7 +203,7 @@ int do_kill()
 /*===========================================================================*
  *                           do_srv_kill                                    *
  *===========================================================================*/
-int do_srv_kill()
+int do_srv_kill(void)
 {
 /* Perform the srv_kill(pid, signo) system call. */
 
@@ -209,6 +218,72 @@ int do_srv_kill()
   return check_sig(m_in.pid, m_in.sig_nr, TRUE /* ksig */);
 }
 
+/*===========================================================================*
+ *                             stop_proc                                    *
+ *===========================================================================*/
+static int stop_proc(struct mproc *rmp, int may_delay)
+{
+/* Try to stop the given process in the kernel. If successful, mark the process
+ * as stopped and return TRUE.  If the process is still busy sending a message,
+ * the behavior depends on the 'may_delay' parameter. If set, the process will
+ * be marked as having a delay call pending, and the function returns FALSE. If
+ * not set, the caller already knows that the process has no delay call, and PM
+ * will panic.
+ */
+  int r;
+
+  assert(!(rmp->mp_flags & (PROC_STOPPED | DELAY_CALL | UNPAUSED)));
+
+  r = sys_delay_stop(rmp->mp_endpoint);
+
+  /* If the process is still busy sending a message, the kernel will give us
+   * EBUSY now and send a SIGSNDELAY to the process as soon as sending is done.
+   */
+  switch (r) {
+  case OK:
+       rmp->mp_flags |= PROC_STOPPED;
+
+       return TRUE;
+
+  case EBUSY:
+       if (!may_delay)
+               panic("stop_proc: unexpected delay call");
+
+       rmp->mp_flags |= DELAY_CALL;
+
+       return FALSE;
+
+  default:
+       panic("sys_delay_stop failed: %d", r);
+  }
+}
+
+/*===========================================================================*
+ *                             try_resume_proc                              *
+ *===========================================================================*/
+static void try_resume_proc(struct mproc *rmp)
+{
+/* Resume the given process if possible. */
+  int r;
+
+  assert(rmp->mp_flags & PROC_STOPPED);
+
+  /* If the process is blocked on a VFS call, do not resume it now. Most likely    * it will be unpausing, in which case the process must remain stopped.
+   * Otherwise, it will still be resumed once the VFS call returns. If the
+   * process has died, do not resume it either.
+   */
+  if (rmp->mp_flags & (VFS_CALL | EXITING))
+       return;
+
+  if ((r = sys_resume(rmp->mp_endpoint)) != OK)
+       panic("sys_resume failed: %d", r);
+
+  /* Also unset the unpaused flag. We can safely assume that a stopped process
+   * need only be unpaused once, but once it is resumed, all bets are off.
+   */
+  rmp->mp_flags &= ~(PROC_STOPPED | UNPAUSED);
+}
+
 /*===========================================================================*
  *                             process_ksig                                 *
  *===========================================================================*/
@@ -261,23 +336,30 @@ int process_ksig(endpoint_t proc_nr_e, int signo)
    * signal settings. The process may also have forked, exited etcetera.
    */
   if (signo == SIGSNDELAY && (rmp->mp_flags & DELAY_CALL)) {
+       /* When getting SIGSNDELAY, the process is stopped at least until the
+        * receipt of the SIGSNDELAY signal is acknowledged to the kernel. The
+        * process is not stopped on PROC_STOP in the kernel. However, now that
+        * there is no longer a delay call, stop_proc() is guaranteed to
+        * succeed immediately.
+        */
        rmp->mp_flags &= ~DELAY_CALL;
 
-       /*
-        * If the VFS_CALL flag is still set we have a process which is stopped
-        * and we only need to wait for a reply from VFS. We are going to check
-        * the pending signal then
+       assert(!(rmp->mp_flags & PROC_STOPPED));
+
+       /* If the delay call was to PM, it may have resulted in a VFS call. In
+        * that case, we must wait with further signal processing until VFS has
+        * replied. Stop the process.
         */
-       if (rmp->mp_flags & VFS_CALL)
+       if (rmp->mp_flags & VFS_CALL) {
+               stop_proc(rmp, FALSE /*may_delay*/);
+
                return OK;
-       if (rmp->mp_flags & PM_SIG_PENDING)
-               panic("process_ksig: bad process state");
+       }
 
        /* Process as many normal signals as possible. */
        check_pending(rmp);
 
-       if (rmp->mp_flags & DELAY_CALL)
-               panic("process_ksig: multiple delay calls?");
+       assert(!(rmp->mp_flags & DELAY_CALL));
   }
   
   /* See if the process is still alive */
@@ -311,7 +393,7 @@ int ksig;                   /* non-zero means signal comes from kernel  */
  * context from the sigcontext structure.
  * If there is insufficient stack space, kill the process.
  */
-  int r, slot, badignore;
+  int slot, badignore;
 
   slot = (int) (rmp - mproc);
   if ((rmp->mp_flags & (IN_USE | EXITING)) != IN_USE) {
@@ -326,8 +408,8 @@ int ksig;                   /* non-zero means signal comes from kernel  */
 
        sigaddset(&rmp->mp_sigtrace, signo);
 
-       if (!(rmp->mp_flags & STOPPED))
-               stop_proc(rmp, signo);  /* a signal causes it to stop */
+       if (!(rmp->mp_flags & TRACE_STOPPED))
+               trace_stop(rmp, signo); /* a signal causes it to stop */
 
        return;
   }
@@ -337,14 +419,20 @@ int ksig;                 /* non-zero means signal comes from kernel  */
        if(ksig)
                sigaddset(&rmp->mp_ksigpending, signo);
 
-       if (!(rmp->mp_flags & PM_SIG_PENDING)) {
-               /* No delay calls: VFS_CALL implies the process called us. */
-               if ((r = sys_stop(rmp->mp_endpoint)) != OK)
-                       panic("sys_stop failed: %d", r);
-
-               rmp->mp_flags |= PM_SIG_PENDING;
+       /* Process the signal once VFS replies. Stop the process in the
+        * meantime, so that it cannot make another call after the VFS reply
+        * comes in but before we look at its signals again. Since we always
+        * stop the process to deliver signals during a VFS call, the
+        * PROC_STOPPED flag doubles as an indicator in restart_sigs() that
+        * signals must be rechecked after a VFS reply comes in.
+        */
+       if (!(rmp->mp_flags & (PROC_STOPPED | DELAY_CALL))) {
+               /* If a VFS call is ongoing and the process is not yet stopped,
+                * the process must have made a call to PM. Therefore, there
+                * can be no delay calls in this case.
+                */
+               stop_proc(rmp, FALSE /*delay_call*/);
        }
-
        return;
   }
 
@@ -400,7 +488,7 @@ int ksig;                   /* non-zero means signal comes from kernel  */
        return;
   }
 
-  if ((rmp->mp_flags & STOPPED) && signo != SIGKILL) {
+  if ((rmp->mp_flags & TRACE_STOPPED) && signo != SIGKILL) {
        /* If the process is stopped for a debugger, do not deliver any signals
         * (except SIGKILL) in order not to confuse the debugger. The signals
         * will be delivered using the check_pending() calls in do_trace().
@@ -415,17 +503,13 @@ int ksig;                 /* non-zero means signal comes from kernel  */
         * applicable. This may involve a roundtrip to VFS, in which case we'll
         * have to check back later.
         */
-       if (!(rmp->mp_flags & UNPAUSED)) {
-               unpause(rmp);
-
-               if (!(rmp->mp_flags & UNPAUSED)) {
-                       /* not yet unpaused; continue later */
-                       sigaddset(&rmp->mp_sigpending, signo);
-                       if(ksig)
-                               sigaddset(&rmp->mp_ksigpending, signo);
+       if (!unpause(rmp)) {
+               /* not yet unpaused; continue later */
+               sigaddset(&rmp->mp_sigpending, signo);
+               if(ksig)
+                       sigaddset(&rmp->mp_ksigpending, signo);
 
-                       return;
-               }
+               return;
        }
 
        /* Then send the actual signal to the process, by setting up a signal
@@ -564,7 +648,6 @@ register struct mproc *rmp;
    * changed.  At each such place, check_pending() should be called to
    * check for newly unblocked signals.
    */
-
   int i;
   int ksig;
 
@@ -576,8 +659,14 @@ register struct mproc *rmp;
                sigdelset(&rmp->mp_ksigpending, i);
                sig_proc(rmp, i, FALSE /*trace*/, ksig);
 
-               if (rmp->mp_flags & VFS_CALL)
+               if (rmp->mp_flags & VFS_CALL) {
+                       /* Signals must be rechecked upon return from the new
+                        * VFS call, unless the process was killed. In both
+                        * cases, the process is stopped.
+                        */
+                       assert(rmp->mp_flags & PROC_STOPPED);
                        break;
+               }
        }
   }
 }
@@ -590,7 +679,6 @@ struct mproc *rmp;
 {
 /* VFS has replied to a request from us; do signal-related work.
  */
-  int r;
 
   if (rmp->mp_flags & (VFS_CALL | EXITING)) return;
 
@@ -598,29 +686,27 @@ struct mproc *rmp;
        /* Tracer requested exit with specific exit value */
        exit_proc(rmp, rmp->mp_exitstatus, FALSE /*dump_core*/);
   }
-  else if (rmp->mp_flags & PM_SIG_PENDING) {
-       /* We saved signal(s) for after finishing a VFS call. Deal with this.
-        * PM_SIG_PENDING remains set to indicate the process is still stopped.
+  else if (rmp->mp_flags & PROC_STOPPED) {
+       /* If a signal arrives while we are performing a VFS call, the process
+        * will always be stopped immediately. Thus, if the process is stopped
+        * once the reply from VFS arrives, we might have to check signals.
         */
-       check_pending(rmp);
+       assert(!(rmp->mp_flags & DELAY_CALL));
 
-       /* The process may now be VFS-blocked again, because a signal exited the
-        * process or was caught. Restart the process only when this is NOT the
-        * case.
+       /* We saved signal(s) for after finishing a VFS call. Deal with this.
+        * PROC_STOPPED remains set to indicate the process is still stopped.
         */
-       if (!(rmp->mp_flags & VFS_CALL)) {
-               rmp->mp_flags &= ~(PM_SIG_PENDING | UNPAUSED);
+       check_pending(rmp);
 
-               if ((r = sys_resume(rmp->mp_endpoint)) != OK)
-                       panic("sys_resume failed: %d", r);
-       }
+       /* Resume the process now, unless there is a reason not to. */
+       try_resume_proc(rmp);
   }
 }
 
 /*===========================================================================*
  *                             unpause                                      *
  *===========================================================================*/
-static void unpause(rmp)
+static int unpause(rmp)
 struct mproc *rmp;             /* which process */
 {
 /* A signal is to be sent to a process.  If that process is hanging on a
@@ -629,43 +715,38 @@ struct mproc *rmp;                /* which process */
  * interruptible calls such as READs and WRITEs from pipes, ttys and the like.
  */
   message m;
-  int r;
-
-  /* If we're already waiting for a delayed call, don't do anything now. */
-  if (rmp->mp_flags & DELAY_CALL)
-       return;
 
-  /* Check to see if process is hanging on a WAIT or SIGSUSPEND call. */
-  if (rmp->mp_flags & (WAITING | SIGSUSPENDED)) {
-       /* Stop process from running. No delay calls: it called us. */
-       if ((r = sys_stop(rmp->mp_endpoint)) != OK)
-               panic("sys_stop failed: %d", r);
+  assert(!(rmp->mp_flags & VFS_CALL));
 
-       rmp->mp_flags |= UNPAUSED;
+  /* If the UNPAUSED flag is set, VFS replied to an earlier unpause request. */
+  if (rmp->mp_flags & UNPAUSED) {
+       assert((rmp->mp_flags & (DELAY_CALL | PROC_STOPPED)) == PROC_STOPPED);
 
-       /* We interrupt the actual call from sig_send() below. */
-       return;
+       return TRUE;
   }
 
-  /* Not paused in PM. Let VFS try to unpause the process. */
-  if (!(rmp->mp_flags & PM_SIG_PENDING)) {
-       /* Stop process from running. */
-       r = sys_delay_stop(rmp->mp_endpoint);
+  /* If the process is already stopping, don't do anything now. */
+  if (rmp->mp_flags & DELAY_CALL)
+       return FALSE;
 
-       /* If the process is still busy sending a message, the kernel will give
-        * us EBUSY now and send a SIGSNDELAY to the process as soon as sending
-        * is done.
+  /* Check to see if process is hanging on a WAIT or SIGSUSPEND call. */
+  if (rmp->mp_flags & (WAITING | SIGSUSPENDED)) {
+       /* Stop the process from running. Do not interrupt the actual call yet.
+        * sig_send() will interrupt the call and resume the process afterward.
+        * No delay calls: we know for a fact that the process called us.
         */
-       if (r == EBUSY) {
-               rmp->mp_flags |= DELAY_CALL;
+       stop_proc(rmp, FALSE /*may_delay*/);
 
-               return;
-       }
-       else if (r != OK) panic("sys_stop failed: %d", r);
-
-       rmp->mp_flags |= PM_SIG_PENDING;
+       return TRUE;
   }
 
+  /* Not paused in PM. Let VFS try to unpause the process. The process needs to
+   * be stopped for this. If it is not already stopped, try to stop it now. If
+   * that does not succeed immediately, postpone signal delivery.
+   */
+  if (!(rmp->mp_flags & PROC_STOPPED) && !stop_proc(rmp, TRUE /*may_delay*/))
+       return FALSE;
+
   m.m_type = PM_UNPAUSE;
   m.PM_PROC = rmp->mp_endpoint;
 
@@ -673,6 +754,8 @@ struct mproc *rmp;          /* which process */
 
   /* Also tell VM. */
   vm_notify_sig_wrapper(rmp->mp_endpoint);
+
+  return FALSE;
 }
 
 /*===========================================================================*
@@ -688,8 +771,7 @@ int signo;                  /* signal to send to process (1 to _NSIG-1) */
   struct sigmsg sigmsg;
   int r, sigflags, slot;
 
-  if (!(rmp->mp_flags & UNPAUSED))
-       panic("sig_send: process not unpaused");
+  assert(rmp->mp_flags & PROC_STOPPED);
 
   sigflags = rmp->mp_sigact[signo].sa_flags;
   slot = (int) (rmp - mproc);
@@ -718,8 +800,9 @@ int signo;                  /* signal to send to process (1 to _NSIG-1) */
 
   /* Ask the kernel to deliver the signal */
   r = sys_sigsend(rmp->mp_endpoint, &sigmsg);
-  /* sys_sigsend can fail legitimately with EFAULT or ENOMEM if
-   * the process memory can't accomodate the signal handler.
+  /* sys_sigsend can fail legitimately with EFAULT or ENOMEM if the process
+   * memory can't accommodate the signal handler.  The target process will be
+   * killed in that case, so do not bother interrupting or resuming it.
    */
   if(r == EFAULT || r == ENOMEM) {
        return(FALSE);
@@ -734,14 +817,22 @@ int signo;                        /* signal to send to process (1 to _NSIG-1) */
        rmp->mp_flags &= ~(WAITING | SIGSUSPENDED);
 
        setreply(slot, EINTR);
-  }
 
-  /* Was the process stopped just for this signal? Then resume it. */
-  if ((rmp->mp_flags & (PM_SIG_PENDING | UNPAUSED)) == UNPAUSED) {
-       rmp->mp_flags &= ~UNPAUSED;
+       /* The process must just have been stopped by unpause(), which means
+        * that the UNPAUSE flag is not set.
+        */
+       assert(!(rmp->mp_flags & UNPAUSED));
+
+       try_resume_proc(rmp);
 
-       if ((r = sys_resume(rmp->mp_endpoint)) != OK)
-               panic("sys_resume failed: %d", r);
+       assert(!(rmp->mp_flags & PROC_STOPPED));
+  } else {
+       /* If the process was not suspended in PM, VFS must first have
+        * confirmed that it has tried to unsuspend any blocking call. Thus, we
+        * got here from restart_sigs() as part of handling PM_UNPAUSE_REPLY,
+        * and restart_sigs() will resume the process later.
+        */
+       assert(rmp->mp_flags & UNPAUSED);
   }
 
   return(TRUE);
index 17d4e1d9b80202b62c3d6637c98ffd2ffb8b83b5..e2077422ea70dfe4ccaa15cac19d922fbdfd4004 100644 (file)
@@ -42,7 +42,6 @@ int do_trace()
   register struct mproc *child;
   struct ptrace_range pr;
   int i, r, req;
-  message m;
 
   req = m_in.request;
 
@@ -137,7 +136,7 @@ int do_trace()
   if ((child = find_proc(m_in.pid)) == NULL) return(ESRCH);
   if (child->mp_flags & EXITING) return(ESRCH);
   if (child->mp_tracer != who_p) return(ESRCH);
-  if (!(child->mp_flags & STOPPED)) return(EBUSY);
+  if (!(child->mp_flags & TRACE_STOPPED)) return(EBUSY);
 
   switch (req) {
   case T_EXIT:         /* exit */
@@ -202,7 +201,7 @@ int do_trace()
        }
 
        /* Resume the child as if nothing ever happened. */ 
-       child->mp_flags &= ~STOPPED;
+       child->mp_flags &= ~TRACE_STOPPED;
        child->mp_trace_flags = 0;
 
        check_pending(child);
@@ -229,7 +228,7 @@ int do_trace()
                }
        }
 
-       child->mp_flags &= ~STOPPED;
+       child->mp_flags &= ~TRACE_STOPPED;
 
        check_pending(child);
 
@@ -243,9 +242,9 @@ int do_trace()
 }
 
 /*===========================================================================*
- *                             stop_proc                                    *
+ *                             trace_stop                                   *
  *===========================================================================*/
-void stop_proc(rmp, signo)
+void trace_stop(rmp, signo)
 register struct mproc *rmp;
 int signo;
 {
@@ -257,7 +256,7 @@ int signo;
   r = sys_trace(T_STOP, rmp->mp_endpoint, 0L, (long *) 0);
   if (r != OK) panic("sys_trace failed: %d", r);
  
-  rmp->mp_flags |= STOPPED;
+  rmp->mp_flags |= TRACE_STOPPED;
   if (wait_test(rpmp, rmp)) {
        sigdelset(&rmp->mp_sigtrace, signo);
 
index abb5693fb8d2accc0f1a7088a719dd08d684c463..386c103a8489cfbcc8df87dac4aa0bd9d130459b 100644 (file)
@@ -73,7 +73,7 @@ static void pid_psinfo(int i)
        if (!task) {
                if (is_zombie(i))
                        state = STATE_ZOMBIE;   /* zombie */
-               else if (mproc[pi].mp_flags & STOPPED)
+               else if (mproc[pi].mp_flags & TRACE_STOPPED)
                        state = STATE_STOP;     /* stopped (traced) */
                else if (proc[i].p_rts_flags == 0)
                        state = STATE_RUN;      /* in run-queue */
index 6362356f8de52fd678dc4a25cd0077ff2f05189c..8b5fab77b63f9a48a6f4c137e480e5f76110c62a 100644 (file)
@@ -57,7 +57,7 @@ MINIX_TESTS= \
  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 \
 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 \
 41 42 43 44 45 46    48 49 50    52 53 54 55 56    58 59 60 \
-61       64 65 66 67 68 69 70 71 72 73 74 75 76 77 78
+61       64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79
 
 .if ${MACHINE_ARCH} == "i386"
 MINIX_TESTS+= \
index 72022e4417b6ccc1d9340abf011856e27c514cee..037d42a3f61e8d46e3b916afd1f687abc83ea041 100755 (executable)
--- a/test/run
+++ b/test/run
@@ -28,7 +28,7 @@ rootscripts="testisofs testvnd"
 alltests="1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 \
          21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 \
          41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 \
-         61 62 63 64 65 66 67 68 69 70 71 72       75 76 77 78 \
+         61 62 63 64 65 66 67 68 69 70 71 72       75 76 77 78 79 \
         sh1 sh2 interp mfs isofs vnd"
 tests_no=`expr 0`
 
diff --git a/test/test79.c b/test/test79.c
new file mode 100644 (file)
index 0000000..3c7504e
--- /dev/null
@@ -0,0 +1,497 @@
+/* Tests for PM signal handling robustness - by D.C. van Moolenbroek */
+/*
+ * The signal handling code must not rely on priorities assigned to services,
+ * and so, this test (like any test!) must also pass if PM and/or VFS are not
+ * given a fixed high priority.  A good way to verify this is to let PM and VFS
+ * be scheduled by SCHED rather than KERNEL, and to give them the same priority
+ * as (or slightly lower than) normal user processes.  Note that if VFS is
+ * configured to use a priority *far lower* than user processes, starvation may
+ * cause this test not to complete in some scenarios.  In that case, Ctrl+C
+ * should still be able to kill the test.
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <sys/time.h>
+#include <sys/utsname.h>
+
+#define ITERATIONS 1
+
+#include "common.h"
+
+#define NR_SIGNALS     20000
+
+#define MAX_SIGNALERS  3
+
+static const int signaler_sig[MAX_SIGNALERS] = { SIGUSR1, SIGUSR2, SIGHUP };
+static pid_t signaler_pid[MAX_SIGNALERS];
+static int sig_counter;
+
+enum {
+       JOB_RUN = 0,
+       JOB_CALL_PM,
+       JOB_CALL_VFS,
+       JOB_SET_MASK,
+       JOB_BLOCK_PM,
+       JOB_BLOCK_VFS,
+       JOB_CALL_PM_VFS,
+       NR_JOBS
+};
+
+#define OPT_NEST       0x1
+#define OPT_ALARM      0x2
+#define OPT_ALL                0x3
+
+struct link {
+       pid_t pid;
+       int sndfd;
+       int rcvfd;
+};
+
+/*
+ * Spawn a child process, with a pair of pipes to talk to it bidirectionally.
+ */
+static void
+spawn(struct link *link, void (*proc)(struct link *))
+{
+       int up[2], dn[2];
+
+       fflush(stdout);
+       fflush(stderr);
+
+       if (pipe(up) != 0) e(0);
+       if (pipe(dn) != 0) e(0);
+
+       link->pid = fork();
+
+       switch (link->pid) {
+       case 0:
+               close(up[1]);
+               close(dn[0]);
+
+               link->rcvfd = up[0];
+               link->sndfd = dn[1];
+
+               errct = 0;
+
+               proc(link);
+
+               /* Close our pipe FDs on exit, so that we can make zombies. */
+               exit(errct);
+       case -1:
+               e(0);
+               break;
+       }
+
+       close(up[0]);
+       close(dn[1]);
+
+       link->sndfd = up[1];
+       link->rcvfd = dn[0];
+}
+
+/*
+ * Wait for a child process to terminate, and clean up.
+ */
+static void
+collect(struct link *link)
+{
+       int status;
+
+       close(link->sndfd);
+       close(link->rcvfd);
+
+       if (waitpid(link->pid, &status, 0) <= 0) e(0);
+
+       if (!WIFEXITED(status)) e(0);
+       else errct += WEXITSTATUS(status);
+}
+
+/*
+ * Forcibly terminate a child process, and clean up.
+ */
+static void
+terminate(struct link *link)
+{
+       int status;
+
+       if (kill(link->pid, SIGKILL) != 0) e(0);
+
+       close(link->sndfd);
+       close(link->rcvfd);
+
+       if (waitpid(link->pid, &status, 0) <= 0) e(0);
+
+       if (WIFSIGNALED(status)) {
+               if (WTERMSIG(status) != SIGKILL) e(0);
+       } else {
+               if (!WIFEXITED(status)) e(0);
+               else errct += WEXITSTATUS(status);
+       }
+}
+
+/*
+ * Send an integer value to the child or parent.
+ */
+static void
+snd(struct link *link, int val)
+{
+       if (write(link->sndfd, (void *) &val, sizeof(val)) != sizeof(val))
+               e(0);
+}
+
+/*
+ * Receive an integer value from the child or parent, or -1 on EOF.
+ */
+static int
+rcv(struct link *link)
+{
+       int r, val;
+
+       if ((r = read(link->rcvfd, (void *) &val, sizeof(val))) == 0)
+               return -1;
+
+       if (r != sizeof(val)) e(0);
+
+       return val;
+}
+
+/*
+ * Set a signal handler for a particular signal, blocking either all or no
+ * signals when the signal handler is invoked.
+ */
+static void
+set_handler(int sig, void (*proc)(int), int block)
+{
+       struct sigaction act;
+
+       memset(&act, 0, sizeof(act));
+       if (block) sigfillset(&act.sa_mask);
+       act.sa_handler = proc;
+
+       if (sigaction(sig, &act, NULL) != 0) e(0);
+}
+
+/*
+ * Generic signal handler for the worker process.
+ */
+static void
+worker_handler(int sig)
+{
+       int i;
+
+       switch (sig) {
+       case SIGUSR1:
+       case SIGUSR2:
+       case SIGHUP:
+               for (i = 0; i < MAX_SIGNALERS; i++) {
+                       if (signaler_sig[i] != sig) continue;
+
+                       if (signaler_pid[i] == -1) e(0);
+                       else if (kill(signaler_pid[i], SIGUSR1) != 0) e(0);
+                       break;
+               }
+               if (i == MAX_SIGNALERS) e(0);
+               break;
+       case SIGTERM:
+               exit(errct);
+               break;
+       case SIGALRM:
+               /* Do nothing. */
+               break;
+       default:
+               e(0);
+       }
+}
+
+/*
+ * Procedure for the worker process.  Sets up its own environment using
+ * information sent to it by the parent, sends an acknowledgement to the
+ * parent, and loops executing the job given to it until a SIGTERM comes in.
+ */
+static void __dead
+worker_proc(struct link *parent)
+{
+       struct utsname name;
+       struct itimerval it;
+       struct timeval tv;
+       sigset_t set, oset;
+       uid_t uid;
+       int i, job, options;
+
+       job = rcv(parent);
+       options = rcv(parent);
+
+       for (i = 0; i < MAX_SIGNALERS; i++) {
+               set_handler(signaler_sig[i], worker_handler,
+                   !(options & OPT_NEST));
+
+               signaler_pid[i] = rcv(parent);
+       }
+
+       set_handler(SIGTERM, worker_handler, 1 /* block */);
+       set_handler(SIGALRM, worker_handler, !(options & OPT_NEST));
+
+       snd(parent, 0);
+
+       if (options & OPT_ALARM) {
+               /* The timer would kill wimpy platforms such as ARM. */
+               if (uname(&name) < 0) e(0);
+               if (strcmp(name.machine, "arm")) {
+                       it.it_value.tv_sec = 0;
+                       it.it_value.tv_usec = 1;
+                       it.it_interval.tv_sec = 0;
+                       it.it_interval.tv_usec = 1;
+                       if (setitimer(ITIMER_REAL, &it, NULL) != 0) e(0);
+               }
+       }
+
+       switch (job) {
+       case JOB_RUN:
+               for (;;);
+               break;
+       case JOB_CALL_PM:
+               /*
+                * Part of the complication of the current system in PM comes
+                * from the fact that when a process is being stopped, it might
+                * already have started sending a message.  That message will
+                * arrive at its destination regardless of the process's run
+                * state.  PM must avoid setting up a signal handler (and
+                * changing the process's signal mask as part of that) if such
+                * a message is still in transit, because that message might,
+                * for example, query (or even change) the signal mask.
+                */
+               for (;;) {
+                       if (sigprocmask(SIG_BLOCK, NULL, &set) != 0) e(0);
+                       if (sigismember(&set, SIGUSR1)) e(0);
+               }
+               break;
+       case JOB_CALL_VFS:
+               for (;;) {
+                       tv.tv_sec = 0;
+                       tv.tv_usec = 0;
+                       select(0, NULL, NULL, NULL, &tv);
+               }
+               break;
+       case JOB_SET_MASK:
+               for (;;) {
+                       sigfillset(&set);
+                       if (sigprocmask(SIG_SETMASK, &set, &oset) != 0) e(0);
+                       if (sigprocmask(SIG_SETMASK, &oset, NULL) != 0) e(0);
+               }
+               break;
+       case JOB_BLOCK_PM:
+               for (;;) {
+                       sigemptyset(&set);
+                       sigsuspend(&set);
+               }
+               break;
+       case JOB_BLOCK_VFS:
+               for (;;)
+                       select(0, NULL, NULL, NULL, NULL);
+               break;
+       case JOB_CALL_PM_VFS:
+               uid = getuid();
+               for (;;)
+                       setuid(uid);
+       default:
+               e(0);
+               exit(1);
+       }
+}
+
+/*
+ * Signal handler procedure for the signaler processes, counting the number of
+ * signals received from the worker process.
+ */
+static void
+signaler_handler(int sig)
+{
+       sig_counter++;
+}
+
+/*
+ * Procedure for the signaler processes.  Gets the pid of the worker process
+ * and the signal to use, and then repeatedly sends that signal to the worker
+ * process, waiting for a SIGUSR1 signal back from the worker before
+ * continuing.  This signal ping-pong is repeated for a set number of times.
+ */
+static void
+signaler_proc(struct link *parent)
+{
+       sigset_t set, oset;
+       pid_t pid;
+       int i, sig, nr;
+
+       pid = rcv(parent);
+       sig = rcv(parent);
+       nr = rcv(parent);
+       sig_counter = 0;
+
+       sigfillset(&set);
+       if (sigprocmask(SIG_SETMASK, &set, &oset) != 0) e(0);
+
+       set_handler(SIGUSR1, signaler_handler, 1 /*block*/);
+
+       for (i = 0; nr == 0 || i < nr; i++) {
+               if (sig_counter != i) e(0);
+
+               if (kill(pid, sig) != 0 && nr > 0) e(0);
+
+               sigsuspend(&oset);
+       }
+
+       if (sig_counter != nr) e(0);
+}
+
+/*
+ * Set up the worker and signaler processes, wait for the signaler processes to
+ * do their work and terminate, and then terminate the worker process.
+ */
+static void
+sub79a(int job, int signalers, int options)
+{
+       struct link worker, signaler[MAX_SIGNALERS];
+       int i;
+
+       spawn(&worker, worker_proc);
+
+       snd(&worker, job);
+       snd(&worker, options);
+
+       for (i = 0; i < signalers; i++) {
+               spawn(&signaler[i], signaler_proc);
+
+               snd(&worker, signaler[i].pid);
+       }
+       for (; i < MAX_SIGNALERS; i++)
+               snd(&worker, -1);
+
+       if (rcv(&worker) != 0) e(0);
+
+       for (i = 0; i < signalers; i++) {
+               snd(&signaler[i], worker.pid);
+               snd(&signaler[i], signaler_sig[i]);
+               snd(&signaler[i], NR_SIGNALS);
+       }
+
+       for (i = 0; i < signalers; i++)
+               collect(&signaler[i]);
+
+       if (kill(worker.pid, SIGTERM) != 0) e(0);
+
+       collect(&worker);
+}
+
+/*
+ * Stress test for signal handling.  One worker process gets signals from up to
+ * three signaler processes while performing one of a number of jobs.  It
+ * replies to each signal by signaling the source, thus creating a ping-pong
+ * effect for each of the signaler processes.  The signal ping-ponging is
+ * supposed to be reliable, and the most important aspect of the test is that
+ * no signals get lost.  The test is performed a number of times, varying the
+ * job executed by the worker process, the number of signalers, whether signals
+ * are blocked while executing a signal handler in the worker, and whether the
+ * worker process has a timer running at high frequency.
+ */
+static void
+test79a(void)
+{
+       int job, signalers, options;
+
+       subtest = 1;
+
+       for (options = 0; options <= OPT_ALL; options++)
+               for (signalers = 1; signalers <= MAX_SIGNALERS; signalers++)
+                       for (job = 0; job < NR_JOBS; job++)
+                               sub79a(job, signalers, options);
+}
+
+/*
+ * Set up the worker process and optionally a signaler process, wait for a
+ * predetermined amount of time, and then kill all the child processes.
+ */
+static void
+sub79b(int job, int use_signaler, int options)
+{
+       struct link worker, signaler;
+       struct timeval tv;
+       int i;
+
+       spawn(&worker, worker_proc);
+
+       snd(&worker, job);
+       snd(&worker, options);
+
+       if ((i = use_signaler) != 0) {
+               spawn(&signaler, signaler_proc);
+
+               snd(&worker, signaler.pid);
+       }
+       for (; i < MAX_SIGNALERS; i++)
+               snd(&worker, -1);
+
+       if (rcv(&worker) != 0) e(0);
+
+       if (use_signaler) {
+               snd(&signaler, worker.pid);
+               snd(&signaler, signaler_sig[0]);
+               snd(&signaler, 0);
+       }
+
+       /* Use select() so that we can verify we don't get signals. */
+       tv.tv_sec = 0;
+       tv.tv_usec = 100000;
+       if (select(0, NULL, NULL, NULL, &tv) != 0) e(0);
+
+       terminate(&worker);
+
+       if (use_signaler)
+               terminate(&signaler);
+}
+
+/*
+ * This test is similar to the previous one, except that we now kill the worker
+ * process after a while.  This should trigger various process transitions to
+ * the exiting state.  Not much can be verified from this test program, but we
+ * intend to trigger as many internal state verification statements of PM
+ * itself as possible this way.  A signaler process is optional in this test,
+ * and if used, it will not stop after a predetermined number of signals.
+ */
+static void
+test79b(void)
+{
+       int job, signalers, options;
+
+       subtest = 2;
+
+       for (options = 0; options <= OPT_ALL; options++)
+               for (signalers = 0; signalers <= 1; signalers++)
+                       for (job = 0; job < NR_JOBS; job++)
+                               sub79b(job, signalers, options);
+
+}
+
+/*
+ * PM signal handling robustness test program.
+ */
+int
+main(int argc, char **argv)
+{
+       int i, m;
+
+       start(79);
+
+       if (argc == 2)
+               m = atoi(argv[1]);
+       else
+               m = 0xFF;
+
+       for (i = 0; i < ITERATIONS; i++) {
+               if (m & 0x01) test79a();
+               if (m & 0x02) test79b();
+       }
+
+       quit();
+}