[PATCH] tevent and threads - infrastructure improvements.

[Jeremy Allison]

On Wed, Jul 22, 2015 at 04:21:55PM -0700, Jeremy Allison wrote:
> On Wed, Jul 22, 2015 at 04:12:08PM -0700, Jeremy Allison wrote:
> > On Thu, Jul 23, 2015 at 12:52:15AM +0200, Stefan (metze) Metzmacher wrote:
> > 
> > > Something like this would be better:
> > > 
> > > struct tevent_context *tevent_thread_proxy_context(struct tevent_context
> > > *main_ev);
> > > 
> > > The thread can then only schedule immediate events on the returned proxy
> > > context, this will wakeup the main_ev and move the immediate events from
> > > the proxy to the main_ev.
> > > 
> > > The immediate events can be preallocated in before starting the thread.
> > 
> > I don't get this. Can you explain how this would be
> > used in the same way as ev_async_send() ?
> 
> More thing I don't get. Who calls tevent_thread_proxy_context() ?
> Is it the requesting thread ? If you're returning a struct tevent_context *
> that means internal changes to tevent_immediate when you schedule
> to decide if it's a regular context or a "proxy" context. As I've
> said, preallocation is a non-starter for scalable use so I think
> I need to see sample client code before I can understand your
> thinking here.
> 
> Also the code I posted has no internal changes to existing
> tevent code, which is a big plus for me.

Oh yeah, one more thing (sorry, getting a gut reaction
to this as I think about it more :-).

Returning a "proxy" struct tevent_context *
from a call like tevent_thread_proxy_context()
is a really bad API decision.

It pretends to be a regular struct tevent_context *,
but it's not in that the only thing you can do with
it is call tevent_create_immediate() on it - you
shouldn't be able to call tevent_add_fd() or
tevent_add_time() on it as that wouldn't have any
meaning. But the coder doesn't realize that - it
just apprears to be a regular struct tevent_context *
to them - only convention says otherwise.

That's a receipe for coding disasters IMHO.

I knew there was a reason I didn't add any user-visible
new data types in my implementation (other than the
callback function, which is pretty generic).

So nope, *really* don't like that API suggestion
I'm afraid.