host-resolver: Do not cancel threads waiting for new queries during cleanup

While it is currently unclear why it happens, canceling threads waiting in the
new_query condvar does not work as expected. The behavior is not fully
reproducible: Either cancel(), join() or destroying the condvar hangs.

The issue has been seen in the http-fetcher unit tests, where the stream service
triggers the use of the resolver for "localhost" hosts. It is reproducible with
any cleanup following a host_create_from_dns() use on a Ubuntu 14.04 x64 system.
Further, the issue is related to the use of libunwind, as only builds with
--enable-unwind-backtraces are affected.

As we broadcast() the new_query condvar before destruction, a hard cancel() of
these threads is actually not required. Instead we let these threads clean up
themselves after receiving the condvar signal.

diff --git a/src/libstrongswan/networking/host_resolver.c b/src/libstrongswan/networking/host_resolver.c
index cb8b48b459c22315d25d7dd948b7e72af952344e..bad87e434772c33002578fd883dd4cfee986fba3 100644 (file)
--- a/src/libstrongswan/networking/host_resolver.c
+++ b/src/libstrongswan/networking/host_resolver.c
@@ -169,17 +169,19 @@ static void *resolve_hosts(private_host_resolver_t *this)
        while (TRUE)
        {
                this->mutex->lock(this->mutex);
-               thread_cleanup_push((thread_cleanup_t)this->mutex->unlock, this->mutex);
                while (this->queue->remove_first(this->queue,
                                                                                (void**)&query) != SUCCESS)
                {
-                       old = thread_cancelability(TRUE);
+                       if (this->disabled)
+                       {
+                               this->mutex->unlock(this->mutex);
+                               return NULL;
+                       }
                        timed_out = this->new_query->timed_wait(this->new_query,
                                                                        this->mutex, NEW_QUERY_WAIT_TIMEOUT * 1000);
-                       thread_cancelability(old);
                        if (this->disabled)
                        {
-                               thread_cleanup_pop(TRUE);
+                               this->mutex->unlock(this->mutex);
                                return NULL;
                        }
                        else if (timed_out && (this->threads > this->min_threads))
@@ -188,13 +190,13 @@ static void *resolve_hosts(private_host_resolver_t *this)
 
                                this->threads--;
                                this->pool->remove(this->pool, thread, NULL);
-                               thread_cleanup_pop(TRUE);
+                               this->mutex->unlock(this->mutex);
                                thread->detach(thread);
                                return NULL;
                        }
                }
                this->busy_threads++;
-               thread_cleanup_pop(TRUE);
+               this->mutex->unlock(this->mutex);
 
                memset(&hints, 0, sizeof(hints));
                hints.ai_family = query->family;