[PATCH 7/8] Split the global registry into its own wl_registry object
Kristian Høgsberg
krh at bitplanet.net
Tue Oct 9 19:38:04 PDT 2012
The only way to make the global object listener interface thread safe is to
make it its own interface and make different listeners different wl_proxies.
The core of the problem is the callback we do when a global show up or
disappears, which we can't do with a lock held. On the other hand we can't
iterate the global list or the listener list without a lock held as new
globals or listeners may come and go during the iteration.
Making a copy of the list under the lock and then iterating after dropping
the lock wont work either. In case of the listener list, once we drop the
lock another thread may unregister a listener and destroy the callbackk
data, which means that when we eventually call that listener we'll pass it
free memory and break everything.
We did already solve the thread-safe callback problem, however. It's what
we do for all protocol events. So we can just make the global registry
functionality its own new interface and give each thread its own proxy.
That way, the thread will do its own callbacks (with no locks held) and
destroy the proxy when it's no longer interested in wl_registry events.
---
protocol/wayland.xml | 37 ++++++++++-------
src/wayland-client.c | 110 --------------------------------------------------
src/wayland-client.h | 16 --------
src/wayland-server.c | 71 ++++++++++++++++++++++----------
4 files changed, 72 insertions(+), 162 deletions(-)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 86b82b8..3f2d628 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -32,14 +32,6 @@
The core global object. This is a special singleton object. It
is used for internal wayland protocol features.
</description>
- <request name="bind">
- <description summary="bind an object to the display">
- Binds a new, client-created object to the server using @name as
- the identifier.
- </description>
- <arg name="name" type="uint" summary="unique number id for object"/>
- <arg name="id" type="new_id"/>
- </request>
<request name="sync">
<description summary="asynchronous roundtrip">
@@ -51,6 +43,10 @@
<arg name="callback" type="new_id" interface="wl_callback"/>
</request>
+ <request name="get_registry">
+ <arg name="callback" type="new_id" interface="wl_registry"/>
+ </request>
+
<event name="error">
<description summary="fatal error event">
The error event is sent out when a fatal (non-recoverable)
@@ -74,6 +70,24 @@
summary="server is out of memory"/>
</enum>
+ <event name="delete_id">
+ <description summary="acknowledge object id deletion">
+ Server has deleted the id and client can now reuse it.
+ </description>
+ <arg name="id" type="uint" />
+ </event>
+ </interface>
+
+ <interface name="wl_registry" version="1">
+ <request name="bind">
+ <description summary="bind an object to the display">
+ Binds a new, client-created object to the server using @name as
+ the identifier.
+ </description>
+ <arg name="name" type="uint" summary="unique number id for object"/>
+ <arg name="id" type="new_id"/>
+ </request>
+
<event name="global">
<description summary="announce global object">
Notify the client of global objects. These are objects that
@@ -96,13 +110,6 @@
</description>
<arg name="name" type="uint"/>
</event>
-
- <event name="delete_id">
- <description summary="acknowledge object id deletion">
- Server has deleted the id and client can now reuse it.
- </description>
- <arg name="id" type="uint" />
- </event>
</interface>
<interface name="wl_callback" version="1">
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 5cbee61..0b988fd 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -42,12 +42,6 @@
#include "wayland-client.h"
#include "wayland-private.h"
-struct wl_global_listener {
- wl_display_global_func_t handler;
- void *data;
- struct wl_list link;
-};
-
struct wl_proxy {
struct wl_object object;
struct wl_display *display;
@@ -75,13 +69,8 @@ struct wl_display {
int close_fd;
pthread_t display_thread;
struct wl_map objects;
- struct wl_list global_listener_list;
- struct wl_list global_list;
struct wl_event_queue queue;
pthread_mutex_t mutex;
-
- wl_display_global_func_t global_handler;
- void *global_handler_data;
};
static int wl_debug = 0;
@@ -128,36 +117,6 @@ wl_display_create_queue(struct wl_display *display)
return queue;
}
-WL_EXPORT struct wl_global_listener *
-wl_display_add_global_listener(struct wl_display *display,
- wl_display_global_func_t handler, void *data)
-{
- struct wl_global_listener *listener;
- struct wl_global *global;
-
- listener = malloc(sizeof *listener);
- if (listener == NULL)
- return NULL;
-
- listener->handler = handler;
- listener->data = data;
- wl_list_insert(display->global_listener_list.prev, &listener->link);
-
- wl_list_for_each(global, &display->global_list, link)
- (*listener->handler)(display, global->id, global->interface,
- global->version, listener->data);
-
- return listener;
-}
-
-WL_EXPORT void
-wl_display_remove_global_listener(struct wl_display *display,
- struct wl_global_listener *listener)
-{
- wl_list_remove(&listener->link);
- free(listener);
-}
-
WL_EXPORT struct wl_proxy *
wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
{
@@ -272,22 +231,6 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
pthread_mutex_unlock(&proxy->display->mutex);
}
-/* Can't do this, there may be more than one instance of an
- * interface... */
-WL_EXPORT uint32_t
-wl_display_get_global(struct wl_display *display,
- const char *interface, uint32_t version)
-{
- struct wl_global *global;
-
- wl_list_for_each(global, &display->global_list, link)
- if (strcmp(interface, global->interface) == 0 &&
- version <= global->version)
- return global->id;
-
- return 0;
-}
-
static void
display_handle_error(void *data,
struct wl_display *display, struct wl_object *object,
@@ -299,46 +242,6 @@ display_handle_error(void *data,
}
static void
-display_handle_global(void *data,
- struct wl_display *display,
- uint32_t id, const char *interface, uint32_t version)
-{
- struct wl_global_listener *listener;
- struct wl_global *global;
-
- global = malloc(sizeof *global);
- global->id = id;
- global->interface = strdup(interface);
- global->version = version;
- wl_list_insert(display->global_list.prev, &global->link);
-
- wl_list_for_each(listener, &display->global_listener_list, link)
- (*listener->handler)(display,
- id, interface, version, listener->data);
-}
-
-static void
-wl_global_destroy(struct wl_global *global)
-{
- wl_list_remove(&global->link);
- free(global->interface);
- free(global);
-}
-
-static void
-display_handle_global_remove(void *data,
- struct wl_display *display, uint32_t id)
-{
- struct wl_global *global;
-
- wl_list_for_each(global, &display->global_list, link)
- if (global->id == id) {
- wl_global_destroy(global);
- break;
- }
-}
-
-static void
display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
{
struct wl_proxy *proxy;
@@ -356,8 +259,6 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
static const struct wl_display_listener display_listener = {
display_handle_error,
- display_handle_global,
- display_handle_global_remove,
display_handle_delete_id
};
@@ -435,8 +336,6 @@ wl_display_connect_to_fd(int fd)
display->fd = fd;
wl_map_init(&display->objects);
- wl_list_init(&display->global_listener_list);
- wl_list_init(&display->global_list);
wl_event_queue_init(&display->queue);
wl_map_insert_new(&display->objects, WL_MAP_CLIENT_SIDE, NULL);
@@ -494,18 +393,9 @@ wl_display_connect(const char *name)
WL_EXPORT void
wl_display_disconnect(struct wl_display *display)
{
- struct wl_global *global, *gnext;
- struct wl_global_listener *listener, *lnext;
-
wl_connection_destroy(display->connection);
wl_map_release(&display->objects);
wl_event_queue_release(&display->queue);
- wl_list_for_each_safe(global, gnext,
- &display->global_list, link)
- wl_global_destroy(global);
- wl_list_for_each_safe(listener, lnext,
- &display->global_listener_list, link)
- free(listener);
if (display->close_fd)
close(display->fd);
diff --git a/src/wayland-client.h b/src/wayland-client.h
index aa92afb..f064010 100644
--- a/src/wayland-client.h
+++ b/src/wayland-client.h
@@ -68,22 +68,6 @@ int wl_display_flush(struct wl_display *display);
void wl_display_roundtrip(struct wl_display *display);
struct wl_event_queue *wl_display_create_queue(struct wl_display *display);
-struct wl_global_listener;
-typedef void (*wl_display_global_func_t)(struct wl_display *display,
- uint32_t id,
- const char *interface,
- uint32_t version,
- void *data);
-void
-wl_display_remove_global_listener(struct wl_display *display,
- struct wl_global_listener *listener);
-struct wl_global_listener *
-wl_display_add_global_listener(struct wl_display *display,
- wl_display_global_func_t handler, void *data);
-uint32_t
-wl_display_get_global(struct wl_display *display,
- const char *interface, uint32_t version);
-
void wl_log_set_handler_client(wl_log_func_t handler);
#ifdef __cplusplus
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 41b515b..474cd1b 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -86,6 +86,7 @@ struct wl_display {
uint32_t id;
uint32_t serial;
+ struct wl_list registry_resource_list;
struct wl_list global_list;
struct wl_list socket_list;
struct wl_list client_list;
@@ -901,9 +902,9 @@ wl_pointer_end_grab(struct wl_pointer *pointer)
}
static void
-display_bind(struct wl_client *client,
- struct wl_resource *resource, uint32_t name,
- const char *interface, uint32_t version, uint32_t id)
+registry_bind(struct wl_client *client,
+ struct wl_resource *resource, uint32_t name,
+ const char *interface, uint32_t version, uint32_t id)
{
struct wl_global *global;
struct wl_display *display = resource->data;
@@ -920,6 +921,10 @@ display_bind(struct wl_client *client,
global->bind(client, global->data, version, id);
}
+static const struct wl_registry_interface registry_interface = {
+ registry_bind
+};
+
static void
display_sync(struct wl_client *client,
struct wl_resource *resource, uint32_t id)
@@ -934,9 +939,40 @@ display_sync(struct wl_client *client,
wl_resource_destroy(callback);
}
-struct wl_display_interface display_interface = {
- display_bind,
+static void
+unbind_resource(struct wl_resource *resource)
+{
+ wl_list_remove(&resource->link);
+ free(resource);
+}
+
+static void
+display_get_registry(struct wl_client *client,
+ struct wl_resource *resource, uint32_t id)
+{
+ struct wl_display *display = resource->data;
+ struct wl_resource *registry_resource;
+ struct wl_global *global;
+
+ registry_resource =
+ wl_client_add_object(client, &wl_registry_interface,
+ ®istry_interface, id, display);
+ registry_resource->destroy = unbind_resource;
+
+ wl_list_insert(&display->registry_resource_list,
+ ®istry_resource->link);
+
+ wl_list_for_each(global, &display->global_list, link)
+ wl_resource_post_event(registry_resource,
+ WL_REGISTRY_GLOBAL,
+ global->name,
+ global->interface->name,
+ global->interface->version);
+}
+
+static const struct wl_display_interface display_interface = {
display_sync,
+ display_get_registry
};
static void
@@ -951,19 +987,11 @@ bind_display(struct wl_client *client,
void *data, uint32_t version, uint32_t id)
{
struct wl_display *display = data;
- struct wl_global *global;
client->display_resource =
wl_client_add_object(client, &wl_display_interface,
&display_interface, id, display);
client->display_resource->destroy = destroy_client_display_resource;
-
- wl_list_for_each(global, &display->global_list, link)
- wl_resource_post_event(client->display_resource,
- WL_DISPLAY_GLOBAL,
- global->name,
- global->interface->name,
- global->interface->version);
}
WL_EXPORT struct wl_display *
@@ -989,6 +1017,7 @@ wl_display_create(void)
wl_list_init(&display->global_list);
wl_list_init(&display->socket_list);
wl_list_init(&display->client_list);
+ wl_list_init(&display->registry_resource_list);
display->id = 1;
display->serial = 0;
@@ -1031,7 +1060,7 @@ wl_display_add_global(struct wl_display *display,
void *data, wl_global_bind_func_t bind)
{
struct wl_global *global;
- struct wl_client *client;
+ struct wl_resource *resource;
global = malloc(sizeof *global);
if (global == NULL)
@@ -1043,9 +1072,9 @@ wl_display_add_global(struct wl_display *display,
global->bind = bind;
wl_list_insert(display->global_list.prev, &global->link);
- wl_list_for_each(client, &display->client_list, link)
- wl_resource_post_event(client->display_resource,
- WL_DISPLAY_GLOBAL,
+ wl_list_for_each(resource, &display->registry_resource_list, link)
+ wl_resource_post_event(resource,
+ WL_REGISTRY_GLOBAL,
global->name,
global->interface->name,
global->interface->version);
@@ -1056,11 +1085,11 @@ wl_display_add_global(struct wl_display *display,
WL_EXPORT void
wl_display_remove_global(struct wl_display *display, struct wl_global *global)
{
- struct wl_client *client;
+ struct wl_resource *resource;
- wl_list_for_each(client, &display->client_list, link)
- wl_resource_post_event(client->display_resource,
- WL_DISPLAY_GLOBAL_REMOVE, global->name);
+ wl_list_for_each(resource, &display->registry_resource_list, link)
+ wl_resource_post_event(resource, WL_REGISTRY_GLOBAL_REMOVE,
+ global->name);
wl_list_remove(&global->link);
free(global);
}
--
1.7.10.2
More information about the wayland-devel
mailing list