[PATCH/RFC] Re: logpipe error handling patch

From: Gerrit Pape <pape_at_smarden.org>
Date: Fri, 1 Aug 2014 05:32:16 +0000

On Wed, Jul 30, 2014 at 03:01:16PM +0200, Jan Pobrislo wrote:
> On Wed, 30 Jul 2014 00:37:47 +0100
> Laurent Bercot <ska-supervision_at_skarnet.org> wrote:
> > At this point, the service is
> > dysfunctional and there's no way to recover it: so runsv should not
> > be accepting commands anymore, and close its control pipe instead.
>
> I was under the assumption that the pipe is left open so you can send
> signals in case the supervised processes have trouble exiting the
> normal way.

The service stopped already in this situation, I'm with Laurent in this
case. But not the log service, so its control pipe should stay open and
commands processed.

What do you think about this patch?:


    When runsv is told to exit, it now no longer processes any control
    characters read from the control pipe (it still does for the log
    service). It waits until the log service has terminated and then
    exits. The sv program now properly reports this through the status
    command.

---
 man/runsv.8     |  1 +
 man/sv.8        |  1 +
 package/CHANGES |  2 ++
 src/runsv.c     | 25 ++++++++++++++-----------
 src/sv.c        |  5 ++++-
 5 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/man/runsv.8 b/man/runsv.8
index 7c5abfc..8d364c6 100644
--- a/man/runsv.8
+++ b/man/runsv.8
_at_@ -150,6 +150,7 @@ exits.
 If the service is down and a log service exists,
 .B runsv
 closes the standard input of the log service, and waits for it to terminate.
+No more control characters will be processed.
 If the log service is down,
 .B runsv
 exits.
diff --git a/man/sv.8 b/man/sv.8
index 7b09602..0c9f4c3 100644
--- a/man/sv.8
+++ b/man/sv.8
_at_@ -78,6 +78,7 @@ exits.
 If the service is down and a log service exists,
 .BR runsv (8)
 closes the standard input of the log service and waits for it to terminate.
+No more commands will be processed.
 If the log service is down,
 .BR runsv (8)
 exits.
diff --git a/package/CHANGES b/package/CHANGES
index 82d117c..1286c8f 100644
--- a/package/CHANGES
+++ b/package/CHANGES
_at_@ -1,5 +1,7 @@
 2.1.2
 
+  * runsv.c, sv.c: on exit, properly wait for log service to terminate.
+  * man/runsv.8, man/sv.8: adapt.
   * man/sv.8: "sv exit" does not send a TERM signal to the log service
     (thx Jonathan Nieder).
   * fmt_ptime.c: 64 bits time_t fix for svlogd (tnx Jérémie
diff --git a/src/runsv.c b/src/runsv.c
index ecf4677..dc628fa 100644
--- a/src/runsv.c
+++ b/src/runsv.c
_at_@ -31,6 +31,7 @@ int selfpipe[2];
 #define S_DOWN 0
 #define S_RUN 1
 #define S_FINISH 2
+#define S_WAIT 3
 /* ctrl */
 #define C_NOOP 0
 #define C_TERM 1
_at_@ -149,6 +150,9 @@ void update_status(struct svdir *s) {
   case S_FINISH:
     buffer_puts(&b, "finish");
     break;
+  case S_WAIT:
+    buffer_puts(&b, "wait");
+    break;
   }
   if (s->ctrl & C_PAUSE) buffer_puts(&b, ", paused");
   if (s->ctrl & C_TERM) buffer_puts(&b, ", got TERM");
_at_@ -566,6 +570,13 @@ int main(int argc, char **argv) {
             continue;
           }
         svd[0].state =S_DOWN;
+        if (svd[0].want == W_EXIT) {
+          svd[0].state =S_WAIT;
+          svd[1].want =W_EXIT;
+          update_status(&svd[1]);
+          if (close(logpipe[1]) == -1) warn("unable to close logpipe[1]");
+          if (close(logpipe[0]) == -1) warn("unable to close logpipe[0]");
+        }
         taia_uint(&deadline, 1);
         taia_add(&deadline, &svd[0].start, &deadline);
         taia_now(&svd[0].start);
_at_@ -586,22 +597,14 @@ int main(int argc, char **argv) {
         }
       }
     }
-    if (read(svd[0].fdcontrol, &ch, 1) == 1) ctrl(&svd[0], ch);
+    if (read(svd[0].fdcontrol, &ch, 1) == 1)
+      if (svd[0].state != S_WAIT) ctrl(&svd[0], ch);
     if (haslog)
       if (read(svd[1].fdcontrol, &ch, 1) == 1) ctrl(&svd[1], ch);
 
     if (sigterm) { ctrl(&svd[0], 'x'); sigterm =0; }
 
-    if ((svd[0].want == W_EXIT) && (svd[0].state == S_DOWN)) {
-      if (svd[1].pid == 0) _exit(0);
-      if (svd[1].want != W_EXIT) {
-        svd[1].want =W_EXIT;
-        /* stopservice(&svd[1]); */
-        update_status(&svd[1]);
-        if (close(logpipe[1]) == -1) warn("unable to close logpipe[1]");
-        if (close(logpipe[0]) == -1) warn("unable to close logpipe[0]");
-      }
-    }
+    if (svd[0].state == S_WAIT) if (svd[1].pid == 0) _exit(0);
   }
   _exit(0);
 }
diff --git a/src/sv.c b/src/sv.c
index 06b2e41..db0bd1d 100644
--- a/src/sv.c
+++ b/src/sv.c
_at_@ -26,6 +26,7 @@
 #define RUN     "run: "
 #define FINISH  "finish: "
 #define DOWN    "down: "
+#define WAIT    "wait: "
 #define TIMEOUT "timeout: "
 #define KILL    "kill: "
 
_at_@ -132,9 +133,10 @@ unsigned int svstatus_print(char *m) {
   case 0: outs(DOWN); break;
   case 1: outs(RUN); break;
   case 2: outs(FINISH); break;
+  case 3: outs(WAIT); break;
   }
   outs(m); outs(": ");
-  if (svstatus[19]) {
+  if (pid) {
     outs("(pid "); sulong[fmt_ulong(sulong, pid)] =0;
     outs(sulong); outs(") ");
   }
_at_@ -146,6 +148,7 @@ unsigned int svstatus_print(char *m) {
   if (pid && svstatus[16]) outs(", paused");
   if (!pid && (svstatus[17] == 'u')) outs(", want up");
   if (pid && (svstatus[17] == 'd')) outs(", want down");
+  if (svstatus[19] == 3) outs(", want exit");
   if (pid && svstatus[18]) outs(", got TERM");
   return(pid ? 1 : 2);
 }

Received on Fri Aug 01 2014 - 05:32:16 UTC