process: Fix bug with child processes stuck forever when parent segfaults

After a fork() the child needs to close the unused end of the process
control pipes. Failing to do so will prevent EOF and EPIPE signals
from being delivered correctly back to the child when the parent
closes its end of the pipe.

Furthermore, the code's assumption about the behaviour of the pipe
with one writer and multiple readers was wrong. When there are
multiple readers, only one will be woken up and the rest will be left
blocking the read in case there was nothing left to read. If read
returns zero, that means EOF, eg. the job control parent has closed
its end of the pipe.

These fixes applied there should be no more problem with a lot of
child processes stuck in waiting the parent to give them a permission
to go. They will either fail with SIGPIPE when they request the
permission to run or they will read zero bytes from the pipe and
continue executing instantly.

Signed-off-by: Timo Kokkonen <timo.t.kokkonen@iki.fi>

diff --git a/process.c b/process.c
index e226c90d3371355cfffae139db471114ce6d3ee2..1e5b0b755a3673f94a0e007461a76519f8f5db52 100644 (file)
--- a/process.c
+++ b/process.c
@@ -282,6 +282,30 @@ int do_fork(void)
         */
        is_limited_fork = 0;
 
+       /*
+        * Close unused ends of the job control pipes. Only the parent
+        * which controls the jobs may have the write end open of the
+        * job_get_permission_fd and the read end of the
+        * job_request_fd. Failing to close the pipe ends properly
+        * will cause the childs to wait forever for the run
+        * permission in case parent dies prematurely.
+        *
+        * Note! The file descriptor must be closed once and only
+        * once. They are marked to -1 to make it impossible for
+        * subseqent do_fork() calls from closing them again (in which
+        * case some other file descriptor might already be reserved
+        * for the same number) and prevent accidentally closing some
+        * innocent file descriptors that are still in use.
+        */
+       if (job_get_permission_fd[1] >= 0) {
+               close(job_get_permission_fd[1]);
+               job_get_permission_fd[1] = -1;
+       }
+       if (job_request_fd[0] >= 0) {
+               close(job_request_fd[0]);
+               job_request_fd[0] = -1;
+       }
+
        /* reset child's child count */
        child_count = 0;
        parent_count++;
@@ -330,18 +354,16 @@ int do_fork_limited(void)
 
        /*
         * The parent will tell us when we can continue. If there were
-        * multiple children waiting for their turn to run parent's
-        * write to the pipe will wake up every process reading
-        * it. However, only one will be reading the content and
-        * getting the permission to run. So we will read as many
-        * times as needed until we get our own permission to run.
+        * multiple children waiting for their turn to run only one
+        * will be reading the content byte from the pipe and getting
+        * the permission to run.
         */
-       do {
-               ret = read(job_get_permission_fd[0], &byte, sizeof(byte));
-               if (ret == 1)
-                       break;
+       ret = read(job_get_permission_fd[0], &byte, sizeof(byte));
+       if (ret == 0)
+               pr_err("Error requesting run, did the parent die?\n");
 
-       } while(1);
+       if (ret < 0)
+               pr_err("Job control request failure: %m\n");
 
        pr_info("Continuing\n");
        return child;