tevent_loop_set_nesting_hook deprecated?

[Stefan (metze) Metzmacher]

Hi Tridge,

> I notice you marked the new interface tevent_loop_set_nesting_hook()
> as deprecated. It is a bit of a strange idea marking a new
> interface as deprecated :-)

:-)

> I presume you marked it as deprecated as you would like to discourage
> new semi-async code. That doesn't really make sense though, as the
> caller of tevent_loop_set_nesting_hook() isn't semi-async, it is just
> adding code in case some other routine is semi-async. It is needed
> because vfs_unixuid changes the global state of the process (the
> euid/egid/groups) and that doesn't fit with other code that may depend
> on a particicular value of state state (ie. depend on us running as
> root).

I'm fine with removing the compiler warning,
maybe I can add we can let samba4 define TEVENT_DEPRECATED_NOWARN
As tevent will also be used by other external projects. I marked it
as deprecated, because samba4 should be the only user of this function.

> I also don't think it is realistic to want Samba to never have any
> semi-async code. The particular case that triggered the addition of
> tevent_loop_set_nesting_hook() demonstrates how hard it is to make
> everything completely async. To make it async, all internal calls in
> all of the VFS layer would need to be fully async to cope with the
> possible calls to winbind. That is a pretty big overhead for something
> that is really a non-issue.
> 
> semi-async code is not evil. It is a heck of a lot better than
> non-async code, and the only real downside to semi-async code is that
> the order of completion of multiple outstanding semi-async calls is
> the reverse order the calls were made with. 

I think semi-async code make thinks really hard to debug,
and we added a lot of complex hacks, e.g. in the packet code
to detect that the same event handler is called twice.

Also we have a lot of places in our client libraries,
where we just crash when we get timeouts, because the
timeout event is triggered and calls the higherlevel completion
function, which then destroys a request which is still
actively processed in the nested event stack.

> The existance of a semi-async call does nothing at all to harm
> full-async code. The full-async code can continue to operate
> completely correctly in the presence of any number of outstanding
> semi-async calls, so the only routines that can be adversely affected
> by semi-async calls are themselves.

That's not true, see my example above, it removes that garantee that
only one event is processed at a time. As that makes sure that a timeout
handler is triggered not as the same time the main working function of
the request is handled. And adding complexity handle that in every
full-async code, is really ugly!

> This is much better than the non-async approach that we will be left with if semi-async is
> deprecated. If a non-async call is made, then everything else
> stops. That should indeed be avoided, and if you want to deprecate
> that then I'd fully support you. Deprecating the much better solution
> of using semi-async makes no sense to me.

Samba4 should just ignore the deprecation warning (or we just remove
it), but it should need the TEVENT_DEPRECATED define to get that function.

I know we need to support semi-async code for while, but with the
tevent_req infrastructure it's relatively easy to convert the semi-async
code to become full-async, which is the best solution.

I know it's a lot of work to convert all semi-async code, but I think
that should be the end goal.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
Url : http://lists.samba.org/archive/samba-technical/attachments/20090319/3c586b1e/signature.bin