[PATCH wayland 1/2] client: Keep track of proxy validity and number of reference holders
Jonas Ådahl
jadahl at gmail.com
Sat Nov 3 14:26:10 PDT 2012
When events are queued, the associated proxy objects (target proxy and
potentially closure argument proxies) are verified being valid. However,
as any event may destroy some proxy object, validity needs to be
verified again before dispatching. Before this change this was done by
again looking up the object via the display object map, but that did not
work because a delete_id event could be dispatched out-of-order if it
was queued in another queue, causing the object map to either have a new
proxy object with the same id or none at all, had it been destroyed in
an earlier event in the queue.
Instead, make wl_proxy reference counted and increase the reference
counter of every object associated with an event when it is queued. In
wl_proxy_destroy() set a flag saying the proxy has been destroyed by the
application and only free the proxy if the reference counter reaches
zero after decreasing it.
Before dispatching, verify that a proxy object still is valid by
checking that the flag set in wl_proxy_destroy() has not been set. When
dequeuing the event, all associated proxy objects are dereferenced and
free:ed if the reference counter reaches zero. As proxy reference counter
is initiated to 1, when dispatching an event it can never reach zero
without having the destroyed flag set.
Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
---
Hi Kristian,
Following up on the discussion the other day about this issue, we cannot
only rely on reference counting for knowing if we can drop events or not
since multiple events may be reference holders of the same destroyed
proxy. Instead I added another wayland-client specific field to
wl_closure to keep a pointer to the target proxy object.
To verify validity of the proxy I introduced a new flag (that also
absorbed `id_deleted') that is set in wl_proxy_destroy(). Reference
counting is still needed so that wl_proxy_destroy() doesn't free any
memory before everyone is done with it.
Jonas
src/wayland-client.c | 128 +++++++++++++++++++++++++++++++++++++++++--------
src/wayland-private.h | 2 +
2 files changed, 110 insertions(+), 20 deletions(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 7e50b40..d3a7970 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -45,11 +45,17 @@
/** \cond */
+enum wl_proxy_flag {
+ WL_PROXY_FLAG_ID_DELETED = (1 << 0),
+ WL_PROXY_FLAG_DESTROYED = (1 << 1)
+};
+
struct wl_proxy {
struct wl_object object;
struct wl_display *display;
struct wl_event_queue *queue;
- int id_deleted;
+ uint32_t flags;
+ int refcount;
void *user_data;
};
@@ -216,7 +222,8 @@ wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
proxy->object.implementation = NULL;
proxy->display = display;
proxy->queue = factory->queue;
- proxy->id_deleted = 0;
+ proxy->flags = 0;
+ proxy->refcount = 1;
pthread_mutex_lock(&display->mutex);
proxy->object.id = wl_map_insert_new(&display->objects,
@@ -243,7 +250,8 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
proxy->object.id = id;
proxy->display = display;
proxy->queue = factory->queue;
- proxy->id_deleted = 0;
+ proxy->flags = 0;
+ proxy->refcount = 1;
wl_map_insert_at(&display->objects, id, proxy);
@@ -259,9 +267,11 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
WL_EXPORT void
wl_proxy_destroy(struct wl_proxy *proxy)
{
- pthread_mutex_lock(&proxy->display->mutex);
+ struct wl_display *display = proxy->display;
+
+ pthread_mutex_lock(&display->mutex);
- if (proxy->id_deleted)
+ 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,
@@ -270,9 +280,14 @@ wl_proxy_destroy(struct wl_proxy *proxy)
wl_map_insert_at(&proxy->display->objects,
proxy->object.id, NULL);
- pthread_mutex_unlock(&proxy->display->mutex);
- free(proxy);
+ proxy->flags |= WL_PROXY_FLAG_DESTROYED;
+
+ proxy->refcount--;
+ if (!proxy->refcount)
+ free(proxy);
+
+ pthread_mutex_unlock(&display->mutex);
}
/** Set a proxy's listener
@@ -401,7 +416,7 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
proxy = wl_map_lookup(&display->objects, id);
if (proxy != WL_ZOMBIE_OBJECT)
- proxy->id_deleted = 1;
+ proxy->flags |= WL_PROXY_FLAG_ID_DELETED;
else
wl_map_remove(&display->objects, id);
@@ -514,6 +529,8 @@ wl_display_connect_to_fd(int fd)
display->proxy.object.implementation = (void(**)(void)) &display_listener;
display->proxy.user_data = display;
display->proxy.queue = &display->queue;
+ display->proxy.flags = 0;
+ display->proxy.refcount = 1;
display->connection = wl_connection_create(display->fd);
if (display->connection == NULL) {
@@ -673,6 +690,31 @@ create_proxies(struct wl_proxy *sender, struct wl_closure *closure)
return 0;
}
+static void
+increase_closure_args_refcount(struct wl_closure *closure)
+{
+ const char *signature;
+ struct argument_details arg;
+ int i, count;
+ struct wl_proxy *proxy;
+
+ signature = closure->message->signature;
+ count = arg_count_for_signature(signature) + 2;
+ for (i = 2; i < count; i++) {
+ signature = get_next_argument(signature, &arg);
+ switch (arg.type) {
+ case 'n':
+ case 'o':
+ proxy = *(struct wl_proxy **) closure->args[i];
+ if (proxy)
+ proxy->refcount++;
+ break;
+ default:
+ break;
+ }
+ }
+}
+
static int
queue_event(struct wl_display *display, int len)
{
@@ -709,6 +751,15 @@ queue_event(struct wl_display *display, int len)
return -1;
}
+ if (wl_closure_lookup_objects(closure, &display->objects) != 0) {
+ wl_closure_destroy(closure);
+ return -1;
+ }
+
+ increase_closure_args_refcount(closure);
+ proxy->refcount++;
+ closure->proxy = proxy;
+
if (wl_list_empty(&proxy->queue->event_list))
pthread_cond_signal(&proxy->queue->cond);
wl_list_insert(proxy->queue->event_list.prev, &closure->link);
@@ -717,31 +768,68 @@ queue_event(struct wl_display *display, int len)
}
static void
+decrease_closure_args_refcount(struct wl_closure *closure)
+{
+ const char *signature;
+ struct argument_details arg;
+ int i, count;
+ struct wl_proxy *proxy;
+
+ signature = closure->message->signature;
+ count = arg_count_for_signature(signature) + 2;
+ for (i = 2; i < count; i++) {
+ signature = get_next_argument(signature, &arg);
+ switch (arg.type) {
+ case 'n':
+ case 'o':
+ proxy = *(struct wl_proxy **) closure->args[i];
+ if (proxy) {
+ if (proxy->flags & WL_PROXY_FLAG_DESTROYED)
+ *(void **) closure->args[i] = NULL;
+
+ proxy->refcount--;
+ if (!proxy->refcount)
+ free(proxy);
+ }
+ break;
+ default:
+ break;
+ }
+ }
+}
+
+static void
dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
{
struct wl_closure *closure;
struct wl_proxy *proxy;
- uint32_t id;
- int opcode, ret;
+ int opcode;
+ bool proxy_destroyed;
closure = container_of(queue->event_list.next,
struct wl_closure, link);
wl_list_remove(&closure->link);
- id = closure->buffer[0];
opcode = closure->buffer[1] & 0xffff;
- /* Verify that the receiving object is still valid and look up
- * proxies for any arguments. We have to do this just before
- * calling the handler, since preceeding events may have
- * destroyed either the proxy or the proxy args since the
- * event was queued. */
- proxy = wl_map_lookup(&display->objects, id);
- ret = wl_closure_lookup_objects(closure, &display->objects);
+ /* Verify that the receiving object is still valid by checking if has
+ * been destroyed by the application. */
+
+ decrease_closure_args_refcount(closure);
+ proxy = closure->proxy;
+ proxy_destroyed = !!(proxy->flags & WL_PROXY_FLAG_DESTROYED);
+
+ proxy->refcount--;
+ if (!proxy->refcount)
+ free(proxy);
+
+ if (proxy_destroyed) {
+ wl_closure_destroy(closure);
+ return;
+ }
pthread_mutex_unlock(&display->mutex);
- if (proxy != WL_ZOMBIE_OBJECT &&
- proxy->object.implementation && ret == 0) {
+ if (proxy->object.implementation) {
if (wl_debug)
wl_closure_print(closure, &proxy->object, false);
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 1b98ce2..4ec9896 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -59,6 +59,7 @@ void wl_map_for_each(struct wl_map *map, wl_iterator_func_t func, void *data);
struct wl_connection;
struct wl_closure;
+struct wl_proxy;
struct wl_connection *wl_connection_create(int fd);
void wl_connection_destroy(struct wl_connection *connection);
@@ -80,6 +81,7 @@ struct wl_closure {
void *args[20];
uint32_t *start;
struct wl_list link;
+ struct wl_proxy *proxy;
uint32_t buffer[0];
};
--
1.7.10.4
More information about the wayland-devel
mailing list