thread: Don't hold mutex when calling cleanup handlers while terminating

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.

1 files changed, 14 insertions, 12 deletions

diff --git a/src/libstrongswan/threading/thread.c b/src/libstrongswan/threading/thread.c
index 3d87e7fca..de5cbaa21 100644
--- a/src/libstrongswan/threading/thread.c
+++ b/src/libstrongswan/threading/thread.c
@@ -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);
 }
 
 /**