On Sun, Jan 7, 2018 at 2:34 PM, Laurent Bercot <ska-dietl...@skarnet.org> wrote:
>> It depends whether you consider init to be required to reap zombies
>> as fast as possible.
>>
>> I don't see that as a requirement (so far, feel free to convince
>> me otherwise).
>
>
>  I don't see it as a hard requirement either; I don't really mind having
> zombies around for a few seconds, provided they eventually get reapt.
>
>  However, most people are used to the sysvinit behaviour, and rely on
> it. Few people are aware that bb init may wait for one second before
> reaping zombies, because the window is not hit often, only bb init does
> that, and bb init is not as common as other inits. So it's a
> peculiarity that people do not plan for, and it may bite them.
>
>  The javascript test I mentioned failed on Alpine Linux because it hit
> the one second window; the test was incorrectly designed, but it worked
> with basically every other init out there.
>  The current thread's OP observed zombies when killing runsvdir, which
> led him on a red herring chase, a misguided patch, and wasted time in
> this thread.
>
>  I'm personally not impacted, because I have my own pid 1 (s6-svscan)
> which reaps zombies as soon as they happen; and stricto sensu, I don't
> think bb init's behaviour is a bug. But changing it to mimic the
> behaviour of all the other inits would certainly prevent similar
> annoying, if minor, problems from arising in the future.

How about this:

diff --git a/init/init.c b/init/init.c
index 6f3374e..2797b3b 100644
--- a/init/init.c
+++ b/init/init.c
@@ -592,9 +592,10 @@ static void waitfor(pid_t pid)
 }

 /* Run all commands of a particular type */
-static void run_actions(int action_type)
+static int run_actions(int action_type)
 {
     struct init_action *a;
+    int started = 0;

     for (a = init_action_list; a; a = a->next) {
         if (!(a->action_type & action_type))
@@ -609,10 +610,14 @@ static void run_actions(int action_type)
             /* Only run stuff with pid == 0. If pid != 0,
              * it is already running
              */
-            if (a->pid == 0)
+            if (a->pid == 0) {
                 a->pid = run(a);
+                if (a->pid > 0)
+                    started = 1;
+            }
         }
     }
+    return started;
 }

 static void new_init_action(uint8_t action_type, const char *command,
const char *cons)
@@ -1205,21 +1210,26 @@ int init_main(int argc UNUSED_PARAM, char **argv)
      */
     while (1) {
         int maybe_WNOHANG;
+        int started;

         maybe_WNOHANG = check_delayed_sigs();

         /* (Re)run the respawn/askfirst stuff */
-        run_actions(RESPAWN | ASKFIRST);
+        started = run_actions(RESPAWN | ASKFIRST);
         maybe_WNOHANG |= check_delayed_sigs();

-        /* Don't consume all CPU time - sleep a bit */
-        sleep(1);
-        maybe_WNOHANG |= check_delayed_sigs();
+        if (started) {
+            /* In case "respawn" processes die at once, don't consume
+             * lots of CPU - sleep a bit after each respawn.
+             */
+            sleep(1);
+            maybe_WNOHANG |= check_delayed_sigs();
+        }

         /* Wait for any child process(es) to exit.
          *
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to