[PATCH wayland] client: ensure thread safety for wl_display_roundtrip_queue()

[Pekka Paalanen]

On Tue, 16 Feb 2016 14:26:53 -0800
Bill Spitzak <spitzak at gmail.com> wrote:

> On Mon, Feb 15, 2016 at 7:48 PM, Jonas Ådahl <jadahl at gmail.com> wrote:
> 
> The proxy wrapper approach can be tested out by appling this patch:
> > https://lists.freedesktop.org/archives/wayland-devel/2015-June/023054.html  
> 
> 
> Paraphrased here:
> 
> > In short, instead of
> >
> >         bar = wl_foo_get_bar(foo);
> >         wl_proxy_set_queue((struct wl_proxy *) bar, queue);
> >         wl_bar_add_listener(bar, ...);
> >
> > with this RFC a client does
> >
> >         foo_wrapper = wl_proxy_create_wrapper((struct wl_proxy *) foo);
> >         wl_proxy_set_queue((struct wl_proxy *) foo_wrapper, queue);
> >         bar = wl_foo_get(foo_wrapper);
> >         wl_bar_add_listener(bar, ...);  
> 
> This seems unnecessarily complex. Instead the wl_proxy can be in an
> unassigned state (identified by the id being set to a special value that
> will never be used otherwise). This avoids the extra object, allows the
> same code to be used even if there are different constructors for bar, and
> allows other setup before the id is assigned (for instance
> wl_bar_add_listener is not thread safe and above examples rely on the queue
> being the one belonging to the current thread):
> 
>    bar = wl_bar_new();
>    wl_proxy_set_queue((struct wl_proxy*)bar, queue);
>    wl_bar_add_listener(bar, ...);
>    wl_foo_get_bar_id(foo, bar, ...);

That seems like it would work, if we were to create a completely new
API for handling object creation. It just changes the whole C bindings
API paradigm.

Currently the C API is generated for this:

struct wl_foo *foo_obj = ...;
struct wl_bar *bar_obj = wl_foo_create_bar(foo_obj);

To implement your proposal, we have to abandon wl_foo_create_bar() as
legacy, and do this instead:

struct wl_foo *foo_obj = ...;
struct wl_bar *bar_obj;

bar_obj = wl_bar_new();
wl_proxy_set_queue((wl_proxy *)bar_obj, queue);
wl_bar_add_listener(bar_obj, ...);
/* whatever else to init bar_obj */
wl_foo_create_barEX(foo_obj, bar_obj);

Yeah, that should work. And it should avoid the problem fixed in
https://cgit.freedesktop.org/wayland/wayland/commit/?id=853c24e6998f747150e4233cf41bfa8268964cc2
because wl_bar_new() would not allocate an id like wl_proxy_create()
did.

The problem is it's fundamentally different to the existing C bindings
API and even the libwayland-client ABI.

It might be worth investigating, though. If it is possible to migrate
to the new API without breaking anything, it just could be worth it.


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: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160218/62e329ef/attachment.sig>