From: Iliya Peregoudov <iliyap@rambler.ru>
Date: Thu, 25 Oct 2012 06:18:58 +0000 (+0400)
Subject: Create threads as joinable, not detached.
X-Git-Tag: release_2_2_1~241
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fe2066fdf9d73c8dc53cc383b0a5dc835d9bb480;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 5cd868558d5..43e455f4891 100644
--- a/src/include/radiusd.h
+++ b/src/include/radiusd.h
@@ -593,6 +593,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/event.c b/src/main/event.c
index 2f71ff339dc..d839f8063d5 100644
--- a/src/main/event.c
+++ b/src/main/event.c
@@ -3738,10 +3738,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 99ac6ea87ec..49128ee9cf0 100644
--- a/src/main/threads.c
+++ b/src/main/threads.c
@@ -131,6 +131,7 @@ typedef struct THREAD_POOL {
 	unsigned long request_count;
 	time_t time_last_spawned;
 	int cleanup_delay;
+	int stop_flag;
 	int spawn_flag;
 
 #ifdef WNOHANG
@@ -519,6 +520,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.
 		 *
@@ -621,7 +628,6 @@ static THREAD_HANDLE *spawn_thread(time_t now)
 {
 	int rcode;
 	THREAD_HANDLE *handle;
-	pthread_attr_t attr;
 
 	/*
 	 *	Ensure that we don't spawn too many threads.
@@ -644,30 +650,19 @@ static THREAD_HANDLE *spawn_thread(time_t now)
 	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.
@@ -763,6 +758,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;
 	thread_pool.spawn_flag = *spawn_flag;
 	
 	/*
@@ -777,7 +773,7 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
 		       strerror(errno));
 		return -1;
 	}
-	
+
 	/*
 	 *	Create the hash table of child PID's
 	 */
@@ -874,6 +870,40 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
 }
 
 
+/*
+ *	Stop all threads in the pool.
+ */
+void thread_pool_stop(void)
+{
+	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);
+	}
+}
+
+
 /*
  *	Assign a new request to a free thread.
  *
@@ -999,6 +1029,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);
 		}
 	}