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 700c105ba2a95ef3e3d535f423864206a86878bf..3248a8de8e2710e420dd393ba1351789099b2084 100644 (file)
--- 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 80fc0f7008a31d8af8a51869c1f130f2ff6bf0d1..742aeee568ea2641cc99bc8ae305e1e214dd89a1 100644 (file)
--- 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 6b892b10e1b696b9e2d7b6cf85573248397353eb..6fe964f023fe06ac03b7e64860862fb841e102ed 100644 (file)
--- 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);
                }
        }