]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
thread: Don't hold mutex when calling cleanup handlers while terminating
authorTobias Brunner <tobias@strongswan.org>
Wed, 13 Apr 2016 09:58:32 +0000 (11:58 +0200)
committerTobias Brunner <tobias@strongswan.org>
Wed, 13 Apr 2016 11:55:20 +0000 (13:55 +0200)
This could interfere with cleanup handlers that try to acquire
mutexes while other threads holding these try to e.g. cancel the threads.

As cleanup handlers are only queued by the threads themselves we don't need
any synchronization to access the list.

Fixes #1401.

src/libstrongswan/threading/thread.c

index 3d87e7fca56f06545fe6f0e5eead8dc391566dd0..de5cbaa216ae4c3684909419b80bcea6f937de18 100644 (file)
@@ -278,18 +278,27 @@ static private_thread_t *thread_create_internal()
 }
 
 /**
- * Main cleanup function for threads.
+ * Remove and run all cleanup handlers in reverse order.
  */
-static void thread_cleanup(private_thread_t *this)
+static void thread_cleanup_popall_internal(private_thread_t *this)
 {
        cleanup_handler_t *handler;
-       this->mutex->lock(this->mutex);
+
        while (this->cleanup_handlers->remove_last(this->cleanup_handlers,
-                                                                                          (void**)&handler) == SUCCESS)
+                                                                                         (void**)&handler) == SUCCESS)
        {
                handler->cleanup(handler->arg);
                free(handler);
        }
+}
+
+/**
+ * Main cleanup function for threads.
+ */
+static void thread_cleanup(private_thread_t *this)
+{
+       thread_cleanup_popall_internal(this);
+       this->mutex->lock(this->mutex);
        this->terminated = TRUE;
        thread_destroy(this);
 }
@@ -417,15 +426,8 @@ void thread_cleanup_pop(bool execute)
 void thread_cleanup_popall()
 {
        private_thread_t *this = (private_thread_t*)thread_current();
-       cleanup_handler_t *handler;
 
-       while (this->cleanup_handlers->get_count(this->cleanup_handlers))
-       {
-               this->cleanup_handlers->remove_last(this->cleanup_handlers,
-                                                                                       (void**)&handler);
-               handler->cleanup(handler->arg);
-               free(handler);
-       }
+       thread_cleanup_popall_internal(this);
 }
 
 /**