Fix vipw not resuming correctly when suspended

Closes #185

If vipw is suspended (e.g. via control-Z) and then resumed, it often gets
immediately suspended. This is easier to reproduce on a multi-core system.

root@buster:~# /usr/sbin/vipw

[1]+  Stopped                 /usr/sbin/vipw
root@buster:~# fg
/usr/sbin/vipw

[1]+  Stopped                 /usr/sbin/vipw

root@buster:~# fg
[vipw resumes on the second fg]

The problem is that vipw forks a child process and calls waitpid() with the
WUNTRACED flag. When the child process (running the editor) is suspended, the
parent sends itself SIGSTOP to suspend the main vipw process. However, because
the main vipw is in the same process group as the editor which received the ^Z,
the kernel already sent the main vipw SIGTSTP.

If the main vipw receives SIGTSTP before the child, it will be suspended and
then, once resumed, will proceed to suspend itself again.

To fix this, run the child process in its own process group as the foreground
process group. That way, control-Z will only affect the child process and the
parent can use the existing logic to suspend the parent.

diff --git a/src/vipw.c b/src/vipw.c
index 2cfac6b4810e05321647ec3e42211bbe036882c6..04f0b23325d51f64a59e2511e0a976aa8b8f898e 100644 (file)
--- a/src/vipw.c
+++ b/src/vipw.c
@@ -207,6 +207,8 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
        struct stat st1, st2;
        int status;
        FILE *f;
+       pid_t orig_pgrp, editor_pgrp = -1;
+       sigset_t mask, omask;
        /* FIXME: the following should have variable sizes */
        char filebackup[1024], fileedit[1024];
        char *to_rename;
@@ -294,6 +296,8 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
                editor = DEFAULT_EDITOR;
        }
 
+       orig_pgrp = tcgetpgrp(STDIN_FILENO);
+
        pid = fork ();
        if (-1 == pid) {
                vipwexit ("fork", 1, 1);
@@ -303,6 +307,14 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
                char *buf;
                int status;
 
+               /* Wait for parent to make us the foreground pgrp. */
+               if (orig_pgrp != -1) {
+                       pid = getpid();
+                       setpgid(0, 0);
+                       while (tcgetpgrp(STDIN_FILENO) != pid)
+                               continue;
+               }
+
                buf = (char *) malloc (strlen (editor) + strlen (fileedit) + 2);
                snprintf (buf, strlen (editor) + strlen (fileedit) + 2,
                          "%s %s", editor, fileedit);
@@ -325,19 +337,50 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
                }
        }
 
+       /* Run child in a new pgrp and make it the foreground pgrp. */
+       if (orig_pgrp != -1) {
+               setpgid(pid, pid);
+               tcsetpgrp(STDIN_FILENO, pid);
+
+               /* Avoid SIGTTOU when changing foreground pgrp below. */
+               sigemptyset(&mask);
+               sigaddset(&mask, SIGTTOU);
+               sigprocmask(SIG_BLOCK, &mask, &omask);
+       }
+
        for (;;) {
                pid = waitpid (pid, &status, WUNTRACED);
                if ((pid != -1) && (WIFSTOPPED (status) != 0)) {
                        /* The child (editor) was suspended.
-                        * Suspend vipw. */
+                        * Restore terminal pgrp and suspend vipw. */
+                       if (orig_pgrp != -1) {
+                               editor_pgrp = tcgetpgrp(STDIN_FILENO);
+                               if (editor_pgrp == -1) {
+                                       fprintf (stderr, "%s: %s: %s", Prog,
+                                                "tcgetpgrp", strerror (errno));
+                               }
+                               if (tcsetpgrp(STDIN_FILENO, orig_pgrp) == -1) {
+                                       fprintf (stderr, "%s: %s: %s", Prog,
+                                                "tcsetpgrp", strerror (errno));
+                               }
+                       }
                        kill (getpid (), SIGSTOP);
                        /* wake child when resumed */
-                       kill (pid, SIGCONT);
+                       if (editor_pgrp != -1) {
+                               if (tcsetpgrp(STDIN_FILENO, editor_pgrp) == -1) {
+                                       fprintf (stderr, "%s: %s: %s", Prog,
+                                                "tcsetpgrp", strerror (errno));
+                               }
+                       }
+                       killpg (pid, SIGCONT);
                } else {
                        break;
                }
        }
 
+       if (orig_pgrp != -1)
+               sigprocmask(SIG_SETMASK, &omask, NULL);
+
        if (-1 == pid) {
                vipwexit (editor, 1, 1);
        } else if (   WIFEXITED (status)