host-resolver: Do not cancel threads waiting for new queries during cleanup
authorMartin Willi <martin@revosec.ch>
Tue, 24 Feb 2015 15:00:38 +0000 (16:00 +0100)
committerMartin Willi <martin@revosec.ch>
Tue, 24 Feb 2015 15:00:38 +0000 (16:00 +0100)
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.

src/libstrongswan/networking/host_resolver.c

index cb8b48b..bad87e4 100644 (file)
@@ -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;