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

Attachment: pgpeGYFzzEVQV.pgp
Description: PGP signature

Reply via email to