lib/util: make use of tfork in samba_runcmd_send()

This makes it possible to use samba_runcmd_send() in processes like smbd
that install a SIGCHLD handler that reaps all terminated children.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

2 files changed, 59 insertions, 50 deletions

diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c
index 9264cbba11b..ac74371e76d 100644
--- a/lib/util/util_runcmd.c
+++ b/lib/util/util_runcmd.c
@@ -29,6 +29,8 @@
 #include "system/filesys.h"
 #include "../lib/util/tevent_unix.h"
 #include "../lib/util/util_runcmd.h"
+#include "../lib/util/tfork.h"
+#include "../lib/util/sys_rw.h"
 
 static int samba_runcmd_state_destructor(struct samba_runcmd_state *state)
 {
@@ -106,8 +108,9 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->pid = fork();
-	if (state->pid == (pid_t)-1) {
+	state->tfork = tfork_create();
+	if (state->tfork == NULL) {
+		printf("state->tfork == NULL\n");
 		close(p1[0]);
 		close(p1[1]);
 		close(p2[0]);
@@ -117,7 +120,7 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
 		tevent_req_error(req, errno);
 		return tevent_req_post(req, ev);
 	}
-
+	state->pid = tfork_child_pid(state->tfork);
 	if (state->pid != 0) {
 		/* the parent */
 		close(p1[1]);
@@ -126,14 +129,17 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
 		state->fd_stdout = p1[0];
 		state->fd_stderr = p2[0];
 		state->fd_stdin  = p3[1];
+		state->fd_status = tfork_event_fd(state->tfork);
 
 		set_blocking(state->fd_stdout, false);
 		set_blocking(state->fd_stderr, false);
 		set_blocking(state->fd_stdin,  false);
+		set_blocking(state->fd_status, false);
 
 		smb_set_close_on_exec(state->fd_stdin);
 		smb_set_close_on_exec(state->fd_stdout);
 		smb_set_close_on_exec(state->fd_stderr);
+		smb_set_close_on_exec(state->fd_status);
 
 		talloc_set_destructor(state, samba_runcmd_state_destructor);
 
@@ -145,6 +151,7 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
 		if (tevent_req_nomem(state->fde_stdout, req)) {
 			close(state->fd_stdout);
 			close(state->fd_stderr);
+			close(state->fd_status);
 			return tevent_req_post(req, ev);
 		}
 		tevent_fd_set_auto_close(state->fde_stdout);
@@ -155,11 +162,26 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
 						  samba_runcmd_io_handler,
 						  req);
 		if (tevent_req_nomem(state->fde_stdout, req)) {
+			close(state->fd_stdout);
 			close(state->fd_stderr);
+			close(state->fd_status);
 			return tevent_req_post(req, ev);
 		}
 		tevent_fd_set_auto_close(state->fde_stderr);
 
+		state->fde_status = tevent_add_fd(ev, state,
+						  state->fd_status,
+						  TEVENT_FD_READ,
+						  samba_runcmd_io_handler,
+						  req);
+		if (tevent_req_nomem(state->fde_stdout, req)) {
+			close(state->fd_stdout);
+			close(state->fd_stderr);
+			close(state->fd_status);
+			return tevent_req_post(req, ev);
+		}
+		tevent_fd_set_auto_close(state->fde_status);
+
 		if (!timeval_is_zero(&endtime)) {
 			tevent_req_set_endtime(req, ev, endtime);
 		}
@@ -231,6 +253,10 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
 	char *p;
 	int n, fd;
 
+	if (!(flags & TEVENT_FD_READ)) {
+		return;
+	}
+
 	if (fde == state->fde_stdout) {
 		level = state->stdout_log_level;
 		fd = state->fd_stdout;
@@ -238,10 +264,33 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
 		level = state->stderr_log_level;
 		fd = state->fd_stderr;
 	} else {
-		return;
-	}
+		int status;
 
-	if (!(flags & TEVENT_FD_READ)) {
+		status = tfork_status(&state->tfork, false);
+		if (status == -1) {
+			if (errno == EAGAIN || errno == EWOULDBLOCK) {
+				return;
+			}
+			DBG_ERR("Bad read on status pipe\n");
+			tevent_req_error(req, errno);
+			return;
+		}
+
+		if (WIFEXITED(status)) {
+			status = WEXITSTATUS(status);
+		} else if (WIFSIGNALED(status)) {
+			status = WTERMSIG(status);
+		} else {
+			status = ECHILD;
+		}
+
+		DBG_NOTICE("Child %s exited %d\n", state->arg0, status);
+		if (status != 0) {
+			tevent_req_error(req, status);
+			return;
+		}
+
+		tevent_req_done(req);
 		return;
 	}
 
@@ -253,53 +302,11 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
 		if (fde == state->fde_stdout) {
 			talloc_free(fde);
 			state->fde_stdout = NULL;
+			return;
 		}
 		if (fde == state->fde_stderr) {
 			talloc_free(fde);
 			state->fde_stderr = NULL;
-		}
-		if (state->fde_stdout == NULL &&
-		    state->fde_stderr == NULL) {
-			int status;
-			/* the child has closed both stdout and
-			 * stderr, assume its dead */
-			pid_t pid = waitpid(state->pid, &status, 0);
-			if (pid != state->pid) {
-				if (errno == ECHILD) {
-					/* this happens when the
-					   parent has set SIGCHLD to
-					   SIG_IGN. In that case we
-					   can only get error
-					   information for the child
-					   via its logging. We should
-					   stop using SIG_IGN on
-					   SIGCHLD in the standard
-					   process model.
-					*/
-					DEBUG(0, ("Error in waitpid() unexpectedly got ECHILD "
-						  "for %s child %d - %s, "
-						  "someone has set SIGCHLD to SIG_IGN!\n",
-					state->arg0, (int)state->pid, strerror(errno)));
-					tevent_req_error(req, errno);
-					return;
-				}
-				DEBUG(0,("Error in waitpid() for child %s - %s \n",
-					 state->arg0, strerror(errno)));
-				if (errno == 0) {
-					errno = ECHILD;
-				}
-				tevent_req_error(req, errno);
-				return;
-			}
-			status = WEXITSTATUS(status);
-			DEBUG(3,("Child %s exited with status %d\n",
-				 state->arg0, status));
-			if (status != 0) {
-				tevent_req_error(req, status);
-				return;
-			}
-
-			tevent_req_done(req);
 			return;
 		}
 		return;
diff --git a/lib/util/util_runcmd.h b/lib/util/util_runcmd.h
index 26fdbc66042..55329615a25 100644
--- a/lib/util/util_runcmd.h
+++ b/lib/util/util_runcmd.h
@@ -27,9 +27,11 @@ struct samba_runcmd_state {
 	int stderr_log_level;
 	struct tevent_fd *fde_stdout;
 	struct tevent_fd *fde_stderr;
-	int fd_stdin, fd_stdout, fd_stderr;
+	struct tevent_fd *fde_status;
+	int fd_stdin, fd_stdout, fd_stderr, fd_status;
 	char *arg0;
 	pid_t pid;
+	struct tfork *tfork;
 	char buf[1024];
 	uint16_t buf_used;
 };