Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

[Pekka Paalanen]

On Tue, 12 Jan 2016 13:03:19 +0900
"Jaeyoon Jung" <jaeyoon.jung at lge.com> wrote:

> > -----Original Message-----
> > From: wayland-devel [mailto:wayland-devel-bounces at lists.freedesktop.org] On
> > Behalf Of Jonas Adahl
> > Sent: Tuesday, January 12, 2016 12:27 PM
> > To: Jaeyoon Jung
> > Cc: wayland-devel at lists.freedesktop.org
> > Subject: Re: [PATCH v2] server: Calculate remaining data size after a closure
> > is processed
> > 
> > On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:  
> > > When processing a closure, data in the connection can be consumed again
> > > if the closure itself invokes extra event dispatch. In that case the
> > > remaining data size is also altered, so the variable len should be
> > > updated after the closure is processed.
> > >
> > > Signed-off-by: Jaeyoon Jung <jaeyoon.jung at lge.com>  
> > 
> > I don't like the name wl_connection_size (wl_connection_pending_input()
> > or something would be more descriptive).  
> 
> I just wanted to have a shorter name. wl_connection_pending_input()
> looks better to me as well, and I'll update the patch accordingly.
> Thanks.
> 
>  
> > A good follow up would be a test case where this would otherwise
> > cause issues. I assume it would be triggered by a
> > wl_event_loop_dispatch() in a interface request handler?  
> 
> Yes, exactly.

Hi,

could you humor me and explain in what sort of situations dispatching
from within a request handler is useful?

I thought recursive dispatch is a programming pattern that can only
lead to painful and erratic problems later. Don't we consider this to
be bad practice also in the client API?

If you do it in the server, that is like: this message causes more
messages to be dispatched - how does that make sense? A server must not
block waiting for any client message.

Also, inspecting future messages before finishing the processing of the
current message is racy: the following messages may or may not be in
the receive buffer and nothing guarantees either way, you just get
lucky a lot of times. Wayland IPC does not guarantee more than one
message to be delivered at a time, this is why we have things like
wl_surface.commit request.

If someone starts hardening libwayland against programmer errors, I'd
expect one thing to be to abort() on recursive dispatch. Can you tell
me why we should support recursive dispatch?

I have nothing against this patch per se, mind. Just the whole premise
that is behind this patch smells fishy to me and makes me think your
software architecture that uses recursive dispatch is fundamentally
broken.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20160202/9bf084dc/attachment.sig>