Hi Jonas, What are the shortcomings of my approach ? Because we want to use this approach internally for thread safety.
If there are no shortcomings, is it not better to use this approach than introducing many new APIs ? Best regards Emre Ucan Software Group I (ADITG/SW1) Tel. +49 5121 49 6937 > -----Original Message----- > From: wayland-devel [mailto:wayland-devel- > [email protected]] On Behalf Of Jonas Ådahl > Sent: Dienstag, 16. Februar 2016 04:49 > To: Ucan, Emre (ADITG/SW1) > Cc: [email protected] > Subject: Re: [PATCH wayland] client: ensure thread safety for > wl_display_roundtrip_queue() > > On Mon, Feb 15, 2016 at 02:19:43PM +0000, Ucan, Emre (ADITG/SW1) wrote: > > We have to block other threads from reading events. Otherwise other > > threads may assign done event to the default queue, if they call > > wl_display_read_events before setting the queue for the callback proxy. > > This would cause a deadlock in wl_display_dispatch_queue, because the > > default queue is not dispatched. > > > > We can call wl_display_prepare_read_queue API which blocks on success > > other threads from reading events and call wl_display_cancel read > > after the queue is set to the proxy. This new implementation ensures > > thread safety for the API. > > This is the work-around available to avoid the issue, but it is not the > correct > solution for libwayland-client. Right now we have two potential > solutions: one is to add a "proxy wrapper" where one configures the proxy > wrapper object (setting the queue) before using it to issue requests, the > other is to add thread safe functions and patch the scanner to add thread > safe helpers which would be used instead of for example > "wl_display_sync()". > > The proxy wrapper approach can be tested out by appling this patch: > https://lists.freedesktop.org/archives/wayland-devel/2015- > June/023054.html > > For more information, check the corresponding bug in bugzilla: > https://bugs.freedesktop.org/show_bug.cgi?id=91273 > > > Jonas > > > > > Signed-off-by: Emre Ucan <[email protected]> > > --- > > src/wayland-client.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/src/wayland-client.c b/src/wayland-client.c index > > ef12bfc..00d8cda 100644 > > --- a/src/wayland-client.c > > +++ b/src/wayland-client.c > > @@ -1075,12 +1075,26 @@ wl_display_roundtrip_queue(struct wl_display > *display, struct wl_event_queue *qu > > struct wl_callback *callback; > > int done, ret = 0; > > > > + /*We have to block other threads from reading events. Otherwise > other threads > > + * may assign done event to the default queue, if they call > wl_display_read_events > > + * before setting the queue for the callback proxy. This would cause a > deadlock > > + * in wl_display_dispatch_queue, because the default queue is not > dispatched.*/ > > + while (ret >= 0 && wl_display_prepare_read_queue(display, > queue)) > > + ret = wl_display_dispatch_queue_pending(display, queue); > > + > > + if (ret < 0) > > + return ret; > > + > > done = 0; > > callback = wl_display_sync(display); > > - if (callback == NULL) > > + if (callback == NULL) { > > + wl_display_cancel_read(display); > > return -1; > > + } > > + > > wl_proxy_set_queue((struct wl_proxy *) callback, queue); > > wl_callback_add_listener(callback, &sync_listener, &done); > > + wl_display_cancel_read(display); > > while (!done && ret >= 0) > > ret = wl_display_dispatch_queue(display, queue); > > > > -- > > 1.7.9.5 > > > > _______________________________________________ > > wayland-devel mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
