[PATCH] tevent and threads - infrastructure improvements.

[Uri Simchoni]

So, passing around talloc'd objects between threads seems like a bad
idea for overall multi-threaded app architecture, if talloc is not
mt-safe. The question that can be asked is - can tevent be used in a
mt app, with talloc being just an implementation detail? The answer
seems to be:
1. Yes, but it would loose some of tevent's appeal. For example one
would have to use tevent_req_callback_data_void() instead of
tevent_req_callback_data().
2. The new functionality is an exception - the pp_private has to be
talloc'd. To be convenient, I'd want to pass my object from thread to
thread, and not have to create a "pp_private wrapper". A fix for that
can be to create the wrapping object (tallocated) inside
tevent_threaded_async_call().

Just my 2c...

Otherwise it looks good to me except I'd want to see eventfd2 with
EFD_CLOEXEC used on Linux - it both resets all writers in one call
without having to loop, and avoids the CLOEXEC race condition between
creating the pipe and setting CLOEXEC (remember - it's a
multi-threaded app, someone may be executing a child process in
another therad). pipe2 can also be used to avoid that race.


On Thu, Jul 23, 2015 at 3:08 AM, Jeff Layton <jlayton at samba.org> wrote:
> On Wed, 22 Jul 2015 12:09:29 -0700
> Jeremy Allison <jra at samba.org> wrote:
>
>> It always irritated me that someone close to the Samba
>> Team (who shall remain nameless) selected libev over
>> tevent when designing some new code.
>>
>
> I appreciate Jeremy looking to _not_ call me out on this, but I'll step
> forward. I ended up recommending the move to libev. It wasn't an easy
> decision as there are a lot of really nice things about tevent.
>
>> The reason was that libev has the function:
>>
>> ev_async_send()
>>
>> which allows one thread to schedule a callback
>> on the event loop of another thread.
>>
>
> That was a nice feature of libev, but in the end Jeremy was quite right
> to point out that that function just writes to a pipe to trigger the
> event (very similar to the equivalent code that we ended up with before
> we switched to libev). The ev_async_* stuff is really just syntactic
> sugar, but it does neatly hide the pipe handlers.
>
> For the record, the _main_ reason we ended up dropping tevent was
> actually problems in using talloc. tevent relies heavily on talloc and
> we hit a number of problems when trying to tear down talloc objects in
> threaded code.
>
> I strongly considered keeping tevent and just trying hard(er) to ensure
> that we didn't end up mucking with any talloc contexts from multiple
> threads, but it was pretty difficult to ensure that, given the way that
> talloc destructors get called and the way that we needed to do
> allocations.
>
> So, while I think adding this to tevent is a very good idea, it
> probably wouldn't have been sufficient for us to keep it. A threadsafe
> talloc would probably have helped considerably however.
>
> What would be _really_ cool (and fun!) would be to write a talloc 3.0
> that is threadsafe, ideally using atomic operations and (maybe) RCU to
> avoid locking.
>
> Well...I can dream can't I? ;)
>
>> I knew this could be done in tevent, but didn't
>> have an elegant (a matter of opinion of course :-)
>> solution, so I coded one up. Here it is for your
>> review.
>>
>> It has the advantage that if you don't need
>> the functionality to schedule a callback
>> on the event loop of another thread, you don't
>> pay any costs in regular tevent usage.
>>
>> It adds 3 functions to tevent:
>>
>> int tevent_threaded_context_register(struct tevent_context *ev);
>>
>> which sets up the background data structures
>> needed for the functionality.
>>
>> int tevent_threaded_context_unregister(struct tevent_context *ev);
>>
>> which tears them down again, and finally:
>>
>> int tevent_threaded_async_call(struct tevent_context *dest_ev_ctx,
>>                                 void (*callback_fn)(struct tevent_context *,
>>                                                 void *),
>>                                 void **pp_private);
>>
>> which asynchronously schedules callback_fn to be invoked
>> on the destination context under the thread that's running
>> it's event loop.
>>
>> It (and the tests I added) pass valgrind --tool=drd
>> but Volker, you are the threaded code master, so
>> I'd love you to take a closer look.
>>
>> Please review.
>>
>> Cheers !
>>
>> Jeremy.
>>
>> PS. Just wanted to leave this here:
>>
>> http://bholley.net/blog/2015/must-be-this-tall-to-write-multi-threaded-code.html
>>
>
> Nice article. Thanks for posting the link!
>
>> PPS. This is why I've been a bit quiet recently,
>> getting this done took a lot of thinking time :-).
>> Back to your normally scheduled Jeremy service
>> now...
>
>
> --
> Jeff Layton <jlayton at samba.org>
>