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 
 #include 
@@ -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_h