From: Iliya Peregoudov <iliyap@rambler.ru>
Date: Thu, 25 Oct 2012 07:29:42 +0000 (+0400)
Subject: Create threads as joinable, not detached.
X-Git-Tag: release_3_0_0_beta1~1650
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3758d9b1c7636571f32365cf0e470e500cfa9bfa;p=thirdparty%2Ffreeradius-server.git

Create threads as joinable, not detached.
Stop and join all threads before detaching modules.

This prevents a crash on exit where the modules are free'd before
the threads stop using them.
---

diff --git a/src/include/radiusd.h b/src/include/radiusd.h
index 700c105ba2a..3248a8de8e2 100644
--- a/src/include/radiusd.h
+++ b/src/include/radiusd.h
@@ -713,6 +713,7 @@ void		xlat_free(void);
 
 /* threads.c */
 extern		int thread_pool_init(CONF_SECTION *cs, int *spawn_flag);
+extern		void thread_pool_stop(void);
 extern		int thread_pool_addrequest(REQUEST *, RAD_REQUEST_FUNP);
 extern		pid_t rad_fork(void);
 extern		pid_t rad_waitpid(pid_t pid, int *status);
diff --git a/src/main/process.c b/src/main/process.c
index 80fc0f7008a..742aeee568e 100644
--- a/src/main/process.c
+++ b/src/main/process.c
@@ -4109,10 +4109,11 @@ static int proxy_hash_cb(UNUSED void *ctx, void *data)
 void radius_event_free(void)
 {
 	/*
-	 *	FIXME: Stop all threads, or at least check that
-	 *	they're all waiting on the semaphore, and the queues
-	 *	are empty.
+	 *	Stop and join all threads.
 	 */
+#ifdef HAVE_PTHREAD_H
+	thread_pool_stop();
+#endif
 
 #ifdef WITH_PROXY
 	/*
diff --git a/src/main/threads.c b/src/main/threads.c
index 6b892b10e1b..6fe964f023f 100644
--- a/src/main/threads.c
+++ b/src/main/threads.c
@@ -141,6 +141,7 @@ typedef struct THREAD_POOL {
 	unsigned long request_count;
 	time_t time_last_spawned;
 	int cleanup_delay;
+	int stop_flag;
 #endif	/* WITH_GCD */
 	int spawn_flag;
 
@@ -586,6 +587,12 @@ static void *request_handler_thread(void *arg)
 		ERR_clear_error ();
 #endif
 
+		/*
+		 *	The server is exiting.  Don't dequeue any
+		 *	requests.
+		 */
+		if (thread_pool.stop_flag) break;
+
 		/*
 		 *	Try to grab a request from the queue.
 		 *
@@ -714,7 +721,6 @@ static THREAD_HANDLE *spawn_thread(time_t now, int do_trigger)
 {
 	int rcode;
 	THREAD_HANDLE *handle;
-	pthread_attr_t attr;
 
 	/*
 	 *	Ensure that we don't spawn too many threads.
@@ -738,30 +744,19 @@ static THREAD_HANDLE *spawn_thread(time_t now, int do_trigger)
 	handle->timestamp = time(NULL);
 
 	/*
-	 *	Initialize the thread's attributes to detached.
-	 *
-	 *	We could call pthread_detach() later, but if the thread
-	 *	exits between the create & detach calls, it will need to
-	 *	be joined, which will never happen.
-	 */
-	pthread_attr_init(&attr);
-	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-
-	/*
-	 *	Create the thread detached, so that it cleans up it's
-	 *	own memory when it exits.
+	 *	Create the thread joinable, so that it can be cleaned up
+	 *	using pthread_join().
 	 *
 	 *	Note that the function returns non-zero on error, NOT
 	 *	-1.  The return code is the error, and errno isn't set.
 	 */
-	rcode = pthread_create(&handle->pthread_id, &attr,
+	rcode = pthread_create(&handle->pthread_id, 0,
 			request_handler_thread, handle);
 	if (rcode != 0) {
 		radlog(L_ERR, "Thread create failed: %s",
 		       strerror(rcode));
 		return NULL;
 	}
-	pthread_attr_destroy(&attr);
 
 	/*
 	 *	One more thread to go into the list.
@@ -850,6 +845,7 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
 	thread_pool.total_threads = 0;
 	thread_pool.max_thread_num = 1;
 	thread_pool.cleanup_delay = 5;
+	thread_pool.stop_flag = 0;
 #endif
 	thread_pool.spawn_flag = *spawn_flag;
 	
@@ -865,7 +861,7 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
 		       strerror(errno));
 		return -1;
 	}
-	
+
 	/*
 	 *	Create the hash table of child PID's
 	 */
@@ -977,6 +973,42 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
 }
 
 
+/*
+ *	Stop all threads in the pool.
+ */
+void thread_pool_stop(void)
+{
+#ifndef WITH_GCD
+	int i;
+	int total_threads;
+	THREAD_HANDLE *handle;
+	THREAD_HANDLE *next;
+
+	/*
+	 *	Set pool stop flag.
+	 */
+	thread_pool.stop_flag = 1;
+
+	/*
+	 *	Wakeup all threads to make them see stop flag.
+	 */
+	total_threads = thread_pool.total_threads;
+	for (i = 0; i != total_threads; i++) {
+		sem_post(&thread_pool.semaphore);
+	}
+
+	/*
+	 *	Join and free all threads.
+	 */
+	for (handle = thread_pool.head; handle; handle = next) {
+		next = handle->next;
+		pthread_join(handle->pthread_id, NULL);
+		delete_thread(handle);
+	}
+#endif
+}
+
+
 #ifdef WITH_GCD
 int request_enqueue(REQUEST *request)
 {
@@ -1073,6 +1105,7 @@ static void thread_pool_manage(time_t now)
 		 *	has agreed.
 		 */
 		if (handle->status == THREAD_EXITED) {
+			pthread_join(handle->pthread_id, NULL);
 			delete_thread(handle);
 		}
 	}