From: Wouter Wijngaards <wouter@nlnetlabs.nl>
Date: Fri, 25 Jan 2008 15:45:54 +0000 (+0000)
Subject: more fixes, more tests.
X-Git-Tag: release-0.9~37
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=32396dc6770caeefbac6fef7d760befd3c3fc74c;p=thirdparty%2Funbound.git

more fixes, more tests.


git-svn-id: file:///svn/unbound/trunk@903 be551aaa-1e26-0410-a405-d3ace91eadb9
---

diff --git a/doc/Changelog b/doc/Changelog
index dfa6297cf..f4fcc428e 100644
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -11,6 +11,10 @@
 	- fix pass async_id=NULL to _async resolve().
 	- rewrote _wait() routine, so that it is threadsafe.
 	- cancelation is threadsafe.
+	- asynclook extended test in tpkg.
+	- fixed two races where forked bg process waits for (somehow shared?)
+	  locks, so does not service the query pipe on the bg side.
+	  Now those locks are only held for fg_threads and for bg_as_a_thread.
 
 24 January 2008: Wouter
 	- tested the cancel() function.
diff --git a/libunbound/context.c b/libunbound/context.c
index c1436121f..26a03c8ed 100644
--- a/libunbound/context.c
+++ b/libunbound/context.c
@@ -162,16 +162,20 @@ context_new(struct ub_val_ctx* ctx, char* name, int rrtype, int rrclass,
 }
 
 struct alloc_cache* 
-context_obtain_alloc(struct ub_val_ctx* ctx)
+context_obtain_alloc(struct ub_val_ctx* ctx, int locking)
 {
 	struct alloc_cache* a;
 	int tnum = 0;
-	lock_basic_lock(&ctx->cfglock);
+	if(locking) {
+		lock_basic_lock(&ctx->cfglock);
+	}
 	a = ctx->alloc_list;
 	if(a)
 		ctx->alloc_list = a->super; /* snip off list */
 	else	tnum = ctx->thr_next_num++;
-	lock_basic_unlock(&ctx->cfglock);
+	if(locking) {
+		lock_basic_unlock(&ctx->cfglock);
+	}
 	if(a) {
 		a->super = &ctx->superalloc;
 		return a;
@@ -184,14 +188,19 @@ context_obtain_alloc(struct ub_val_ctx* ctx)
 }
 
 void 
-context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc)
+context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc,
+	int locking)
 {
 	if(!ctx || !alloc)
 		return;
-	lock_basic_lock(&ctx->cfglock);
+	if(locking) {
+		lock_basic_lock(&ctx->cfglock);
+	}
 	alloc->super = ctx->alloc_list;
 	ctx->alloc_list = alloc;
-	lock_basic_unlock(&ctx->cfglock);
+	if(locking) {
+		lock_basic_unlock(&ctx->cfglock);
+	}
 }
 
 uint8_t* 
diff --git a/libunbound/context.h b/libunbound/context.h
index 5be7dc8ac..80b10d967 100644
--- a/libunbound/context.h
+++ b/libunbound/context.h
@@ -231,16 +231,19 @@ struct ctx_query* context_new(struct ub_val_ctx* ctx, char* name, int rrtype,
 /**
  * Get a new alloc. Creates a new one or uses a cached one.
  * @param ctx: context
+ * @param locking: if true, cfglock is locked while getting alloc.
  * @return an alloc, or NULL on mem error.
  */
-struct alloc_cache* context_obtain_alloc(struct ub_val_ctx* ctx);
+struct alloc_cache* context_obtain_alloc(struct ub_val_ctx* ctx, int locking);
 
 /**
  * Release an alloc. Puts it into the cache.
  * @param ctx: context
+ * @param locking: if true, cfglock is locked while releasing alloc.
  * @param alloc: alloc to relinquish.
  */
-void context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc);
+void context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc,
+	int locking);
 
 /**
  * Serialize a context query that questions data.
diff --git a/libunbound/worker.c b/libunbound/worker.c
index c387b7f74..feb1985b7 100644
--- a/libunbound/worker.c
+++ b/libunbound/worker.c
@@ -74,7 +74,8 @@ libworker_delete(struct libworker* w)
 	if(!w) return;
 	if(w->env) {
 		mesh_delete(w->env->mesh);
-		context_release_alloc(w->ctx, w->env->alloc);
+		context_release_alloc(w->ctx, w->env->alloc, 
+			!w->is_bg || w->is_bg_thread);
 		ldns_buffer_free(w->env->scratch_buffer);
 		regional_destroy(w->env->scratch);
 		ub_randfree(w->env->rnd);
@@ -90,12 +91,13 @@ libworker_delete(struct libworker* w)
 
 /** setup fresh libworker struct */
 static struct libworker*
-libworker_setup(struct ub_val_ctx* ctx)
+libworker_setup(struct ub_val_ctx* ctx, int is_bg)
 {
 	unsigned int seed;
 	struct libworker* w = (struct libworker*)calloc(1, sizeof(*w));
 	struct config_file* cfg = ctx->env->cfg;
 	if(!w) return NULL;
+	w->is_bg = is_bg;
 	w->ctx = ctx;
 	w->env = (struct module_env*)malloc(sizeof(*w->env));
 	if(!w->env) {
@@ -103,7 +105,7 @@ libworker_setup(struct ub_val_ctx* ctx)
 		return NULL;
 	}
 	*w->env = *ctx->env;
-	w->env->alloc = context_obtain_alloc(ctx);
+	w->env->alloc = context_obtain_alloc(ctx, !w->is_bg || w->is_bg_thread);
 	if(!w->env->alloc) {
 		libworker_delete(w);
 		return NULL;
@@ -125,16 +127,20 @@ libworker_setup(struct ub_val_ctx* ctx)
 	seed = (unsigned int)time(NULL) ^ (unsigned int)getpid() ^
 		(((unsigned int)w->thread_num)<<17);
 	seed ^= (unsigned int)w->env->alloc->next_id;
-	lock_basic_lock(&ctx->cfglock);
-	/* Openssl RAND_... functions are not as threadsafe as documented,
-	 * put a lock around them. */
+	if(!w->is_bg || w->is_bg_thread) {
+		/* Openssl RAND_... functions are not as threadsafe 
+		 * as documented, put a lock around them. */
+		lock_basic_lock(&ctx->cfglock);
+	}
 	if(!ub_initstate(seed, w->env->rnd, RND_STATE_SIZE)) {
 		lock_basic_unlock(&ctx->cfglock);
 		seed = 0;
 		libworker_delete(w);
 		return NULL;
 	}
-	lock_basic_unlock(&ctx->cfglock);
+	if(!w->is_bg || w->is_bg_thread) {
+		lock_basic_unlock(&ctx->cfglock);
+	}
 	seed = 0;
 
 	w->base = comm_base_create();
@@ -311,7 +317,7 @@ int libworker_bg(struct ub_val_ctx* ctx)
 	lock_basic_lock(&ctx->cfglock);
 	if(ctx->dothread) {
 		lock_basic_unlock(&ctx->cfglock);
-		w = libworker_setup(ctx);
+		w = libworker_setup(ctx, 1);
 		w->is_bg_thread = 1;
 #ifdef ENABLE_LOCK_CHECKS
 		w->thread_num = 1; /* for nicer DEBUG checklocks */
@@ -322,7 +328,7 @@ int libworker_bg(struct ub_val_ctx* ctx)
 		lock_basic_unlock(&ctx->cfglock);
 		switch((ctx->bg_pid=fork())) {
 			case 0:
-				w = libworker_setup(ctx);
+				w = libworker_setup(ctx, 1);
 				if(!w) fatal_exit("out of memory");
 				/* close non-used parts of the pipes */
 				close(ctx->qqpipe[1]);
@@ -490,7 +496,7 @@ setup_qinfo_edns(struct libworker* w, struct ctx_query* q,
 
 int libworker_fg(struct ub_val_ctx* ctx, struct ctx_query* q)
 {
-	struct libworker* w = libworker_setup(ctx);
+	struct libworker* w = libworker_setup(ctx, 0);
 	uint16_t qflags, qid;
 	struct query_info qinfo;
 	struct edns_data edns;
diff --git a/libunbound/worker.h b/libunbound/worker.h
index f1969b99d..275f10463 100644
--- a/libunbound/worker.h
+++ b/libunbound/worker.h
@@ -68,6 +68,8 @@ struct libworker {
 	/** context we are operating under */
 	struct ub_val_ctx* ctx;
 
+	/** is this the bg worker? */
+	int is_bg;
 	/** is this a bg worker that is threaded (not forked)? */
 	int is_bg_thread;
 
diff --git a/testdata/05-asynclook.tpkg b/testdata/05-asynclook.tpkg
index 4e4b5b6d8..e1d7d555c 100644
Binary files a/testdata/05-asynclook.tpkg and b/testdata/05-asynclook.tpkg differ