From 7f2a82103d8537438470f5d560b5bfd12e52a915 Mon Sep 17 00:00:00 2001 From: Ondrej Kos Date: Mon, 14 Jan 2013 13:56:51 +0100 Subject: Add common SIGCHLD handling for providers backport of https://fedorahosted.org/sssd/changeset/6a9bdb6289bb374d203861cef16f312185725cbc --- Makefile.am | 11 +- src/providers/child_common.c | 222 +++++++++++++++++++++++++++++++++++++-- src/providers/child_common.h | 25 +++++ src/providers/data_provider_be.c | 8 ++ src/providers/dp_backend.h | 1 + 5 files changed, 259 insertions(+), 8 deletions(-) diff --git a/Makefile.am b/Makefile.am index 156de2503..5991a9f18 100644 --- a/Makefile.am +++ b/Makefile.am @@ -426,6 +426,7 @@ sssd_pam_LDADD = \ $(SSSD_LIBS) sssd_be_SOURCES = \ + src/providers/child_common.c \ src/providers/data_provider_be.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ @@ -941,7 +942,9 @@ krb5_child_SOURCES = \ src/providers/child_common.c \ src/providers/dp_pam_data_util.c \ src/util/user_info_msg.c \ - src/util/sss_krb5.c + src/util/sss_krb5.c \ + src/util/util.c \ + src/util/signal.c krb5_child_CFLAGS = \ $(AM_CFLAGS) \ $(POPT_CFLAGS) \ @@ -950,13 +953,16 @@ krb5_child_LDADD = \ $(TALLOC_LIBS) \ $(TEVENT_LIBS) \ $(POPT_LIBS) \ + $(DHASH_LIBS) \ $(KRB5_LIBS) ldap_child_SOURCES = \ $(SSSD_DEBUG_OBJ) \ src/providers/ldap/ldap_child.c \ src/providers/child_common.c \ - src/util/sss_krb5.c + src/util/sss_krb5.c \ + src/util/util.c \ + src/util/signal.c ldap_child_CFLAGS = \ $(AM_CFLAGS) \ $(POPT_CFLAGS) \ @@ -966,6 +972,7 @@ ldap_child_LDADD = \ $(TEVENT_LIBS) \ $(POPT_LIBS) \ $(OPENLDAP_LIBS) \ + $(DHASH_LIBS) \ $(KRB5_LIBS) proxy_child_SOURCES = \ diff --git a/src/providers/child_common.c b/src/providers/child_common.c index 16618c780..1cead3893 100644 --- a/src/providers/child_common.c +++ b/src/providers/child_common.c @@ -33,7 +33,217 @@ #include "db/sysdb.h" #include "providers/child_common.h" +struct sss_sigchild_ctx { + struct tevent_context *ev; + hash_table_t *children; + int options; +}; + struct sss_child_ctx { + pid_t pid; + sss_child_fn_t cb; + void *pvt; + struct sss_sigchild_ctx *sigchld_ctx; +}; + +errno_t sss_sigchld_init(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sss_sigchild_ctx **child_ctx) +{ + errno_t ret; + struct sss_sigchild_ctx *sigchld_ctx; + struct tevent_signal *tes; + + sigchld_ctx = talloc_zero(mem_ctx, struct sss_sigchild_ctx); + if (!sigchld_ctx) { + DEBUG(0, ("fatal error initializing sss_sigchild_ctx\n")); + return ENOMEM; + } + sigchld_ctx->ev = ev; + + ret = sss_hash_create(sigchld_ctx, 10, &sigchld_ctx->children); + if (ret != EOK) { + DEBUG(0, ("fatal error initializing children hash table: [%s]\n", + strerror(ret))); + talloc_free(sigchld_ctx); + return ret; + } + + BlockSignals(false, SIGCHLD); + tes = tevent_add_signal(ev, sigchld_ctx, SIGCHLD, SA_SIGINFO, + sss_child_handler, sigchld_ctx); + if (tes == NULL) { + talloc_free(sigchld_ctx); + return EIO; + } + + *child_ctx = sigchld_ctx; + return EOK; +} + +static int sss_child_destructor(void *ptr) +{ + struct sss_child_ctx *child_ctx; + hash_key_t key; + int error; + + child_ctx = talloc_get_type(ptr, struct sss_child_ctx); + key.type = HASH_KEY_ULONG; + key.ul = child_ctx->pid; + + error = hash_delete(child_ctx->sigchld_ctx->children, &key); + if (error != HASH_SUCCESS && error != HASH_ERROR_KEY_NOT_FOUND) { + DEBUG(8, ("failed to delete child_ctx from hash table [%d]: %s\n", + error, hash_error_string(error))); + } + + return 0; +} + +errno_t sss_child_register(TALLOC_CTX *mem_ctx, + struct sss_sigchild_ctx *sigchld_ctx, + pid_t pid, + sss_child_fn_t cb, + void *pvt, + struct sss_child_ctx **child_ctx) +{ + struct sss_child_ctx *child; + hash_key_t key; + hash_value_t value; + int error; + + child = talloc_zero(mem_ctx, struct sss_child_ctx); + if (child == NULL) { + return ENOMEM; + } + + child->pid = pid; + child->cb = cb; + child->pvt = pvt; + child->sigchld_ctx = sigchld_ctx; + + key.type = HASH_KEY_ULONG; + key.ul = pid; + + value.type = HASH_VALUE_PTR; + value.ptr = child; + + error = hash_enter(sigchld_ctx->children, &key, &value); + if (error != HASH_SUCCESS) { + talloc_free(child); + return ENOMEM; + } + + talloc_set_destructor((TALLOC_CTX *) child, sss_child_destructor); + + *child_ctx = child; + return EOK; +} + +struct sss_child_cb_pvt { + struct sss_child_ctx *child_ctx; + int wait_status; +}; + +static void sss_child_invoke_cb(struct tevent_context *ev, + struct tevent_immediate *imm, + void *pvt) +{ + struct sss_child_cb_pvt *cb_pvt; + struct sss_child_ctx *child_ctx; + hash_key_t key; + int error; + + cb_pvt = talloc_get_type(pvt, struct sss_child_cb_pvt); + child_ctx = cb_pvt->child_ctx; + + key.type = HASH_KEY_ULONG; + key.ul = child_ctx->pid; + + error = hash_delete(child_ctx->sigchld_ctx->children, &key); + if (error != HASH_SUCCESS && error != HASH_ERROR_KEY_NOT_FOUND) { + DEBUG(2, ("failed to delete child_ctx from hash table [%d]: %s\n", + error, hash_error_string(error))); + } + + if (child_ctx->cb) { + child_ctx->cb(child_ctx->pid, cb_pvt->wait_status, child_ctx->pvt); + } +} + +void sss_child_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) +{ + struct sss_sigchild_ctx *sigchld_ctx; + struct tevent_immediate *imm; + struct sss_child_cb_pvt *invoke_pvt; + struct sss_child_ctx *child_ctx; + hash_key_t key; + hash_value_t value; + int error; + int wait_status; + pid_t pid; + + sigchld_ctx = talloc_get_type(private_data, struct sss_sigchild_ctx); + key.type = HASH_KEY_ULONG; + + do { + do { + errno = 0; + pid = waitpid(-1, &wait_status, WNOHANG | sigchld_ctx->options); + } while (pid == -1 && errno == EINTR); + + if (pid == -1) { + DEBUG(8, ("waitpid failed [%d]: %s\n", errno, strerror(errno))); + return; + } else if (pid == 0) continue; + + key.ul = pid; + error = hash_lookup(sigchld_ctx->children, &key, &value); + if (error == HASH_SUCCESS) { + child_ctx = talloc_get_type(value.ptr, struct sss_child_ctx); + + imm = tevent_create_immediate(sigchld_ctx->ev); + if (imm == NULL) { + DEBUG(1, ("Out of memory invoking SIGCHLD callback\n")); + return; + } + + invoke_pvt = talloc_zero(child_ctx, struct sss_child_cb_pvt); + if (invoke_pvt == NULL) { + DEBUG(1, ("out of memory invoking SIGCHLD callback\n")); + return; + } + invoke_pvt->child_ctx = child_ctx; + invoke_pvt->wait_status = wait_status; + + tevent_schedule_immediate(imm, sigchld_ctx->ev, + sss_child_invoke_cb, invoke_pvt); + } else if (error == HASH_ERROR_KEY_NOT_FOUND) { + DEBUG(7, ("BUG: waitpid() returned [%d] but it was not in the " + "table. This could be due to a linked library creating " + "processes without registering them with the sigchld " + "handler\n", pid)); + /* We will simply ignore this and return to the loop + * This will prevent a zombie, but may cause unexpected + * behavior in the code that was trying to handle this + * pid. + */ + } else { + DEBUG(2, ("SIGCHLD hash table error [%d]: %s\n", + error, hash_error_string(error))); + /* This is bad, but we should try to check for other + * children anyway, to avoid potential zombies. + */ + } + } while (pid != 0); +} + +struct sss_child_ctx_old { struct tevent_signal *sige; pid_t pid; int child_status; @@ -44,11 +254,11 @@ struct sss_child_ctx { int child_handler_setup(struct tevent_context *ev, int pid, sss_child_callback_t cb, void *pvt) { - struct sss_child_ctx *child_ctx; + struct sss_child_ctx_old *child_ctx; DEBUG(8, ("Setting up signal handler up for pid [%d]\n", pid)); - child_ctx = talloc_zero(ev, struct sss_child_ctx); + child_ctx = talloc_zero(ev, struct sss_child_ctx_old); if (child_ctx == NULL) { return ENOMEM; } @@ -300,7 +510,7 @@ void child_sig_handler(struct tevent_context *ev, int count, void *__siginfo, void *pvt) { int ret, err; - struct sss_child_ctx *child_ctx; + struct sss_child_ctx_old *child_ctx; struct tevent_immediate *imm; if (count <= 0) { @@ -308,7 +518,7 @@ void child_sig_handler(struct tevent_context *ev, return; } - child_ctx = talloc_get_type(pvt, struct sss_child_ctx); + child_ctx = talloc_get_type(pvt, struct sss_child_ctx_old); DEBUG(7, ("Waiting for child [%d].\n", child_ctx->pid)); errno = 0; @@ -363,8 +573,8 @@ static void child_invoke_callback(struct tevent_context *ev, struct tevent_immediate *imm, void *pvt) { - struct sss_child_ctx *child_ctx = - talloc_get_type(pvt, struct sss_child_ctx); + struct sss_child_ctx_old *child_ctx = + talloc_get_type(pvt, struct sss_child_ctx_old); if (child_ctx->cb) { child_ctx->cb(child_ctx->child_status, child_ctx->sige, child_ctx->pvt); } diff --git a/src/providers/child_common.h b/src/providers/child_common.h index 22a77dbbe..1e9f1b6c1 100644 --- a/src/providers/child_common.h +++ b/src/providers/child_common.h @@ -45,6 +45,31 @@ struct io_buffer { size_t size; }; +/* COMMON SIGCHLD HANDLING */ +typedef void (*sss_child_fn_t)(int pid, int wait_status, void *pvt); + +struct sss_sigchild_ctx; +struct sss_child_ctx; + +/* Create a new child context to manage callbacks */ +errno_t sss_sigchld_init(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct sss_sigchild_ctx **child_ctx); + +errno_t sss_child_register(TALLOC_CTX *mem_ctx, + struct sss_sigchild_ctx *sigchld_ctx, + pid_t pid, + sss_child_fn_t cb, + void *pvt, + struct sss_child_ctx **child_ctx); + +void sss_child_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data); + /* Callback to be invoked when a sigchld handler is called. * The tevent_signal * associated with the handler will be * freed automatically when this function returns. diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 7703c2d6b..8a72994bd 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -42,6 +42,7 @@ #include "sbus/sssd_dbus.h" #include "providers/dp_backend.h" #include "providers/fail_over.h" +#include "providers/child_common.h" #include "resolv/async_resolv.h" #include "monitor/monitor_interfaces.h" @@ -1340,6 +1341,13 @@ int be_process_init(TALLOC_CTX *mem_ctx, return EIO; } + ret = sss_sigchld_init(ctx, ctx->ev, &ctx->sigchld_ctx); + if (ret != EOK) { + DEBUG(0, ("Could not initialize sigchld context: [%s]\n", + strerror(ret))); + return ret; + } + return EOK; } diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h index 27ae9d73c..9e55de700 100644 --- a/src/providers/dp_backend.h +++ b/src/providers/dp_backend.h @@ -93,6 +93,7 @@ struct be_ctx { const char *identity; const char *conf_path; struct be_failover_ctx *be_fo; + struct sss_sigchild_ctx *sigchld_ctx; /* Functions to be invoked when the * backend goes online or offline -- cgit