summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKristian Høgsberg <krh@bitplanet.net>2013-11-14 21:29:06 -0800
committerKristian Høgsberg <krh@bitplanet.net>2013-11-15 20:49:36 -0800
commit853c24e6998f747150e4233cf41bfa8268964cc2 (patch)
treec9ae09bb76bb6b0e06695e95d1c39f9ee9d4b744
parent81c57614d11787c00b8859cbaef650e284b4f188 (diff)
client: Introduce functions to allocate and marshal proxies atomically
The server requires clients to only allocate one ID ahead of the previously highest ID in order to keep the ID range tight. Failure to do so will make the server close the client connection. However, the way we allocate new IDs is racy. The generated code looks like: new_proxy = wl_proxy_create(...); wl_proxy_marshal(proxy, ... new_proxy, ...); If two threads do this at the same time, there's a chance that thread A will allocate a proxy, then get pre-empted by thread B which then allocates a proxy and then passes it to wl_proxy_marshal(). The ID for thread As proxy will be one higher that the currently highest ID, but the ID for thread Bs proxy will be two higher. But since thread B prempted thread A before it could send its new ID, B will send its new ID first, the server will see the ID from thread Bs proxy first, and will reject it. We fix this by introducing wl_proxy_marshal_constructor(). This function is identical to wl_proxy_marshal(), except that it will allocate a wl_proxy for NEW_ID arguments and send it, all under the display mutex. By introducing a new function, we maintain backwards compatibility with older code from the generator, and make sure that the new generated code has an explicit dependency on a new enough libwayland-client.so. A virtual Wayland merit badge goes to Kalle Vahlman, who tracked this down and analyzed the issue. Reported-by: Kalle Vahlman <kalle.vahlman@movial.com>
-rw-r--r--src/Makefile.am2
-rw-r--r--src/scanner.c43
-rw-r--r--src/wayland-client.c211
-rw-r--r--src/wayland-client.h8
4 files changed, 189 insertions, 75 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index 4226f63..fb2df1c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -27,7 +27,7 @@ libwayland_server_la_SOURCES = \
event-loop.c
libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt -lm
-libwayland_client_la_LDFLAGS = -version-info 1:0:1
+libwayland_client_la_LDFLAGS = -version-info 2:0:2
libwayland_client_la_SOURCES = \
wayland-protocol.c \
wayland-client.c
diff --git a/src/scanner.c b/src/scanner.c
index 9624618..a030181 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -699,31 +699,34 @@ emit_stubs(struct wl_list *message_list, struct interface *interface)
"{\n");
if (ret) {
printf("\tstruct wl_proxy *%s;\n\n"
- "\t%s = wl_proxy_create("
- "(struct wl_proxy *) %s,\n",
- ret->name, ret->name, interface->name);
+ "\t%s = wl_proxy_marshal_constructor("
+ "(struct wl_proxy *) %s,\n"
+ "\t\t\t %s_%s, ",
+ ret->name, ret->name,
+ interface->name,
+ interface->uppercase_name,
+ m->uppercase_name);
+
if (ret->interface_name == NULL)
- printf("\t\t\t interface);\n");
+ printf("interface");
else
- printf("\t\t\t &%s_interface);\n",
- ret->interface_name);
-
- printf("\tif (!%s)\n"
- "\t\treturn NULL;\n\n",
- ret->name);
+ printf("&%s_interface", ret->interface_name);
+ } else {
+ printf("\twl_proxy_marshal((struct wl_proxy *) %s,\n"
+ "\t\t\t %s_%s",
+ interface->name,
+ interface->uppercase_name,
+ m->uppercase_name);
}
- printf("\twl_proxy_marshal((struct wl_proxy *) %s,\n"
- "\t\t\t %s_%s",
- interface->name,
- interface->uppercase_name,
- m->uppercase_name);
-
wl_list_for_each(a, &m->arg_list, link) {
- if (a->type == NEW_ID && a->interface_name == NULL)
- printf(", interface->name, version");
- printf(", ");
- printf("%s", a->name);
+ if (a->type == NEW_ID) {
+ if (a->interface_name == NULL)
+ printf(", interface->name, version");
+ printf(", NULL");
+ } else {
+ printf(", %s", a->name);
+ }
}
printf(");\n");
diff --git a/src/wayland-client.c b/src/wayland-client.c
index e92317a..1486b73 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -194,6 +194,29 @@ wl_display_create_queue(struct wl_display *display)
return queue;
}
+static struct wl_proxy *
+proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
+{
+ struct wl_proxy *proxy;
+ struct wl_display *display = factory->display;
+
+ proxy = malloc(sizeof *proxy);
+ if (proxy == NULL)
+ return NULL;
+
+ proxy->object.interface = interface;
+ proxy->object.implementation = NULL;
+ proxy->dispatcher = NULL;
+ proxy->display = display;
+ proxy->queue = factory->queue;
+ proxy->flags = 0;
+ proxy->refcount = 1;
+
+ proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
+
+ return proxy;
+}
+
/** Create a proxy object with a given interface
*
* \param factory Factory proxy object
@@ -216,23 +239,11 @@ wl_display_create_queue(struct wl_display *display)
WL_EXPORT struct wl_proxy *
wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
{
- struct wl_proxy *proxy;
struct wl_display *display = factory->display;
-
- proxy = malloc(sizeof *proxy);
- if (proxy == NULL)
- return NULL;
-
- proxy->object.interface = interface;
- proxy->object.implementation = NULL;
- proxy->dispatcher = NULL;
- proxy->display = display;
- proxy->queue = factory->queue;
- proxy->flags = 0;
- proxy->refcount = 1;
+ struct wl_proxy *proxy;
pthread_mutex_lock(&display->mutex);
- proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
+ proxy = proxy_create(factory, interface);
pthread_mutex_unlock(&display->mutex);
return proxy;
@@ -382,27 +393,107 @@ wl_proxy_add_dispatcher(struct wl_proxy *proxy,
return 0;
}
+static struct wl_proxy *
+create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
+ union wl_argument *args,
+ const struct wl_interface *interface)
+{
+ int i, count;
+ const char *signature;
+ struct argument_details arg;
+ struct wl_proxy *new_proxy = NULL;
+
+ signature = message->signature;
+ count = arg_count_for_signature(signature);
+ for (i = 0; i < count; i++) {
+ signature = get_next_argument(signature, &arg);
+
+ switch (arg.type) {
+ case 'n':
+ new_proxy = proxy_create(proxy, interface);
+ if (new_proxy == NULL)
+ return NULL;
+
+ args[i].o = &new_proxy->object;
+ break;
+ }
+ }
+
+ return new_proxy;
+}
+
/** Prepare a request to be sent to the compositor
*
* \param proxy The proxy object
* \param opcode Opcode of the request to be sent
- * \param ... Extra arguments for the given request
+ * \param args Extra arguments for the given request
+ * \param iterface The interface to use for the new proxy
*
* Translates the request given by opcode and the extra arguments into the
- * wire format and write it to the connection buffer.
+ * wire format and write it to the connection buffer. This version takes an
+ * array of the union type wl_argument.
*
- * The example below creates a proxy object with the wl_surface_interface
- * using a wl_compositor factory interface and sends the
- * \c compositor.create_surface request using \ref wl_proxy_marshal(). Note
- * the \c id is the extra argument to the request as specified by the
- * protocol.
+ * For new-id arguments, this function will allocate a new wl_proxy
+ * and send the ID to the server. The new wl_proxy will be returned
+ * on success or NULL on errror with errno set accordingly.
+ *
+ * \note This is intended to be used by language bindings and not in
+ * non-generated code.
*
- * \code
- * id = wl_proxy_create((struct wl_proxy *) wl_compositor,
- * &wl_surface_interface);
- * wl_proxy_marshal((struct wl_proxy *) wl_compositor,
- * WL_COMPOSITOR_CREATE_SURFACE, id);
- * \endcode
+ * \sa wl_proxy_marshal()
+ *
+ * \memberof wl_proxy
+ */
+WL_EXPORT struct wl_proxy *
+wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
+ uint32_t opcode, union wl_argument *args,
+ const struct wl_interface *interface)
+{
+ struct wl_closure *closure;
+ struct wl_proxy *new_proxy = NULL;
+ const struct wl_message *message;
+
+ pthread_mutex_lock(&proxy->display->mutex);
+
+ message = &proxy->object.interface->methods[opcode];
+ if (interface) {
+ new_proxy = create_outgoing_proxy(proxy, message,
+ args, interface);
+ if (new_proxy == NULL)
+ goto err_unlock;
+ }
+
+ closure = wl_closure_marshal(&proxy->object, opcode, args, message);
+ if (closure == NULL) {
+ fprintf(stderr, "Error marshalling request\n");
+ abort();
+ }
+
+ if (wl_debug)
+ wl_closure_print(closure, &proxy->object, true);
+
+ if (wl_closure_send(closure, proxy->display->connection)) {
+ fprintf(stderr, "Error sending request: %m\n");
+ abort();
+ }
+
+ wl_closure_destroy(closure);
+
+ err_unlock:
+ pthread_mutex_unlock(&proxy->display->mutex);
+
+ return new_proxy;
+}
+
+
+/** Prepare a request to be sent to the compositor
+ *
+ * \param proxy The proxy object
+ * \param opcode Opcode of the request to be sent
+ * \param ... Extra arguments for the given request
+ *
+ * This function is similar to wl_proxy_marshal_constructor(), except
+ * it doesn't create proxies for new-id arguments.
*
* \note This should not normally be used by non-generated code.
*
@@ -421,18 +512,52 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
args, WL_CLOSURE_MAX_ARGS, ap);
va_end(ap);
- wl_proxy_marshal_array(proxy, opcode, args);
+ wl_proxy_marshal_array_constructor(proxy, opcode, args, NULL);
}
/** Prepare a request to be sent to the compositor
*
* \param proxy The proxy object
* \param opcode Opcode of the request to be sent
- * \param args Extra arguments for the given request
+ * \param iterface The interface to use for the new proxy
+ * \param ... Extra arguments for the given request
+ * \return A new wl_proxy for the new_id argument or NULL on error
*
* Translates the request given by opcode and the extra arguments into the
- * wire format and write it to the connection buffer. This version takes an
- * array of the union type wl_argument.
+ * wire format and write it to the connection buffer.
+ *
+ * For new-id arguments, this function will allocate a new wl_proxy
+ * and send the ID to the server. The new wl_proxy will be returned
+ * on success or NULL on errror with errno set accordingly.
+ *
+ * \note This should not normally be used by non-generated code.
+ *
+ * \memberof wl_proxy
+ */
+WL_EXPORT struct wl_proxy *
+wl_proxy_marshal_constructor(struct wl_proxy *proxy, uint32_t opcode,
+ const struct wl_interface *interface, ...)
+{
+ union wl_argument args[WL_CLOSURE_MAX_ARGS];
+ va_list ap;
+
+ va_start(ap, interface);
+ wl_argument_from_va_list(proxy->object.interface->methods[opcode].signature,
+ args, WL_CLOSURE_MAX_ARGS, ap);
+ va_end(ap);
+
+ return wl_proxy_marshal_array_constructor(proxy, opcode,
+ args, interface);
+}
+
+/** Prepare a request to be sent to the compositor
+ *
+ * \param proxy The proxy object
+ * \param opcode Opcode of the request to be sent
+ * \param args Extra arguments for the given request
+ *
+ * This function is similar to wl_proxy_marshal_array_constructor(), except
+ * it doesn't create proxies for new-id arguments.
*
* \note This is intended to be used by language bindings and not in
* non-generated code.
@@ -445,29 +570,7 @@ WL_EXPORT void
wl_proxy_marshal_array(struct wl_proxy *proxy, uint32_t opcode,
union wl_argument *args)
{
- struct wl_closure *closure;
-
- pthread_mutex_lock(&proxy->display->mutex);
-
- closure = wl_closure_marshal(&proxy->object, opcode, args,
- &proxy->object.interface->methods[opcode]);
-
- if (closure == NULL) {
- fprintf(stderr, "Error marshalling request\n");
- abort();
- }
-
- if (wl_debug)
- wl_closure_print(closure, &proxy->object, true);
-
- if (wl_closure_send(closure, proxy->display->connection)) {
- fprintf(stderr, "Error sending request: %m\n");
- abort();
- }
-
- wl_closure_destroy(closure);
-
- pthread_mutex_unlock(&proxy->display->mutex);
+ wl_proxy_marshal_array_constructor(proxy, opcode, args, NULL);
}
static void
diff --git a/src/wayland-client.h b/src/wayland-client.h
index 43ba3fc..2a32785 100644
--- a/src/wayland-client.h
+++ b/src/wayland-client.h
@@ -126,6 +126,14 @@ 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_marshal_constructor(struct wl_proxy *proxy,
+ uint32_t opcode,
+ const struct wl_interface *interface,
+ ...);
+struct wl_proxy *
+wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
+ uint32_t opcode, union wl_argument *args,
+ const struct wl_interface *interface);
void wl_proxy_destroy(struct wl_proxy *proxy);
int wl_proxy_add_listener(struct wl_proxy *proxy,