[RFC wayland 1/2] client: Introduce 'proxy wrapper' concept

[Bill Spitzak]

On Tue, Jul 14, 2015 at 6:26 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

>
> >     bar = wl_new_bar();
> >     wl_proxy_set_queue((struct wl_proxy*)bar, queue); // default queue if
> > this not done
> >     wl_bar_add_listener(bar, ...);
> >     wl_foo_get_bar(foo, bar);
> >
> > Note the last function will have to be renamed to avoid a collision in C
> > but I don't have any good ideas what to name it.
>
> FWIW, this is roughly how Wayland API originally worked. It had to be
> changed, because it had a race in object id allocation and request
> sending, and both sides of the IPC expect object ids to be allocated in
> order.
>
> As id allocation must happen in the same order as sending the requests,
> you cannot split them, because another thread might run in between and
> send another request creating another new object, which screws up the
> id allocation ordering.
>
> Why they have to be allocated in order I no longer remember, if I ever
> knew.
>

My guess is they don't really have to be in order, but that it was
desirable to avoid the overhead of two locks (one for allocating an
objectId and a second for the wl_connection_write), by using one lock
around them all.

In fact you are grabbing a lock in wl_proxy_marshal_array_constructor
surrounding the creation of the wl_proxy object and the queuing of the
message (the closure_send). But this means quite a lot of code is
surrounded by this lock and thus is single threaded. This includes the
malloc of the wl_proxy, the wl_closure, and a buffer inside
wl_closure_send, and the initializing of all this data. The lock also
surrounds the free of the wl_closure and the buffer. This seems like a
mistake.

I would think a wl_proxy could exist without an objectId allocated. Then
you could allocate the id right next to the wl_connection_write in
wl_closure_send, and the lock is only around these two statements. This
would make a much smaller piece of code single-threaded, it would also
preserve the objectid allocation and message order if that really is
necessary, and still only use one lock. And it would allow the wl_proxy to
be created and modified before it is assigned to an actual object.

This would require wl_closure_send to understand newid arguments, just like
it is special-casing fd arguments.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150714/93346d9c/attachment.html>