From: Wouter Wijngaards <wouter@nlnetlabs.nl>
Date: Fri, 9 Mar 2007 13:37:57 +0000 (+0000)
Subject: cleanup and unit test for alloc, also lock protection statements.
X-Git-Tag: release-0.1~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ea78ab0327c695ade2cf561026daa3f65d95da6;p=thirdparty%2Funbound.git

cleanup and unit test for alloc, also lock protection statements.


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

diff --git a/daemon/daemon.c b/daemon/daemon.c
index 2b32e26af..e6e396135 100644
--- a/daemon/daemon.c
+++ b/daemon/daemon.c
@@ -114,8 +114,9 @@ daemon_init()
 	if(!daemon)
 		return NULL;
 	signal_handling_record();
-	lock_basic_init(&daemon->lock);
+	checklock_start();
 	daemon->need_to_exit = 0;
+	alloc_init(&daemon->superalloc, NULL);
 	return daemon;	
 }
 
@@ -302,8 +303,9 @@ daemon_delete(struct daemon* daemon)
 	if(!daemon)
 		return;
 	listening_ports_free(daemon->ports);
-	lock_basic_destroy(&daemon->lock);
+	alloc_clear(&daemon->superalloc);
 	free(daemon->cwd);
 	free(daemon->pidfile);
 	free(daemon);
+	checklock_stop();
 }
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 46edecaa4..c5f26871f 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -43,6 +43,7 @@
 #define DAEMON_H
 
 #include "util/locks.h"
+#include "util/alloc.h"
 struct config_file;
 struct worker;
 struct listen_port;
@@ -52,8 +53,6 @@ struct listen_port;
  * Holds globally visible information.
  */
 struct daemon {
-	/** mutex for exclusive access to this structure. */
-	lock_basic_t lock;
 	/** The config settings */
 	struct config_file* cfg;
 	/** current working directory */
@@ -70,6 +69,8 @@ struct daemon {
 	struct worker** workers;
 	/** do we need to exit unbound (or is it only a reload?) */
 	int need_to_exit;
+	/** master allocation cache */
+	struct alloc_cache superalloc;
 };
 
 /**
diff --git a/daemon/worker.c b/daemon/worker.c
index 6e68902fb..e1c380286 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -44,6 +44,7 @@
 #include "util/net_help.h"
 #include "util/random.h"
 #include "daemon/worker.h"
+#include "daemon/daemon.h"
 #include "util/netevent.h"
 #include "util/config_file.h"
 #include "services/listen_dnsport.h"
@@ -387,6 +388,7 @@ worker_init(struct worker* worker, struct config_file *cfg,
 			fatal_exit("could not set forwarder address");
 		}
 	}
+	alloc_init(&worker->alloc, &worker->daemon->superalloc);
 	return 1;
 }
 
@@ -416,6 +418,7 @@ worker_delete(struct worker* worker)
 	if(worker->cmd_recv_fd != -1)
 		close(worker->cmd_recv_fd);
 	worker->cmd_recv_fd = -1;
+	alloc_clear(&worker->alloc);
 	free(worker);
 }
 
diff --git a/daemon/worker.h b/daemon/worker.h
index 5121bdc0d..5a8cef2a5 100644
--- a/daemon/worker.h
+++ b/daemon/worker.h
@@ -46,6 +46,7 @@
 #include "config.h"
 #include "util/netevent.h"
 #include "util/locks.h"
+#include "util/alloc.h"
 struct listen_dnsport;
 struct outside_network;
 struct config_file;
@@ -105,6 +106,8 @@ struct worker {
 	struct ub_randstate* rndstate;
 	/** do we need to restart (instead of exit) ? */
 	int need_to_restart;
+	/** allocation cache for this thread */
+	struct alloc_cache alloc;
 };
 
 /**
diff --git a/doc/Changelog b/doc/Changelog
index 83e4785a0..8e3666bfe 100644
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -2,6 +2,9 @@
 	- added rwlock writelock checking.
 	  So it will keep track of the writelock, and readlocks are enforced
 	  to not change protected memory areas.
+	- log_hex function to dump hex strings to the logfile.
+	- checklocks zeroes its destroyed lock after checking memory areas.
+	- unit test for alloc.
 
 8 March 2007: Wouter
 	- Reviewed checklock code.
diff --git a/testcode/checklocks.c b/testcode/checklocks.c
index 4ca412811..2e8de0f20 100644
--- a/testcode/checklocks.c
+++ b/testcode/checklocks.c
@@ -71,7 +71,8 @@ static void lock_error(struct checked_lock* lock,
 	log_err("At %s %s:%d", func, file, line);
 	log_err("Error for %s lock: %s",
 		(lock->type==check_lock_mutex)?"mutex": (
-		(lock->type==check_lock_spinlock)?"spinlock": "rwlock"), err);
+		(lock->type==check_lock_spinlock)?"spinlock": (
+		(lock->type==check_lock_rwlock)?"rwlock": "badtype")), err);
 	fatal_exit("bailing out");
 }
 
@@ -111,8 +112,9 @@ acquire_locklock(struct checked_lock* lock,
 
 /** add protected region */
 void 
-lock_protect(struct checked_lock* lock, void* area, size_t size)
+lock_protect(void *p, void* area, size_t size)
 {
+	struct checked_lock* lock = *(struct checked_lock**)p;
 	struct protected_area* e = (struct protected_area*)malloc(
 		sizeof(struct protected_area));
 	if(!e)
@@ -144,6 +146,8 @@ prot_check(struct checked_lock* lock,
 	struct protected_area* p = lock->prot;
 	while(p) {
 		if(memcmp(p->hold, p->region, p->size) != 0) {
+			log_hex("memory prev", p->hold, p->size);
+			log_hex("memory here", p->region, p->size);
 			lock_error(lock, func, file, line, 
 				"protected area modified");
 		}
@@ -211,6 +215,9 @@ static void
 checktype(enum check_lock_type type, struct checked_lock* lock,
         const char* func, const char* file, int line)
 {
+	if(!lock) 
+		fatal_exit("use of null/deleted lock at %s %s:%d", 
+			func, file, line);
 	if(type != lock->type) {
 		lock_error(lock, func, file, line, "wrong lock type");
 	}
@@ -232,12 +239,12 @@ checklock_destroy(enum check_lock_type type, struct checked_lock** lock,
 
 	/* check if delete is OK */
 	acquire_locklock(e, func, file, line);
-	*lock = NULL; /* use after free will fail */
 	if(e->hold_count != 0)
 		lock_error(e, func, file, line, "delete while locked.");
 	if(e->wait_count != 0)
 		lock_error(e, func, file, line, "delete while waited on.");
 	prot_check(e, func, file, line);
+	*lock = NULL; /* use after free will fail */
 	LOCKRET(pthread_mutex_unlock(&e->lock));
 
 	/* contention */
@@ -521,6 +528,7 @@ static void* checklock_main(void* arg)
 	thr->id = pthread_self();
 	/* Hack to get same numbers as in log file */
 	thr->num = *(int*)(thr->arg);
+	log_assert(thr->num < THRDEBUG_MAX_THREADS);
 	log_assert(thread_infos[thr->num] == NULL);
 	thread_infos[thr->num] = thr;
 	LOCKRET(pthread_setspecific(thr_debug_key, thr));
@@ -530,14 +538,9 @@ static void* checklock_main(void* arg)
 	return ret;
 }
 
-/** allocate debug info and create thread */
-void 
-checklock_thrcreate(pthread_t* id, void* (*func)(void*), void* arg)
+/** init the main thread */
+void checklock_start()
 {
-	struct thr_check* thr = (struct thr_check*)calloc(1, 
-		sizeof(struct thr_check));
-	if(!thr)
-		fatal_exit("thrcreate: out of memory");
 	if(!key_created) {
 		struct thr_check* thisthr = (struct thr_check*)calloc(1, 
 			sizeof(struct thr_check));
@@ -548,6 +551,34 @@ checklock_thrcreate(pthread_t* id, void* (*func)(void*), void* arg)
 		LOCKRET(pthread_setspecific(thr_debug_key, thisthr));
 		thread_infos[0] = thisthr;
 	}
+}
+
+/** stop checklocks */
+void checklock_stop()
+{
+	if(key_created) {
+		int i;
+		free(thread_infos[0]);
+		thread_infos[0] = NULL;
+		for(i = 0; i < THRDEBUG_MAX_THREADS; i++)
+			log_assert(thread_infos[i] == NULL);
+			/* should have been cleaned up. */
+		LOCKRET(pthread_key_delete(thr_debug_key));
+		key_created = 0;
+	}
+}
+
+/** allocate debug info and create thread */
+void 
+checklock_thrcreate(pthread_t* id, void* (*func)(void*), void* arg)
+{
+	struct thr_check* thr = (struct thr_check*)calloc(1, 
+		sizeof(struct thr_check));
+	if(!thr)
+		fatal_exit("thrcreate: out of memory");
+	if(!key_created) {
+		checklock_start();
+	}
 	thr->func = func;
 	thr->arg = arg;
 	LOCKRET(pthread_create(id, NULL, checklock_main, thr));
diff --git a/testcode/checklocks.h b/testcode/checklocks.h
index 44f17160c..8170e1333 100644
--- a/testcode/checklocks.h
+++ b/testcode/checklocks.h
@@ -171,7 +171,8 @@ struct checked_lock {
 /**
  * Additional call for the user to specify what areas are protected
  * @param lock: the lock that protects the area. It can be inside the area.
- *	The lock must be inited.
+ *	The lock must be inited. Call with user lock. (any type).
+ *	It demangles the lock itself (struct checked_lock**).
  * @param area: ptr to mem.
  * @param size: length of area.
  * You can call it multiple times with the same lock to give several areas.
@@ -179,7 +180,17 @@ struct checked_lock {
  * at this time and protected right away against unauthorised changes until 
  * the next lock() call is done.
  */
-void lock_protect(struct checked_lock* lock, void* area, size_t size);
+void lock_protect(void* lock, void* area, size_t size);
+
+/**
+ * Initialise checklock. Sets up internal debug structures.
+ */
+void checklock_start();
+
+/**
+ * Cleanup internal debug state.
+ */
+void checklock_stop();
 
 /**
  * Init locks.
diff --git a/testcode/unitmain.c b/testcode/unitmain.c
index 5dd045c87..cf658aa1d 100644
--- a/testcode/unitmain.c
+++ b/testcode/unitmain.c
@@ -47,6 +47,46 @@ int testcount = 0;
 /** test bool x, exits on failure, increases testcount. */
 #define unit_assert(x) testcount++; log_assert(x);
 
+#include "util/alloc.h"
+/** test alloc code */
+static void
+alloc_test() {
+	alloc_special_t *t1, *t2;
+	struct alloc_cache major, minor1, minor2;
+	int i;
+
+	checklock_start();
+	alloc_init(&major, NULL);
+	alloc_init(&minor1, &major);
+	alloc_init(&minor2, &major);
+
+	t1 = alloc_special_obtain(&minor1);
+	alloc_clear(&minor1);
+
+	alloc_special_release(&minor2, t1);
+	t2 = alloc_special_obtain(&minor2);
+	unit_assert( t1 == t2 ); /* reused */
+	alloc_special_release(&minor2, t1);
+
+	for(i=0; i<100; i++) {
+		t1 = alloc_special_obtain(&minor1);
+		alloc_special_release(&minor2, t1);
+	}
+	if(0) {
+		alloc_stats(&minor1);
+		alloc_stats(&minor2);
+		alloc_stats(&major);
+	}
+	/* reuse happened */
+	unit_assert(minor1.num_quar + minor2.num_quar + major.num_quar == 11);
+
+	alloc_clear(&minor1);
+	alloc_clear(&minor2);
+	unit_assert(major.num_quar == 11);
+	alloc_clear(&major);
+	checklock_stop();
+}
+
 #include "util/net_help.h"
 /** test net code */
 static void 
@@ -78,6 +118,7 @@ main(int argc, char* argv[])
 	}
 	printf("Start of %s unit test.\n", PACKAGE_STRING);
 	net_test();
+	alloc_test();
 	printf("%d tests succeeded\n", testcount);
 	return 0;
 }
diff --git a/util/alloc.c b/util/alloc.c
index ff0fc9528..7f6ad7566 100644
--- a/util/alloc.c
+++ b/util/alloc.c
@@ -65,16 +65,20 @@ alloc_init(struct alloc_cache* alloc, struct alloc_cache* super)
 {
 	memset(alloc, 0, sizeof(*alloc));
 	alloc->super = super;
-	lock_quick_init(&alloc->lock);
+	if(!alloc->super) {
+		lock_quick_init(&alloc->lock);
+		lock_protect(&alloc->lock, alloc, sizeof(*alloc));
+	}
 }
 
 void 
-alloc_delete(struct alloc_cache* alloc)
+alloc_clear(struct alloc_cache* alloc)
 {
 	alloc_special_t* p, *np;
 	if(!alloc)
 		return;
-	lock_quick_destroy(&alloc->lock);
+	if(!alloc->super)
+		lock_quick_destroy(&alloc->lock);
 	if(alloc->super && alloc->quar) {
 		/* push entire list into super */
 		p = alloc->quar;
@@ -108,7 +112,6 @@ alloc_special_obtain(struct alloc_cache* alloc)
 		p = alloc->quar;
 		alloc->quar = alloc_special_next(p);
 		alloc->num_quar--;
-		alloc->special_allocated++;
 		alloc_special_clean(p);
 		return p;
 	}
@@ -123,7 +126,6 @@ alloc_special_obtain(struct alloc_cache* alloc)
 		}
 		lock_quick_unlock(&alloc->super->lock);
 		if(p) {
-			alloc->special_allocated++;
 			alloc_special_clean(p);
 			return p;
 		}
@@ -132,7 +134,6 @@ alloc_special_obtain(struct alloc_cache* alloc)
 	prealloc(alloc);
 	if(!(p = (alloc_special_t*)malloc(sizeof(alloc_special_t))))
 		fatal_exit("alloc_special_obtain: out of memory");
-	alloc->special_allocated++;
 	alloc_special_clean(p);
 	return p;
 }
@@ -172,7 +173,6 @@ alloc_special_release(struct alloc_cache* alloc, alloc_special_t* mem)
 	alloc_special_clean(mem);
 	if(alloc->super && alloc->num_quar >= ALLOC_SPECIAL_MAX) {
 		/* push it to the super structure */
-		alloc->special_allocated --;
 		pushintosuper(alloc, mem);
 		return;
 	}
@@ -180,12 +180,11 @@ alloc_special_release(struct alloc_cache* alloc, alloc_special_t* mem)
 	alloc_special_next(mem) = alloc->quar;
 	alloc->quar = mem;
 	alloc->num_quar++;
-	alloc->special_allocated--;
 }
 
 void 
 alloc_stats(struct alloc_cache* alloc)
 {
-	log_info("%salloc: %d allocated, %d in cache.", alloc->super?"":"sup",
-		(int)alloc->special_allocated, (int)alloc->num_quar);
+	log_info("%salloc: %d in cache.", alloc->super?"":"sup",
+		(int)alloc->num_quar);
 }
diff --git a/util/alloc.h b/util/alloc.h
index 05f7930f9..d261b6147 100644
--- a/util/alloc.h
+++ b/util/alloc.h
@@ -73,9 +73,6 @@ struct alloc_cache {
 	alloc_special_t* quar;
 	/** number of items in quarantine. */
 	size_t num_quar;
-
-	/** number of special type allocated */
-	size_t special_allocated;
 };
 
 /**
@@ -92,7 +89,7 @@ void alloc_init(struct alloc_cache* alloc, struct alloc_cache* super);
  * Does not free the alloc struct itself (it was also allocated by caller).
  * @param alloc: is almost zeroed on exit (except some stats).
  */
-void alloc_delete(struct alloc_cache* alloc);
+void alloc_clear(struct alloc_cache* alloc);
 
 /**
  * Get a new special_t element.
diff --git a/util/locks.h b/util/locks.h
index 408f14a33..fb99a79db 100644
--- a/util/locks.h
+++ b/util/locks.h
@@ -77,6 +77,8 @@
 
 #else /* USE_THREAD_DEBUG */
 #define lock_protect(lock, area, size) /* nop */
+#define checklock_start() /* nop */
+#define checklock_stop() /* nop */
 
 #ifdef HAVE_PTHREAD
 #include <pthread.h>
diff --git a/util/log.c b/util/log.c
index 81784df89..146fa8fa7 100644
--- a/util/log.c
+++ b/util/log.c
@@ -166,3 +166,18 @@ verbose(enum verbosity_value level, const char* format, ...)
 	va_end(args);
 }
 
+void 
+log_hex(const char* msg, void* data, size_t length)
+{
+	size_t i;
+	uint8_t* data8 = (uint8_t*)data;
+	const char const* hexchar = "0123456789ABCDEF";
+	char* buf = malloc(length*2 + 1); /* alloc hex chars + \0 */
+	for(i=0; i<length; i++) {
+		buf[i*2] = hexchar[ data8[i] >> 4 ];
+		buf[i*2 + 1] = hexchar[ data8[i] & 0xF ];
+	}
+	buf[length*2] = 0;
+	log_info("%s[%d] %s", msg, length, buf);
+	free(buf);
+}
diff --git a/util/log.h b/util/log.h
index 5b58f4c74..c6fd5abf5 100644
--- a/util/log.h
+++ b/util/log.h
@@ -109,6 +109,15 @@ void log_err(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
  */
 void log_warn(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
 
+/**
+ * Log a hex-string to the log. Can be any length.
+ * performs mallocs to do so, slow. But debug useful.
+ * @param msg: string desc to accompany the hexdump.
+ * @param data: data to dump in hex format.
+ * @param length: length of data.
+ */
+void log_hex(const char* msg, void* data, size_t length);
+
 /**
  * Log fatal error message, and exit the current process.
  * Pass printf formatted arguments. No trailing newline is needed.