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 5cd868558d5e304d47756867b8bfff0936bbb7d9..43e455f4891b22c8f669263c03fed9a6e49ad6b6 100644 (file)
--- 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 2f71ff339dc505c256216a493f0bef11c0d26cbe..d839f8063d5f6d3fef0499266b919cdd73c12172 100644 (file)
--- 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 99ac6ea87ec22dff55d7fdf7f615588e2ddf79db..49128ee9cf000f4288e48b77ab660921ef244778 100644 (file)
--- 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);
                }
        }