Re: [Dovecot] Patch: use child_wait in passdb-checkpassword

2008-10-15 Thread Sascha Wilde
Sascha Wilde [EMAIL PROTECTED] writes:
 Sascha Wilde [EMAIL PROTECTED] writes:
 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...

Done.  You can find all the stuff here:
http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/

The addition of child_wait and changes to passdb-checkpassword are in
changeset 53b57b123f74, the new userdb back end is in a4d3ea1e52bc.

Next steps will be cleaning up and documentation (at least in the sample
config).

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


pgpYTM1H2c3ZA.pgp
Description: PGP signature


Re: [Dovecot] Patch: use child_wait in passdb-checkpassword

2008-10-15 Thread Sascha Wilde
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