Currently when a pipe to some child is closed the main process errors out
hard. This is not great since the exit reason is not shown.
Change this to break out of the poll loop and also restructure the wait
code to use a loop which checks for both exit and signal status.
I also switched rsync and proc in the pfds and replaced an abort with an
errx call.

With this I see which signal killed a process e.g. if I kill -9 rsync:
...
rpki-client: ta/afrinic: loaded from network
rpki-client: ta/apnic: loaded from network
rpki-client: ta/lacnic: loaded from network
rpki-client: poll[1]: hangup
rpki-client: rsync terminated signal 9

-- 
:wq Claudio

Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.131
diff -u -p -r1.131 main.c
--- main.c      7 Apr 2021 16:06:37 -0000       1.131
+++ main.c      8 Apr 2021 08:48:46 -0000
@@ -538,7 +538,7 @@ entity_process(int proc, struct stats *s
                st->gbrs++;
                break;
        default:
-               abort();
+               errx(1, "unknown entity type");
        }
 
        entity_queue--;
@@ -580,7 +580,6 @@ void
 suicide(int sig __attribute__((unused)))
 {
        killme = 1;
-
 }
 
 #define NPFD   4
@@ -589,9 +588,9 @@ int
 main(int argc, char *argv[])
 {
        int              rc = 1, c, st, proc, rsync, http, rrdp, ok,
-                        fl = SOCK_STREAM | SOCK_CLOEXEC;
+                        hangup = 0, fl = SOCK_STREAM | SOCK_CLOEXEC;
        size_t           i, id, outsz = 0, talsz = 0;
-       pid_t            procpid, rsyncpid, httppid, rrdppid;
+       pid_t            pid, procpid, rsyncpid, httppid, rrdppid;
        int              fd[2];
        struct pollfd    pfd[NPFD];
        struct msgbuf   *queues[NPFD];
@@ -599,7 +598,7 @@ main(int argc, char *argv[])
        char            *rsync_prog = "openrsync";
        char            *bind_addr = NULL;
        const char      *cachedir = NULL, *outputdir = NULL;
-       const char      *tals[TALSZ_MAX], *errs;
+       const char      *tals[TALSZ_MAX], *errs, *name;
        struct vrp_tree  v = RB_INITIALIZER(&v);
        struct rusage   ru;
        struct timeval  start_time, now_time;
@@ -871,10 +870,10 @@ main(int argc, char *argv[])
         * parsing process.
         */
 
-       pfd[0].fd = rsync;
-       queues[0] = &rsyncq;
-       pfd[1].fd = proc;
-       queues[1] = &procq;
+       pfd[0].fd = proc;
+       queues[0] = &procq;
+       pfd[1].fd = rsync;
+       queues[1] = &rsyncq;
        pfd[2].fd = http;
        queues[2] = &httpq;
        pfd[3].fd = rrdp;
@@ -909,8 +908,10 @@ main(int argc, char *argv[])
                for (i = 0; i < NPFD; i++) {
                        if (pfd[i].revents & (POLLERR|POLLNVAL))
                                errx(1, "poll[%zu]: bad fd", i);
-                       if (pfd[i].revents & POLLHUP)
-                               errx(1, "poll[%zu]: hangup", i);
+                       if (pfd[i].revents & POLLHUP) {
+                               warnx("poll[%zu]: hangup", i);
+                               hangup = 1;
+                       }
                        if (pfd[i].revents & POLLOUT) {
                                /*
                                 * XXX work around deadlocks because of
@@ -929,7 +930,8 @@ main(int argc, char *argv[])
                                        io_socket_blocking(pfd[i].fd);
                        }
                }
-
+               if (hangup)
+                       break;
 
                /*
                 * Check the rsync and http process.
@@ -938,7 +940,7 @@ main(int argc, char *argv[])
                 * the parser process.
                 */
 
-               if ((pfd[0].revents & POLLIN)) {
+               if ((pfd[1].revents & POLLIN)) {
                        io_simple_read(rsync, &id, sizeof(id));
                        io_simple_read(rsync, &ok, sizeof(ok));
                        rsync_finish(id, ok);
@@ -1013,7 +1015,7 @@ main(int argc, char *argv[])
                 * Dequeue these one by one.
                 */
 
-               if ((pfd[1].revents & POLLIN)) {
+               if ((pfd[0].revents & POLLIN)) {
                        entity_process(proc, &stats, &v);
                }
        }
@@ -1024,10 +1026,6 @@ main(int argc, char *argv[])
                errx(1, "excessive runtime (%d seconds), giving up", timeout);
        }
 
-       assert(entity_queue == 0);
-       logx("all files parsed: generating output");
-       rc = 0;
-
        /*
         * For clean-up, close the input for the parser and rsync
         * process.
@@ -1039,36 +1037,41 @@ main(int argc, char *argv[])
        close(http);
        close(rrdp);
 
-       if (waitpid(procpid, &st, 0) == -1)
-               err(1, "waitpid");
-       if (!WIFEXITED(st) || WEXITSTATUS(st) != 0) {
-               warnx("parser process exited abnormally");
-               rc = 1;
-       }
-       if (!noop) {
-               if (waitpid(rsyncpid, &st, 0) == -1)
-                       err(1, "waitpid");
-               if (!WIFEXITED(st) || WEXITSTATUS(st) != 0) {
-                       warnx("rsync process exited abnormally");
-                       rc = 1;
+       for (;;) {
+               pid = waitpid(WAIT_ANY, &st, 0);
+               if (pid == -1) {
+                       if (errno == EINTR)
+                               continue;
+                       if (errno != ECHILD)
+                               err(1, "wait");
+                       break;
                }
 
-               if (waitpid(httppid, &st, 0) == -1)
-                       err(1, "waitpid");
-               if (!WIFEXITED(st) || WEXITSTATUS(st) != 0) {
-                       warnx("http process exited abnormally");
-                       rc = 1;
-               }
+               if (pid == procpid)
+                       name = "parser";
+               else if (pid == rsyncpid)
+                       name = "rsync";
+               else if (pid == httppid)
+                       name = "http";
+               else if (pid == rrdppid)
+                       name = "rrdp";
+               else
+                       name = "unknown";
 
-               if (rrdpon) {
-                       if (waitpid(rrdppid, &st, 0) == -1)
-                               err(1, "waitpid");
-                       if (!WIFEXITED(st) || WEXITSTATUS(st) != 0) {
-                               warnx("rrdp process exited abnormally");
-                               rc = 1;
-                       }
+               if (WIFSIGNALED(st)) {
+                       warnx("%s terminated signal %d", name, WTERMSIG(st));
+                       rc = 1;
+               } else if (!WIFEXITED(st) || WEXITSTATUS(st) != 0) {
+                       warnx("%s process exited abnormally", name);
+                       rc = 1;
                }
        }
+
+       /* processing did not finish because of error */
+       if (entity_queue != 0)
+               return 1;
+
+       logx("all files parsed: generating output");
 
        repo_cleanup(&fpt);
 

Reply via email to