[RFC wayland 1/2] client: Introduce 'proxy wrapper' concept
Jonas Ådahl
jadahl at gmail.com
Mon Jun 29 04:30:35 PDT 2015
Arnaud Vrac discovered an issue in the libwayland client API causing
race conditions when doing round trips using the wl_display object using
non-default proxy queues.
The problem is that one thread can read and dispatch events after
another thread creates the wl_callback object but before it sets the
queue. This could potentially cause the events to be dropped completely
if the event is dispatched before the creating thread sets the
implementation, or that the event is dispatched on the wrong queue which
might run on another thread.
This patch introduces API to solve this case by introducing "proxy
wrappers". In short, a proxy wrapper is a struct wl_proxy * that will
never itself emit any events, but the client can set a queue, and use it
when sending requests that creates new proxy objects. When sending
requests, the wrapper will work in the same way as the normal proxy
object, but the proxy created by sending a request (for example
wl_display.sync) will default to the same proxy queue as the wrapper.
Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
---
Hi,
This is one of the two alternative solutions that was discussed on #wayland.
The other one being introducing API alternatives to
wl_proxy_marshal_constructor and wl_proxy_marshal_array_constructor that
also take a queue and also changing scanner.c to produce functions that
take a queue for every request that creates a proxy.
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, ...);
and the with other idea that is implemented anywhere yet AFAIK
bar = wl_foo_get_bar_with_queue(foo, queue)
wl_bar_add_listener(bar, ...);
Opinions?
Jonas
src/wayland-client-core.h | 1 +
src/wayland-client.c | 106 +++++++++++++++++++++++++++++++++++++++++-----
src/wayland-private.h | 7 +++
3 files changed, 103 insertions(+), 11 deletions(-)
diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
index dea70d9..0db8b9c 100644
--- a/src/wayland-client-core.h
+++ b/src/wayland-client-core.h
@@ -125,6 +125,7 @@ void wl_proxy_marshal_array(struct wl_proxy *p, uint32_t opcode,
union wl_argument *args);
struct wl_proxy *wl_proxy_create(struct wl_proxy *factory,
const struct wl_interface *interface);
+struct wl_proxy *wl_proxy_create_wrapper(struct wl_proxy *proxy);
struct wl_proxy *wl_proxy_marshal_constructor(struct wl_proxy *proxy,
uint32_t opcode,
const struct wl_interface *interface,
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 6450b67..f8c64c4 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -51,7 +51,8 @@
enum wl_proxy_flag {
WL_PROXY_FLAG_ID_DELETED = (1 << 0),
- WL_PROXY_FLAG_DESTROYED = (1 << 1)
+ WL_PROXY_FLAG_DESTROYED = (1 << 1),
+ WL_PROXY_FLAG_WRAPPER = (1 << 2),
};
struct wl_proxy {
@@ -405,17 +406,19 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
void
proxy_destroy(struct wl_proxy *proxy)
{
- if (proxy->flags & WL_PROXY_FLAG_ID_DELETED)
- wl_map_remove(&proxy->display->objects, proxy->object.id);
- else if (proxy->object.id < WL_SERVER_ID_START)
- wl_map_insert_at(&proxy->display->objects, 0,
- proxy->object.id, WL_ZOMBIE_OBJECT);
- else
- wl_map_insert_at(&proxy->display->objects, 0,
- proxy->object.id, NULL);
-
+ if (proxy->flags != WL_PROXY_FLAG_WRAPPER) {
+ if (proxy->flags & WL_PROXY_FLAG_ID_DELETED)
+ wl_map_remove(&proxy->display->objects,
+ proxy->object.id);
+ else if (proxy->object.id < WL_SERVER_ID_START)
+ wl_map_insert_at(&proxy->display->objects, 0,
+ proxy->object.id, WL_ZOMBIE_OBJECT);
+ else
+ wl_map_insert_at(&proxy->display->objects, 0,
+ proxy->object.id, NULL);
- proxy->flags |= WL_PROXY_FLAG_DESTROYED;
+ proxy->flags |= WL_PROXY_FLAG_DESTROYED;
+ }
proxy->refcount--;
if (!proxy->refcount)
@@ -459,6 +462,11 @@ WL_EXPORT int
wl_proxy_add_listener(struct wl_proxy *proxy,
void (**implementation)(void), void *data)
{
+ if (proxy->flags == WL_PROXY_FLAG_WRAPPER) {
+ wl_log("proxy %p is a wrapper\n", proxy);
+ return -1;
+ }
+
if (proxy->object.implementation || proxy->dispatcher) {
wl_log("proxy %p already has listener\n", proxy);
return -1;
@@ -512,6 +520,11 @@ wl_proxy_add_dispatcher(struct wl_proxy *proxy,
wl_dispatcher_func_t dispatcher,
const void *implementation, void *data)
{
+ if (proxy->flags == WL_PROXY_FLAG_WRAPPER) {
+ wl_log("proxy %p is a wrapper\n", proxy);
+ return -1;
+ }
+
if (proxy->object.implementation || proxy->dispatcher) {
wl_log("proxy %p already has listener\n");
return -1;
@@ -1885,6 +1898,77 @@ wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue)
proxy->queue = &proxy->display->default_queue;
}
+/** Create a proxy wrapper
+ *
+ * \param proxy The proxy object to be wrapped
+ * \return A proxy wrapper for the given proxy
+ *
+ * A proxy wrapper is type of 'struct wl_proxy' instance that can be used for
+ * sending requests instead of using the original proxy. A proxy wrapper does
+ * not have an implementation or dispatcher, and events received on the
+ * object is still emitted on the original proxy. Trying to set an
+ * implementation or dispatcher will have no effect but result in a warning
+ * being logged.
+ *
+ * Even though there is no implementation nor dispatcher, the proxy queue can
+ * be changed. This will affect the default queue of new objects created by
+ * requests sent via the proxy wrapper.
+ *
+ * A proxy wrapper must be destroyed before the proxy it was created from.
+ * Creating a proxy wrapper on a proxy that has either been destroyed or
+ * removed will fail, and NULL will be returned.
+ *
+ * If a user reads and dispatches events on more than one thread, it is
+ * necessary to use a proxy wrapper when sending requests on objects when the
+ * intention is that a newly created proxy is to use a proxy queue different
+ * from the proxy the request was sent on, as creating the new proxy and then
+ * setting the queue is not thread safe.
+ *
+ * For example, a module that runs using its own proxy queue that needs to
+ * do display roundtrip must wrap the wl_display proxy object before sending
+ * the wl_display.sync request. For example:
+ *
+ * struct wl_proxy *wrapped_display;
+ * struct wl_callback *callback;
+ *
+ * wrapped_display = wl_proxy_create_wrapper((struct wl_proxy *) display);
+ * callback = wl_display_sync((struct wl_display *) wrapped_display);
+ * wl_proxy_destroy(wrapped_display);
+ * wl_callback_add_listener(callback, ...);
+ *
+ * \memberof wl_proxy
+ */
+WL_EXPORT struct wl_proxy *
+wl_proxy_create_wrapper(struct wl_proxy *proxy)
+{
+ struct wl_proxy *wrapper;
+
+ wrapper = zalloc(sizeof *wrapper);
+ if (!wrapper)
+ return NULL;
+
+ pthread_mutex_lock(&proxy->display->mutex);
+
+ if ((proxy->flags & WL_PROXY_FLAG_ID_DELETED) ||
+ (proxy->flags & WL_PROXY_FLAG_DESTROYED)) {
+ pthread_mutex_unlock(&proxy->display->mutex);
+ wl_log("proxy %p already deleted or destroyed flag: 0x%x\n",
+ proxy, proxy->flags);
+ return NULL;
+ }
+
+ wrapper->object.interface = proxy->object.interface;
+ wrapper->object.id = proxy->object.id;
+ wrapper->display = proxy->display;
+ wrapper->queue = proxy->queue;
+ wrapper->flags = WL_PROXY_FLAG_WRAPPER;
+ wrapper->refcount = 1;
+
+ pthread_mutex_unlock(&proxy->display->mutex);
+
+ return wrapper;
+}
+
WL_EXPORT void
wl_log_set_handler_client(wl_log_func_t handler)
{
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 17c507c..f072811 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -29,6 +29,7 @@
#define WAYLAND_PRIVATE_H
#include <stdarg.h>
+#include <stdlib.h>
#define WL_HIDE_DEPRECATED 1
@@ -71,6 +72,12 @@ struct wl_map {
typedef void (*wl_iterator_func_t)(void *element, void *data);
+static inline void *
+zalloc(size_t size)
+{
+ return calloc(1, size);
+}
+
void wl_map_init(struct wl_map *map, uint32_t side);
void wl_map_release(struct wl_map *map);
uint32_t wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data);
--
2.1.4
More information about the wayland-devel
mailing list