[PATCH] small fixes for pthreadpool

[Jeremy Allison]

On Mon, Aug 29, 2016 at 11:46:52AM +0200, Volker Lendecke wrote:
> Hi!
> 
> Review appreciated!

LGTM & pushed. I remember 1c4284c7395f23 and why it was added, so
I'm happy now to revert.

Cheers,

Jeremy.

> From 8332db8c45629394cd609d2abd847ae56ea7f8b7 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 29 Aug 2016 11:35:20 +0200
> Subject: [PATCH 1/2] pthreadpool: We always want asserts to abort()
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/lib/pthreadpool/pthreadpool.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
> index fc21d43..2e9f42c 100644
> --- a/source3/lib/pthreadpool/pthreadpool.c
> +++ b/source3/lib/pthreadpool/pthreadpool.c
> @@ -23,6 +23,11 @@
>  #include "system/threads.h"
>  #include "pthreadpool.h"
>  #include "lib/util/dlinklist.h"
> +
> +#ifdef NDEBUG
> +#undef NDEBUG
> +#endif
> +
>  #include <assert.h>
>  
>  struct pthreadpool_job {
> -- 
> 2.1.4
> 
> 
> From 49a3bfac6c5bf3c44c7b9484e81940d9a6a023e2 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 29 Aug 2016 11:35:39 +0200
> Subject: [PATCH 2/2] pthreadpool: Signal job completion without the pool mutex
> 
> This essentially reverts 1c4284c7395f23. We now call an alien function from
> within pthreadpool, and we should not hold a mutex during that call. The alien
> function could (and pthreadpool_tevent_job_signal actually does) lock a mutex.
> We can't guarantee proper lock ordering here, so in theory we could deadlock. I
> haven't seen it in the wild yet, but I could imagine that both _parent pieces
> in pthreadpool and tevent could trigger such a deadlock.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/lib/pthreadpool/pthreadpool.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
> index 2e9f42c..ee59cc4 100644
> --- a/source3/lib/pthreadpool/pthreadpool.c
> +++ b/source3/lib/pthreadpool/pthreadpool.c
> @@ -491,12 +491,13 @@ static void *pthreadpool_server(void *arg)
>  
>  			job.fn(job.private_data);
>  
> -			res = pthread_mutex_lock(&pool->mutex);
> -			assert(res == 0);
> -
>  			ret = pool->signal_fn(job.id,
>  					      job.fn, job.private_data,
>  					      pool->signal_fn_private_data);
> +
> +			res = pthread_mutex_lock(&pool->mutex);
> +			assert(res == 0);
> +
>  			if (ret != 0) {
>  				pthreadpool_server_exit(pool);
>  				pthread_mutex_unlock(&pool->mutex);
> -- 
> 2.1.4
>