The branch, master has been updated
via 143e7d3 lib/util: Make ECHILD in samba_runcmd_io_handler an error
via 30e0238 s4-process_standard: Remove signal(SIGCHLD, SIG_IGN)
from f212143 selftest: rename env member to nt4_member
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 143e7d362797dcb2f92de9231966bf63b604e02e
Author: Andrew Bartlett <abart...@samba.org>
Date: Thu Feb 19 12:41:34 2015 +1300
lib/util: Make ECHILD in samba_runcmd_io_handler an error
We now print out a nasty message and set and error if we get ECHILD,
as we no longer set SIGIGN on SIGCHLD in the standard process model.
This was why samba_kcc was able to fail totally without us noticing.
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Jelmer Vernooij <jel...@samba.org>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Tue Mar 17 07:05:43 CET 2015 on sn-devel-104
commit 30e0238646bc3a891a67dea57baccea8afc1af43
Author: Andrew Bartlett <abart...@samba.org>
Date: Thu Feb 19 12:45:31 2015 +1300
s4-process_standard: Remove signal(SIGCHLD, SIG_IGN)
We replace this with a pipe between parent and child, and then watch
for a read event in the parent to indicate that the child has gone away.
The removal of signal(SIGCHLD, SIG_IGN) requires us to then call
waitpid(). We can't do that in a main loop as we want to get the exit
status to the legitimate waitpid calls in routines like
samba_runcmd_*().
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
lib/util/util_runcmd.c | 6 +-
source4/smbd/process_standard.c | 176 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 179 insertions(+), 3 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c
index 1ec717f..46c455d 100644
--- a/lib/util/util_runcmd.c
+++ b/lib/util/util_runcmd.c
@@ -288,7 +288,11 @@ static void samba_runcmd_io_handler(struct tevent_context
*ev,
SIGCHLD in the standard
process model.
*/
- tevent_req_done(req);
+ DEBUG(0, ("Error in waitpid()
unexpectedly got ECHILD "
+ "for %s child %d - %s, "
+ "someone has set SIGCHLD to
SIG_IGN!\n",
+ state->arg0, state->pid,
strerror(errno)));
+ tevent_req_error(req, errno);
return;
}
DEBUG(0,("Error in waitpid() for child %s - %s
\n",
diff --git a/source4/smbd/process_standard.c b/source4/smbd/process_standard.c
index c5377b3..d3622f9 100644
--- a/source4/smbd/process_standard.c
+++ b/source4/smbd/process_standard.c
@@ -29,6 +29,14 @@
#include "param/param.h"
#include "ldb_wrap.h"
+struct standard_child_state {
+ const char *name;
+ pid_t pid;
+ int to_parent_fd;
+ int from_child_fd;
+ struct tevent_fd *from_child_fde;
+};
+
NTSTATUS process_model_standard_init(void);
/* we hold a pipe open in the parent, and the any child
@@ -42,11 +50,10 @@ static int child_pipe[2];
static void standard_model_init(void)
{
pipe(child_pipe);
- signal(SIGCHLD, SIG_IGN);
}
/*
- handle EOF on the child pipe
+ handle EOF on the parent-to-all-children pipe in the child
*/
static void standard_pipe_handler(struct tevent_context *event_ctx, struct
tevent_fd *fde,
uint16_t flags, void *private_data)
@@ -56,6 +63,132 @@ static void standard_pipe_handler(struct tevent_context
*event_ctx, struct teven
}
/*
+ handle EOF on the child pipe in the parent, so we know when a
+ process terminates without using SIGCHLD or waiting on all possible pids.
+
+ We need to ensure we do not ignore SIGCHLD because we need it to
+ work to get a valid error code from samba_runcmd_*().
+ */
+static void standard_child_pipe_handler(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *private_data)
+{
+ struct standard_child_state *state
+ = talloc_get_type_abort(private_data, struct
standard_child_state);
+ int status = 0;
+ pid_t pid;
+
+ /* the child has closed the pipe, assume its dead */
+ errno = 0;
+ 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 child %d (%s) - %s, someone has set
SIGCHLD "
+ "to SIG_IGN!\n",
+ state->pid, state->name, strerror(errno)));
+ TALLOC_FREE(state);
+ return;
+ }
+ DEBUG(0, ("Error in waitpid() for child %d (%s) - %s \n",
+ state->pid, state->name, strerror(errno)));
+ if (errno == 0) {
+ errno = ECHILD;
+ }
+ TALLOC_FREE(state);
+ return;
+ }
+ if (WIFEXITED(status)) {
+ status = WEXITSTATUS(status);
+ DEBUG(2, ("Child %d (%s) exited with status %d\n",
+ state->pid, state->name, status));
+ } else if (WIFSIGNALED(status)) {
+ status = WTERMSIG(status);
+ DEBUG(0, ("Child %d (%s) terminated with signal %d\n",
+ state->pid, state->name, status));
+ }
+ TALLOC_FREE(state);
+ return;
+}
+
+static struct standard_child_state *setup_standard_child_pipe(struct
tevent_context *ev,
+ const char *name)
+{
+ struct standard_child_state *state;
+ int parent_child_pipe[2];
+ int ret;
+
+ /*
+ * Prepare a pipe to allow us to know when the child exits,
+ * because it will trigger a read event on this private
+ * pipe.
+ *
+ * We do all this before the accept and fork(), so we can
+ * clean up if it fails.
+ */
+ state = talloc_zero(ev, struct standard_child_state);
+ if (state == NULL) {
+ return NULL;
+ }
+
+ if (name == NULL) {
+ name = "";
+ }
+
+ state->name = talloc_strdup(state, name);
+ if (state->name == NULL) {
+ TALLOC_FREE(state);
+ return NULL;
+ }
+
+ ret = pipe(parent_child_pipe);
+ if (ret == -1) {
+ DEBUG(0, ("Failed to create parent-child pipe to handle "
+ "SIGCHLD to track new process for socket\n"));
+ TALLOC_FREE(state);
+ return NULL;
+ }
+
+ smb_set_close_on_exec(parent_child_pipe[0]);
+ smb_set_close_on_exec(parent_child_pipe[1]);
+
+ state->from_child_fd = parent_child_pipe[0];
+ state->to_parent_fd = parent_child_pipe[1];
+
+ /*
+ * The basic purpose of calling this handler is to ensure we
+ * call waitpid() and so avoid zombies (now that we no longer
+ * user SIGIGN on for SIGCHLD), but it also allows us to clean
+ * up other resources in the future.
+ */
+ state->from_child_fde = tevent_add_fd(ev, state,
+ state->from_child_fd,
+ TEVENT_FD_READ,
+ standard_child_pipe_handler,
+ state);
+ if (state->from_child_fde == NULL) {
+ TALLOC_FREE(state);
+ return NULL;
+ }
+ tevent_fd_set_auto_close(state->from_child_fde);
+
+ return state;
+}
+
+/*
called when a listening socket becomes readable.
*/
static void standard_accept_connection(struct tevent_context *ev,
@@ -70,6 +203,12 @@ static void standard_accept_connection(struct
tevent_context *ev,
struct socket_context *sock2;
pid_t pid;
struct socket_address *c, *s;
+ struct standard_child_state *state;
+
+ state = setup_standard_child_pipe(ev, NULL);
+ if (state == NULL) {
+ return;
+ }
/* accept an incoming connection. */
status = socket_accept(sock, &sock2);
@@ -79,18 +218,33 @@ static void standard_accept_connection(struct
tevent_context *ev,
/* this looks strange, but is correct. We need to throttle
things until
the system clears enough resources to handle this new socket
*/
sleep(1);
+ close(state->to_parent_fd);
+ state->to_parent_fd = -1;
+ TALLOC_FREE(state);
return;
}
pid = fork();
if (pid != 0) {
+ close(state->to_parent_fd);
+ state->to_parent_fd = -1;
+
+ if (pid > 0) {
+ state->pid = pid;
+ } else {
+ TALLOC_FREE(state);
+ }
+
/* parent or error code ... */
talloc_free(sock2);
/* go back to the event loop */
return;
}
+ /* this leaves state->to_parent_fd open */
+ TALLOC_FREE(state);
+
pid = getpid();
/* This is now the child code. We need a completely new event_context
to work with */
@@ -149,14 +303,32 @@ static void standard_new_task(struct tevent_context *ev,
void *private_data)
{
pid_t pid;
+ struct standard_child_state *state;
+
+ state = setup_standard_child_pipe(ev, service_name);
+ if (state == NULL) {
+ return;
+ }
pid = fork();
if (pid != 0) {
+ close(state->to_parent_fd);
+ state->to_parent_fd = -1;
+
+ if (pid > 0) {
+ state->pid = pid;
+ } else {
+ TALLOC_FREE(state);
+ }
+
/* parent or error code ... go back to the event loop */
return;
}
+ /* this leaves state->to_parent_fd open */
+ TALLOC_FREE(state);
+
pid = getpid();
/* this will free all the listening sockets and all state that
--
Samba Shared Repository
