abort() calls from messaging code

[Jeremy Allison]

On Thu, Jun 15, 2017 at 11:53:45AM +0200, Volker Lendecke via samba-technical wrote:
> On Tue, Jun 13, 2017 at 05:03:36PM +1200, Andrew Bartlett via samba-technical wrote:
> > (resend from samba.org address subscribed to the list)
> > 
> > I'm seeing odd failures in the messaging code while attempting builds
> > on my new locking code for ldb.  It seems as I reduce the other
> > flapping tests, we perturb the state and start to see different parts
> > of samba misbehave. 
> 
> Fix one race, open another.
> 
> The attached patch should fix it. Review appreciated!
> 
> Sorry, Volker

Arrrgggh. Sorry, this stuff is a black art :-(. Really sorry
I didn't catch this in the 1828011317b review.

Jeremy.

> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de

> From 7ac209c45c9594c373acbd066d52082cc9b0c9d1 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Thu, 15 Jun 2017 11:48:24 +0200
> Subject: [PATCH] tevent_threads: Fix a rundown race introduced with
>  1828011317b
> 
> The race is easily reproduced by adding a poll(NULL,0,10) in between the two
> pthread_mutex_unlock calls in _tevent_threaded_schedule_immediate.
> 
> Before 1828011317b, the main thread was signalled only after the helper
> had already unlocked event_ctx_mutex.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/tevent/tevent_threads.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/lib/tevent/tevent_threads.c b/lib/tevent/tevent_threads.c
> index 8ecda02..4d1a880 100644
> --- a/lib/tevent/tevent_threads.c
> +++ b/lib/tevent/tevent_threads.c
> @@ -381,6 +381,23 @@ static int tevent_threaded_context_destructor(
>  		DLIST_REMOVE(tctx->event_ctx->threaded_contexts, tctx);
>  	}
>  
> +	/*
> +	 * We have to coordinate with _tevent_threaded_schedule_immediate's
> +	 * unlock of the event_ctx_mutex. We're in the main thread here,
> +	 * and we can be scheduled before the helper thread finalizes its
> +	 * call _tevent_threaded_schedule_immediate. This means we would
> +	 * pthreadpool_destroy a locked mutex, which is illegal.
> +	 */
> +	ret = pthread_mutex_lock(&tctx->event_ctx_mutex);
> +	if (ret != 0) {
> +		abort();
> +	}
> +
> +	ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
> +	if (ret != 0) {
> +		abort();
> +	}
> +
>  	ret = pthread_mutex_destroy(&tctx->event_ctx_mutex);
>  	if (ret != 0) {
>  		abort();
> -- 
> 2.1.4
>