[PATCH] tevent and threads - infrastructure improvements - version #2

[Jeremy Allison]

On Mon, Sep 14, 2015 at 05:57:28PM -0700, Jeremy Allison wrote:
> 
> OK. Nailed it. The following patchset consistently
> passes valgrind with drd and helgrind with no error
> messages. The previous patchset *sometimes* (1 time
> in 10 or so :-) gave errors with helgrind.
> 
> The issue in the test is when the sub-thread deletes
> it's local event context helgrind was detecting a
> data race inside the pthread_mutex_unlock() inside
> tevent_thread_proxy_schedule() called from
> master_callback(). The clash was with the
> pthread_mutex_destroy() inside the destructor
> for struct tevent_thread_proxy.
> 
> Changing the test to return the child thread
> event context back to the parent thread, and
> have the parent thread do the talloc_free()
> in it (which destroys the mutex) once the
> child thread has exited and been joined
> removes the race.
> 
> To be honest I can't see how the race
> could actually happen in practice here,
> as the data flow in the child thread is :
> 
> proxy_schedule on master -> wait for master to reply
> 	-> release resources
> 
> and the data flow in master is
> 
> create child thread -> wait for child request ->
> 	send reply to child -> wait for child exit and join.
> 
> so how the 'release resources' could overlap
> with the 'send reply to child' beats me - the
> child must have *gotten* the reply before
> it releases resources, but helgrind occasionally
> complained about it.

Thinking about it, I think I know why helgrind
complained. helgrind doesn't see the write -> pipe,
read <- pipe constructs as a barrier, even though
because they're forming a request / response pair
they *are* a barrier for the purposes of unlocking
then deleting the mutex.

So my test code change works as a helgrind work-around,
but in the real world isn't strictly neccessary,
which is why it's safe to leave the docs with
the talloc_free(ev) construct at the end of the
child thread.

Cheers,

	Jeremy.