util/tfork: Write to the status pipe
authorGary Lockyer <gary@catalyst.net.nz>
Sun, 10 Sep 2017 22:25:49 +0000 (10:25 +1200)
committerRalph Boehme <slow@samba.org>
Sat, 16 Sep 2017 17:53:23 +0000 (19:53 +0200)
The previous design relied on only calling close() of the status pipe.

We now write a single 0 byte to the status FD as well as closing it in the
parent process.  Both of these operations typically trigger a read
event on the other end of the FD, held in the waiter process (the child).

The child process blocks on the status FD, until it becomes readable.

However if there is a sibling process that was launched after the waiter
process they also will hold the status FD open and the status FD would,
until this change, never become readable to the waiter process (the child).

This caused the waiter process (child) not to exit and the parent process
to hang in tfork_status() while expecting the waitpid() to return.

That is, file descriptors are essentially global variables copied
to children in the process tree.  The last child that (unwittingly) holds
the file descriptor open is the one that needs to trigger the close() this
code previously depended on.

Without this change, there is no notification of process death until
all these unrelated children exit for their own reasons.

We can write up to 4K (PIPE_BUF) into this pipe before blocking,
but we only write one byte.  Additionally sys_write() refuses to block.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13037

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
lib/util/tfork.c

index cc2e0a05f4e5b865c59387cd6aae6c2512335d83..71f5e1b6ce91110316567a188c1042236e309975 100644 (file)
@@ -670,7 +670,7 @@ static pid_t tfork_start_waiter_and_worker(struct tfork_state *state,
                }
                _exit(errno);
        }
-       if (nread != 0) {
+       if (nread != 1) {
                _exit(255);
        }
 
@@ -838,7 +838,18 @@ int tfork_status(struct tfork **_t, bool wait)
 
        /*
         * This triggers process exit in the waiter.
+        * We write to the fd as well as closing it, as any tforked sibling
+        * processes will also have the writable end of this socket open.
+        *
         */
+       {
+               size_t nwritten;
+               nwritten = sys_write(t->status_fd, &(char){0}, sizeof(char));
+               if (nwritten != sizeof(char)) {
+                       close(t->status_fd);
+                       return -1;
+               }
+       }
        close(t->status_fd);
 
        do {