Sascha Wilde <[EMAIL PROTECTED]> writes: > Thanks! Seems indeed helpful. I have changed passdb-checkpassword to > use the child-wait stuff, see attached patch. (I have put > child-wait.[ch] into src/lib/)
Doh! Forgot to attach the patch (which isn't to bad as it was faulty anyway...). This time I really attached it! > Now I'll try to do the same for my userdb, so that they should work at > the same time -- stay tuned... I have a first working beta. I'll put a repository with the changes on line soon... cheers sascha -- Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
diff -r cd9fb9098f07 src/auth/auth.c --- a/src/auth/auth.c Wed Oct 15 12:53:05 2008 +0200 +++ b/src/auth/auth.c Wed Oct 15 13:24:49 2008 +0200 @@ -1,6 +1,7 @@ /* Copyright (c) 2005-2008 Dovecot authors, see the included COPYING file */ #include "common.h" +#include "child-wait.h" #include "network.h" #include "buffer.h" #include "str.h" @@ -32,6 +33,8 @@ auth->verbose_debug = getenv("VERBOSE_DEBUG") != NULL || auth->verbose_debug_passwords; auth->verbose = getenv("VERBOSE") != NULL || auth->verbose_debug; + + child_wait_init(); passdb_p = &auth->passdbs; masterdb_p = &auth->masterdbs; @@ -297,5 +300,6 @@ auth_request_handler_deinit(); passdb_cache_deinit(); + child_wait_deinit(); pool_unref(&auth->pool); } diff -r cd9fb9098f07 src/auth/passdb-checkpassword.c --- a/src/auth/passdb-checkpassword.c Wed Oct 15 12:53:05 2008 +0200 +++ b/src/auth/passdb-checkpassword.c Wed Oct 15 13:24:49 2008 +0200 @@ -12,6 +12,7 @@ #include "hash.h" #include "env-util.h" #include "safe-memset.h" +#include "child-wait.h" #include <stdlib.h> #include <unistd.h> @@ -39,6 +40,9 @@ int exit_status; unsigned int exited:1; }; + +struct child_wait *checkpassword_passdb_children; + static void checkpassword_request_close(struct chkpw_auth_request *request) { @@ -142,58 +146,41 @@ } } -static void sigchld_handler(int signo ATTR_UNUSED, void *context) +static void sigchld_handler(struct child_wait_status *child_wait_status, void *context) { + int status = child_wait_status->status; + pid_t pid = child_wait_status->pid; struct checkpassword_passdb_module *module = context; - struct chkpw_auth_request *request; - int status; - pid_t pid; + struct chkpw_auth_request *request = + hash_lookup(module->clients, POINTER_CAST(pid)); - /* FIXME: if we ever do some other kind of forking, this needs fixing */ - while ((pid = waitpid(-1, &status, WNOHANG)) != 0) { - if (pid == -1) { - if (errno != ECHILD && errno != EINTR) - i_error("waitpid() failed: %m"); - return; - } + if (request == NULL) { + i_error("checkpassword: sighandler called for unknown child %d", pid); + return; + } - request = hash_lookup(module->clients, POINTER_CAST(pid)); - if (request == NULL) { - /* unknown child finished */ - if (WIFSIGNALED(status)) { - i_error("checkpassword: Unknown child %s died " - "with signal %d", dec2str(pid), - WTERMSIG(status)); - } - continue; - } + if (WIFSIGNALED(status)) { + i_error("checkpassword: Child %s died with signal %d", + dec2str(pid), WTERMSIG(status)); + } else if (WIFEXITED(status)) { + request->exited = TRUE; + request->exit_status = WEXITSTATUS(status); - if (WIFSIGNALED(status)) { - i_error("checkpassword: Child %s died with signal %d", - dec2str(pid), WTERMSIG(status)); - } else if (WIFEXITED(status)) { - request->exited = TRUE; - request->exit_status = WEXITSTATUS(status); + auth_request_log_debug(request->request, + "checkpassword", "exit_status=%d", + request->exit_status); - auth_request_log_debug(request->request, - "checkpassword", "exit_status=%d", - request->exit_status); - - checkpassword_request_half_finish(request); - request = NULL; - } else { - /* shouldn't happen */ - auth_request_log_debug(request->request, - "checkpassword", "Child exited with status=%d", - status); - } - - if (request != NULL) { - checkpassword_request_finish(request, - PASSDB_RESULT_INTERNAL_FAILURE); - } + checkpassword_request_half_finish(request); + request = NULL; + } else { + /* shouldn't happen */ + auth_request_log_debug(request->request, + "checkpassword", "Child exited with status=%d", + status); + checkpassword_request_finish(request, PASSDB_RESULT_INTERNAL_FAILURE); } } + static void env_put_extra_fields(const char *extra_fields) { @@ -424,6 +411,13 @@ chkpw_auth_request); hash_insert(module->clients, POINTER_CAST(pid), chkpw_auth_request); + + if (checkpassword_passdb_children) { + child_wait_add_pid(checkpassword_passdb_children, pid); + } else { + checkpassword_passdb_children = + child_wait_new_with_pid(pid, sigchld_handler, module); + } } static struct passdb_module * @@ -443,20 +437,12 @@ return &module->module; } -static void checkpassword_init(struct passdb_module *module, - const char *args ATTR_UNUSED) -{ - lib_signals_set_handler(SIGCHLD, TRUE, sigchld_handler, module); -} - static void checkpassword_deinit(struct passdb_module *_module) { struct checkpassword_passdb_module *module = (struct checkpassword_passdb_module *)_module; struct hash_iterate_context *iter; void *key, *value; - - lib_signals_unset_handler(SIGCHLD, sigchld_handler, module); iter = hash_iterate_init(module->clients); while (hash_iterate(iter, &key, &value)) { @@ -465,13 +451,18 @@ } hash_iterate_deinit(&iter); hash_destroy(&module->clients); + + if (checkpassword_passdb_children) { + child_wait_free(checkpassword_passdb_children); + checkpassword_passdb_children = NULL; + } } struct passdb_module_interface passdb_checkpassword = { "checkpassword", checkpassword_preinit, - checkpassword_init, + NULL, checkpassword_deinit, checkpassword_verify_plain, diff -r cd9fb9098f07 src/lib/Makefile.am --- a/src/lib/Makefile.am Wed Oct 15 12:53:05 2008 +0200 +++ b/src/lib/Makefile.am Wed Oct 15 13:24:49 2008 +0200 @@ -15,6 +15,7 @@ base64.c \ bsearch-insert-pos.c \ buffer.c \ + child-wait.c \ close-keep-errno.c \ compat.c \ crc32.c \ @@ -115,6 +116,7 @@ base64.h \ bsearch-insert-pos.h \ buffer.h \ + child-wait.h \ close-keep-errno.h \ compat.h \ crc32.h \ diff -r cd9fb9098f07 src/lib/child-wait.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/lib/child-wait.c Wed Oct 15 13:24:49 2008 +0200 @@ -0,0 +1,108 @@ +/* Copyright (c) 2007-2008 Icecap authors, see the included COPYING file */ + +#include "lib.h" +#include "lib-signals.h" +#include "hash.h" +#include "child-wait.h" + +#include <sys/wait.h> + +struct child_wait { + unsigned int pid_count; + + child_wait_callback_t *callback; + void *context; +}; + +struct hash_table *child_pids; + +#undef child_wait_new_with_pid +struct child_wait * +child_wait_new_with_pid(pid_t pid, child_wait_callback_t *callback, + void *context) +{ + struct child_wait *wait; + + wait = i_new(struct child_wait, 1); + wait->callback = callback; + wait->context = context; + + if (pid != (pid_t)-1) + child_wait_add_pid(wait, pid); + return wait; +} + +void child_wait_free(struct child_wait **_wait) +{ + struct child_wait *wait = *_wait; + struct hash_iterate_context *iter; + void *key, *value; + + *_wait = NULL; + + if (wait->pid_count > 0) { + /* this should be rare, so iterating hash is fast enough */ + iter = hash_iterate_init(child_pids); + while (hash_iterate(iter, &key, &value)) { + if (value == wait) { + hash_remove(child_pids, key); + if (--wait->pid_count == 0) + break; + } + } + hash_iterate_deinit(&iter); + } + + i_free(wait); +} + +void child_wait_add_pid(struct child_wait *wait, pid_t pid) +{ + wait->pid_count++; + hash_insert(child_pids, POINTER_CAST(pid), wait); +} + +void child_wait_remove_pid(struct child_wait *wait, pid_t pid) +{ + wait->pid_count--; + hash_remove(child_pids, POINTER_CAST(pid)); +} + +static void +sigchld_handler(int signo ATTR_UNUSED, void *context ATTR_UNUSED) +{ + struct child_wait_status status; + + while ((status.pid = waitpid(-1, &status.status, WNOHANG)) > 0) { + status.wait = hash_lookup(child_pids, POINTER_CAST(status.pid)); + if (status.wait != NULL) { + child_wait_remove_pid(status.wait, status.pid); + status.wait->callback(&status, status.wait->context); + } + } + + if (status.pid == -1 && errno != EINTR && errno != ECHILD) + i_error("waitpid() failed: %m"); +} + +void child_wait_init(void) +{ + child_pids = hash_create(default_pool, default_pool, 0, NULL, NULL); + + lib_signals_set_handler(SIGCHLD, TRUE, sigchld_handler, NULL); +} + +void child_wait_deinit(void) +{ + struct hash_iterate_context *iter; + void *key, *value; + + lib_signals_unset_handler(SIGCHLD, sigchld_handler, NULL); + + iter = hash_iterate_init(child_pids); + while (hash_iterate(iter, &key, &value)) + i_free(value); + hash_iterate_deinit(&iter); + + hash_destroy(&child_pids); +} diff -r cd9fb9098f07 src/lib/child-wait.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/lib/child-wait.h Wed Oct 15 13:24:49 2008 +0200 @@ -0,0 +1,36 @@ +#ifndef CHILD_WAIT_H +#define CHILD_WAIT_H + +struct child_wait_status { + struct child_wait *wait; + + pid_t pid; + int status; +}; + +typedef void child_wait_callback_t(const struct child_wait_status *status, + void *context); + +struct child_wait * +child_wait_new_with_pid(pid_t pid, child_wait_callback_t *callback, + void *context); +#ifdef CONTEXT_TYPE_SAFETY +# define child_wait_new_with_pid(pid, callback, context) \ + ({(void)(1 ? 0 : callback((const struct child_wait_status *)0, \ + context)); \ + child_wait_new_with_pid(pid, (child_wait_callback_t *)callback, context); }) +#else +# define child_wait_new_with_pid(pid, callback, context) \ + child_wait_new_with_pid(pid, (child_wait_callback_t *)callback, context) +#endif +#define child_wait_new(callback, context) \ + child_wait_new_with_pid((pid_t)-1, callback, context) +void child_wait_free(struct child_wait **wait); + +void child_wait_add_pid(struct child_wait *wait, pid_t pid); +void child_wait_remove_pid(struct child_wait *wait, pid_t pid); + +void child_wait_init(void); +void child_wait_deinit(void); + +#endif
pgpeGYFzzEVQV.pgp
Description: PGP signature