[PATCH 1/3] client: Send errors should be fatal but not abort

[Lloyd Pique]

If the core-client code encounters an error when doing an internal flush
because the send buffers are full, the previous behavior is to
wl_abort(). This is not very friendly if the client is a nested server
running as a service.

This patch instead sets a fatal display error, which is one step
better, and causes the send logic to do nothing once a fatal display
error is set. The client should eventually fall back to its main loop,
and be able to detect the fatal error there.

The existing display test is extended to exercise the error state.

Signed-off-by: Lloyd Pique <***@google.com>
---
COPYING | 1 +
src/wayland-client.c | 9 +++++-
tests/display-test.c | 77 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/COPYING b/COPYING
index eb25a4e..bace5d7 100644
--- a/COPYING
+++ b/COPYING
@@ -2,6 +2,7 @@ Copyright © 2008-2012 Kristian HÞgsberg
Copyright © 2010-2012 Intel Corporation
Copyright © 2011 Benjamin Franzke
Copyright © 2012 Collabora, Ltd.
+Copyright © 2018 Google LLC

Permission is hereby granted, free of charge, to any person obtaining a
copy of this software and associated documentation files (the "Software"),
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 0ccfc66..29ae1e0 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -736,6 +736,9 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
goto err_unlock;
}

+ if (proxy->display->last_error)
+ goto err_unlock;
+
closure = wl_closure_marshal(&proxy->object, opcode, args, message);
if (closure == NULL)
wl_abort("Error marshalling request: %s\n", strerror(errno));
@@ -744,7 +747,11 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
wl_closure_print(closure, &proxy->object, true);

if (wl_closure_send(closure, proxy->display->connection))
- wl_abort("Error sending request: %s\n", strerror(errno));
+ // Convert EAGAIN into EPIPE, since EAGAIN is not really
+ // supposed to be fatal, especially when returned by
+ // wl_display_flush().
+ display_fatal_error(proxy->display,
+ errno != EAGAIN ? errno : EPIPE);

wl_closure_destroy(closure);

diff --git a/tests/display-test.c b/tests/display-test.c
index 9b49a0e..23f0635 100644
--- a/tests/display-test.c
+++ b/tests/display-test.c
@@ -1315,3 +1315,80 @@ TEST(zombie_fd_errant_consumption)

display_destroy(d);
}
+
+static void
+terminate_on_bind_attempt(struct wl_client *client, void *data,
+ uint32_t version, uint32_t id)
+{
+ wl_display_terminate(((struct display *)data)->wl_display);
+}
+
+static void
+fill_client_send_buffers(struct wl_display *display,
+ void (*send_check)(struct wl_display *, int *), int *send_check_result)
+{
+ int sync_count = 4096;
+ int socket_buf = 1024;
+
+ /* Use the minimum send buffer size. We want the client to be
+ * able to fill the buffer easily. */
+ assert(setsockopt(wl_display_get_fd(display), SOL_SOCKET, SO_SNDBUF,
+ &socket_buf, sizeof(socket_buf)) == 0);
+
+ /* Fill up the client and server buffers With the default error
+ * handling, this will abort the client. */
+ while(sync_count--) {
+ wl_callback_destroy(wl_display_sync(display));
+ if (send_check != NULL) {
+ send_check(display, send_check_result);
+ }
+ }
+
+ wl_display_roundtrip(display);
+}
+
+static void
+send_error_client(void)
+{
+ struct client *c = client_connect();
+
+ /* Try and bind a wl_set, which isn't used but has a side effect. */
+ wl_proxy_destroy((struct wl_proxy *)client_get_seat(c));
+
+ /* Verify we do not have any errors at this point */
+ assert(wl_display_get_error(c->wl_display) == 0);
+
+ fill_client_send_buffers(c->wl_display, NULL, NULL);
+
+ /* Verify that we see a fatal display error from the send. */
+ assert(wl_display_get_error(c->wl_display) == EPIPE);
+
+ client_disconnect_nocheck(c);
+}
+
+TEST(client_send_error_tst)
+{
+ struct display *d = display_create();
+
+ test_set_timeout(2);
+
+ client_create_noarg(d, send_error_client);
+
+ /* The client must connect before simulating an unresponsive state. Use
+ * the request for a set to terminate the event loop. */
+ wl_global_create(d->wl_display, &wl_seat_interface,
+ 1, d, terminate_on_bind_attempt);
+
+ /* This handles the initial connect, then returns. */
+ display_run(d);
+
+ /* Not processing any events for a moment should allow the buffers
+ * to be filled up by the client, and it to enter an error state then
+ * disconnect. */
+ test_sleep(1);
+
+ /* We need one last dispatch to handle the terminated client. */
+ wl_event_loop_dispatch(wl_display_get_event_loop(d->wl_display), 0);
+
+ display_destroy(d);
+}

[Lloyd Pique]

This allows a client to occasionally flush the output buffers in the
middle of making a large number of calls which fill them.

A soft flush differs from a normal flush in that the buffer is not
flushed if it is not full enough. The current criteria set is the buffer
being half full.

This does not completely replace the need to call wl_display_flush(),
which ensures all the output is actually sent. However the client can
make this call to properly handle any send errors that might occur.

Includes a test to make sure that it can be used in a situation where a
flush is needed and would return an EAGAIN, which would normally
set a fatal display error if the core-client tried to do an internal
flush if the output buffer became completely full.

Signed-off-by: Lloyd Pique <***@google.com>
---
src/connection.c | 30 ++++++++++++++-
src/wayland-client-core.h | 3 ++
src/wayland-client.c | 77 +++++++++++++++++++++++++++++----------
src/wayland-private.h | 2 +-
src/wayland-server.c | 6 +--
tests/connection-test.c | 10 ++---
tests/display-test.c | 62 +++++++++++++++++++++++++++++++
tests/os-wrappers-test.c | 2 +-
8 files changed, 161 insertions(+), 31 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index f965210..c271fa0 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -285,8 +285,31 @@ decode_cmsg(struct wl_buffer *buffer, struct msghdr *msg)
return 0;
}

+static int
+wl_buffer_is_half_full(struct wl_buffer *b)
+{
+ return wl_buffer_size(b) > sizeof(b->data) / 2;
+}
+
+static int
+wl_fds_out_buffer_is_half_full(struct wl_connection* c)
+{
+ return wl_buffer_size(&c->fds_out) > MAX_FDS_OUT * sizeof(int32_t) / 2;
+}
+
+static int
+connection_soft_flush_check(struct wl_connection* connection)
+{
+ /* For a soft flush, We use the the data buffer being half full, or the
+ * fds_out buffer being at half the message limit as the criteria for
+ * actually performing a flush. */
+
+ return wl_buffer_is_half_full(&connection->out) ||
+ wl_fds_out_buffer_is_half_full(connection);
+}
+
int
-wl_connection_flush(struct wl_connection *connection)
+wl_connection_flush(struct wl_connection *connection, int soft)
{
struct iovec iov[2];
struct msghdr msg;
@@ -297,6 +320,9 @@ wl_connection_flush(struct wl_connection *connection)
if (!connection->want_flush)
return 0;

+ if (soft && !connection_soft_flush_check(connection))
+ return 0;
+
tail = connection->out.tail;
while (connection->out.head - connection->out.tail > 0) {
wl_buffer_get_iov(&connection->out, iov, &count);
@@ -400,7 +426,7 @@ wl_connection_queue(struct wl_connection *connection,
if (connection->out.head - connection->out.tail +
count > ARRAY_LENGTH(connection->out.data)) {
connection->want_flush = 1;
- if (wl_connection_flush(connection) < 0)
+ if (wl_connection_flush(connection, 0) < 0)
return -1;
}

diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
index 03e781b..84b3b41 100644
--- a/src/wayland-client-core.h
+++ b/src/wayland-client-core.h
@@ -234,6 +234,9 @@ wl_display_get_protocol_error(struct wl_display *display,
int
wl_display_flush(struct wl_display *display);

+int
+wl_display_soft_flush(struct wl_display *display);
+
int
wl_display_roundtrip_queue(struct wl_display *display,
struct wl_event_queue *queue);
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 29ae1e0..eea634d 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1957,6 +1957,31 @@ wl_display_get_protocol_error(struct wl_display *display,
return ret;
}

+static int
+display_flush(struct wl_display *display, int soft)
+{
+ int ret;
+
+ pthread_mutex_lock(&display->mutex);
+
+ if (display->last_error) {
+ errno = display->last_error;
+ ret = -1;
+ } else {
+ /* We don't make EPIPE a fatal error here, so that we may try to
+ * read events after the failed flush. When the compositor sends
+ * an error it will close the socket, and if we make EPIPE fatal
+ * here we don't get a chance to process the error. */
+ ret = wl_connection_flush(display->connection, soft);
+ if (ret < 0 && errno != EAGAIN && errno != EPIPE)
+ display_fatal_error(display, errno);
+ }
+
+ pthread_mutex_unlock(&display->mutex);
+
+ return ret;
+}
+

/** Send all buffered requests on the display to the server
*
@@ -1978,26 +2003,40 @@ wl_display_get_protocol_error(struct wl_display *display,
WL_EXPORT int
wl_display_flush(struct wl_display *display)
{
- int ret;
-
- pthread_mutex_lock(&display->mutex);
-
- if (display->last_error) {
- errno = display->last_error;
- ret = -1;
- } else {
- /* We don't make EPIPE a fatal error here, so that we may try to
- * read events after the failed flush. When the compositor sends
- * an error it will close the socket, and if we make EPIPE fatal
- * here we don't get a chance to process the error. */
- ret = wl_connection_flush(display->connection);
- if (ret < 0 && errno != EAGAIN && errno != EPIPE)
- display_fatal_error(display, errno);
- }
-
- pthread_mutex_unlock(&display->mutex);
+ return display_flush(display, 0);
+}

- return ret;
+/** Performs a wl_display_flush() only if the buffer is getting full
+ *
+ * \param display The display context object
+ * \return The number of bytes sent on success, or -1 on failure.
+ *
+ * Performs a conditional flush of buffered data to the server. The data is
+ * only actually flushed if a significant amount of data has been buffered. If
+ * a flush is performed, this function is is equivalent to calling
+ * wl_display_flush(). If a flush is not needed, this function just returns
+ * zero without performing a flush.
+ *
+ * Clients should call this function occasionally while making a large number
+ * of message output calls, where there is a danger of the output buffer
+ * being filled. If the buffers do get completely full, the library will
+ * try to do an internal flush, but if an error occurs the only option
+ * is to set a fatal display error.
+ *
+ * Clients should also check for a return value of -1, check errno, and respond
+ * appropriately, such as polling on the display file descriptor on EAGAIN.
+ *
+ * Clients should still call wl_display_flush() to ensure all output is
+ * completely flushed before blocking on input from the display fd.
+ *
+ * wl_display_soft_flush() does not block.
+ *
+ * \memberof wl_display
+ */
+WL_EXPORT int
+wl_display_soft_flush(struct wl_display *display)
+{
+ return display_flush(display, 1);
}

/** Set the user data associated with a proxy
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 29516ec..ba183fc 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -121,7 +121,7 @@ void
wl_connection_consume(struct wl_connection *connection, size_t size);

int
-wl_connection_flush(struct wl_connection *connection);
+wl_connection_flush(struct wl_connection *connection, int soft);

uint32_t
wl_connection_pending_input(struct wl_connection *connection);
diff --git a/src/wayland-server.c b/src/wayland-server.c
index eae8d2e..43e4099 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -332,7 +332,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
}

if (mask & WL_EVENT_WRITABLE) {
- len = wl_connection_flush(connection);
+ len = wl_connection_flush(connection, 0);
if (len < 0 && errno != EAGAIN) {
destroy_client_with_error(
client, "failed to flush client connection");
@@ -454,7 +454,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
WL_EXPORT void
wl_client_flush(struct wl_client *client)
{
- wl_connection_flush(client->connection);
+ wl_connection_flush(client->connection, 0);
}

/** Get the display object for the given client
@@ -1268,7 +1268,7 @@ wl_display_flush_clients(struct wl_display *display)
int ret;

wl_list_for_each_safe(client, next, &display->client_list, link) {
- ret = wl_connection_flush(client->connection);
+ ret = wl_connection_flush(client->connection, 0);
if (ret < 0 && errno == EAGAIN) {
wl_event_source_fd_update(client->source,
WL_EVENT_WRITABLE |
diff --git a/tests/connection-test.c b/tests/connection-test.c
index 018e2ac..4248f4a 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -76,7 +76,7 @@ TEST(connection_write)
connection = setup(s);

assert(wl_connection_write(connection, message, sizeof message) == 0);
- assert(wl_connection_flush(connection) == sizeof message);
+ assert(wl_connection_flush(connection, 0) == sizeof message);
assert(read(s[1], buffer, sizeof buffer) == sizeof message);
assert(memcmp(message, buffer, sizeof message) == 0);

@@ -118,9 +118,9 @@ TEST(connection_queue)
* we receive the two messages on the other fd. */

assert(wl_connection_queue(connection, message, sizeof message) == 0);
- assert(wl_connection_flush(connection) == 0);
+ assert(wl_connection_flush(connection, 0) == 0);
assert(wl_connection_write(connection, message, sizeof message) == 0);
- assert(wl_connection_flush(connection) == 2 * sizeof message);
+ assert(wl_connection_flush(connection, 0) == 2 * sizeof message);
assert(read(s[1], buffer, sizeof buffer) == 2 * sizeof message);
assert(memcmp(message, buffer, sizeof message) == 0);
assert(memcmp(message, buffer + sizeof message, sizeof message) == 0);
@@ -212,7 +212,7 @@ marshal(struct marshal_data *data, const char *format, int size, ...)
assert(closure);
assert(wl_closure_send(closure, data->write_connection) == 0);
wl_closure_destroy(closure);
- assert(wl_connection_flush(data->write_connection) == size);
+ assert(wl_connection_flush(data->write_connection, 0) == size);
assert(read(data->s[0], data->buffer, sizeof data->buffer) == size);

assert(data->buffer[0] == sender.id);
@@ -476,7 +476,7 @@ marshal_demarshal(struct marshal_data *data,
assert(closure);
assert(wl_closure_send(closure, data->write_connection) == 0);
wl_closure_destroy(closure);
- assert(wl_connection_flush(data->write_connection) == size);
+ assert(wl_connection_flush(data->write_connection, 0) == size);

assert(wl_connection_read(data->read_connection) == size);

diff --git a/tests/display-test.c b/tests/display-test.c
index 23f0635..00eda24 100644
--- a/tests/display-test.c
+++ b/tests/display-test.c
@@ -1392,3 +1392,65 @@ TEST(client_send_error_tst)

display_destroy(d);
}
+
+static void
+display_soft_flush_and_poll(struct wl_display *display, int *eagain_count)
+{
+ struct pollfd pfd;
+
+ /* If a flush is recommended, we should perform one */
+ while (wl_display_soft_flush(display) == -1) {
+ /* For this test, we only expect an EAGAIN error */
+ assert(errno == EAGAIN);
+
+ /* Track the count of EAGAIN */
+ *eagain_count += 1;
+
+ /* Wait for the display to be ready for more output. */
+ pfd.fd = wl_display_get_fd(display);
+ pfd.events = POLLOUT;
+ assert(poll(&pfd, 1, -1) == 1);
+ }
+}
+
+static void
+soft_flush_client(void)
+{
+ int eagain_count = 0;
+ struct client *c = client_connect();
+
+ /* Verify we do not have any errors at this point. */
+ assert(wl_display_get_error(c->wl_display) == 0);
+
+ /* A soft flush should not write anything.*/
+ assert(wl_display_soft_flush(c->wl_display) == 0);
+
+ /* Fill up the client and server buffers With the default error
+ * handling, this will abort the client. */
+ fill_client_send_buffers(c->wl_display, display_soft_flush_and_poll,
+ &eagain_count);
+
+ /* Verify we do not have any errors at this point. */
+ assert(wl_display_get_error(c->wl_display) == 0);
+
+ /* A soft flush should not write anything.*/
+ assert(wl_display_soft_flush(c->wl_display) == 0);
+
+ /* .. but we should have had to wait on at least one flush. */
+ assert(eagain_count > 0);
+
+ client_disconnect_nocheck(c);
+}
+
+TEST(soft_flush_tst)
+{
+ struct display *d = display_create();
+
+ test_set_timeout(2);
+
+ client_create_noarg(d, soft_flush_client);
+
+ display_run(d);
+
+ display_destroy(d);
+}
diff --git a/tests/os-wrappers-test.c b/tests/os-wrappers-test.c
index 102622c..141501d 100644
--- a/tests/os-wrappers-test.c
+++ b/tests/os-wrappers-test.c
@@ -248,7 +248,7 @@ marshal_demarshal(struct marshal_data *data,
assert(closure);
assert(wl_closure_send(closure, data->write_connection) == 0);
wl_closure_destroy(closure);
- assert(wl_connection_flush(data->write_connection) == size);
+ assert(wl_connection_flush(data->write_connection, 0) == size);

assert(wl_connection_read(data->read_connection) == size);

[Lloyd Pique]

To allow more fd's to be buffered, remove the use of the MAX_FDS_OUT
when deciding if there is room in fds_out. A full set of 1024 can now
be enqueued for output.

The consequence is that the flush code must be able to handle sending
more than MAX_FDS_OUT safely. The logic has been changed so that if
there are more than MAX_FDS_OUT of them, that maximum count is sent
along with the data for a single message, rather than the data for all
the messages, as fd's cannot be sent without any data. As the receiver
will always assume it can read at least one message from the input
buffers, one messasge is sent -- it just might have unused fd's.

To keep things sane, an explicit limit to the number of fd's per message
is introduced of three. An error will be generated if there are more.

Signed-off-by: Lloyd Pique <***@google.com>
---
src/connection.c | 93 +++++++++++++++++++-------
src/wayland-client.c | 12 ++--
src/wayland-private.h | 3 +-
src/wayland-server.c | 2 +-
tests/connection-test.c | 141 +++++++++++++++++++++++++++++++++++-----
5 files changed, 205 insertions(+), 46 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index c271fa0..bf77ef8 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -166,6 +166,36 @@ wl_buffer_size(struct wl_buffer *b)
return b->head - b->tail;
}

+static void
+wl_buffer_get_one_msg_iov(struct wl_buffer *b, struct iovec *iov, int *count)
+{
+ uint32_t p[2];
+ int msg_size;
+ uint32_t head, tail;
+
+ wl_buffer_copy(b, p, sizeof p);
+ msg_size = p[1] >> 16;
+
+ head = MASK(b->head);
+ tail = MASK(b->tail);
+
+ if (tail + msg_size <= sizeof b->data) {
+ iov[0].iov_base = b->data + tail;
+ iov[0].iov_len = msg_size;
+ *count = 1;
+ } else if (head == 0) {
+ iov[0].iov_base = b->data + tail;
+ iov[0].iov_len = sizeof b->data - tail;
+ *count = 1;
+ } else {
+ iov[0].iov_base = b->data + tail;
+ iov[0].iov_len = sizeof b->data - tail;
+ iov[1].iov_base = b->data;
+ iov[1].iov_len = msg_size - (sizeof b->data - tail);
+ *count = 2;
+ }
+}
+
struct wl_connection *
wl_connection_create(int fd)
{
@@ -325,7 +355,11 @@ wl_connection_flush(struct wl_connection *connection, int soft)

tail = connection->out.tail;
while (connection->out.head - connection->out.tail > 0) {
- wl_buffer_get_iov(&connection->out, iov, &count);
+ if (wl_buffer_size(&connection->fds_out) > MAX_FDS_OUT * sizeof(int32_t)) {
+ wl_buffer_get_one_msg_iov(&connection->out, iov, &count);
+ } else {
+ wl_buffer_get_iov(&connection->out, iov, &count);
+ }

build_cmsg(&connection->fds_out, cmsg, &clen);

@@ -400,18 +434,18 @@ wl_connection_read(struct wl_connection *connection)
return wl_connection_pending_input(connection);
}

-int
-wl_connection_write(struct wl_connection *connection,
- const void *data, size_t count)
+static int
+connection_buffer_write(struct wl_connection *connection,
+ struct wl_buffer *buffer, const void *data,
+ size_t count)
{
- if (connection->out.head - connection->out.tail +
- count > ARRAY_LENGTH(connection->out.data)) {
+ if (buffer->head - buffer->tail + count > ARRAY_LENGTH(buffer->data)) {
connection->want_flush = 1;
- if (wl_connection_flush(connection) < 0)
+ if (wl_connection_flush(connection, 0) < 0)
return -1;
}

- if (wl_buffer_put(&connection->out, data, count) < 0)
+ if (wl_buffer_put(buffer, data, count) < 0)
return -1;

connection->want_flush = 1;
@@ -419,6 +453,14 @@ wl_connection_write(struct wl_connection *connection,
return 0;
}

+int
+wl_connection_write(struct wl_connection *connection,
+ const void *data, size_t count)
+{
+ return connection_buffer_write(connection, &connection->out, data,
+ count);
+}
+
int
wl_connection_queue(struct wl_connection *connection,
const void *data, size_t count)
@@ -455,13 +497,8 @@ wl_connection_get_fd(struct wl_connection *connection)
static int
wl_connection_put_fd(struct wl_connection *connection, int32_t fd)
{
- if (wl_buffer_size(&connection->fds_out) == MAX_FDS_OUT * sizeof fd) {
- connection->want_flush = 1;
- if (wl_connection_flush(connection) < 0)
- return -1;
- }
-
- return wl_buffer_put(&connection->fds_out, &fd, sizeof fd);
+ return connection_buffer_write(connection, &connection->fds_out, &fd,
+ sizeof fd);
}

const char *
@@ -489,9 +526,10 @@ get_next_argument(const char *signature, struct argument_details *details)
}

int
-arg_count_for_signature(const char *signature)
+arg_count_for_signature(const char *signature, int *out_fd_count)
{
int count = 0;
+ int fd_count = 0;
for(; *signature; ++signature) {
switch(*signature) {
case 'i':
@@ -501,10 +539,14 @@ arg_count_for_signature(const char *signature)
case 'o':
case 'n':
case 'a':
+ ++count;
+ break;
case 'h':
++count;
+ ++fd_count;
}
}
+ if (out_fd_count) *out_fd_count = fd_count;
return count;
}

@@ -583,14 +625,19 @@ wl_closure_init(const struct wl_message *message, uint32_t size,
int *num_arrays, union wl_argument *args)
{
struct wl_closure *closure;
- int count;
+ int count, fd_count;

- count = arg_count_for_signature(message->signature);
+ count = arg_count_for_signature(message->signature, &fd_count);
if (count > WL_CLOSURE_MAX_ARGS) {
wl_log("too many args (%d)\n", count);
errno = EINVAL;
return NULL;
}
+ if (fd_count > WL_CLOSURE_MAX_FD_ARGS) {
+ wl_log("too many fd args (%d)\n", fd_count);
+ errno = EINVAL;
+ return NULL;
+ }

if (size) {
*num_arrays = wl_message_count_arrays(message);
@@ -906,7 +953,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)

message = closure->message;
signature = message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
@@ -1011,7 +1058,7 @@ wl_closure_invoke(struct wl_closure *closure, uint32_t flags,
void * ffi_args[WL_CLOSURE_MAX_ARGS + 2];
void (* const *implementation)(void);

- count = arg_count_for_signature(closure->message->signature);
+ count = arg_count_for_signature(closure->message->signature, NULL);

ffi_types[0] = &ffi_type_pointer;
ffi_args[0] = &data;
@@ -1054,7 +1101,7 @@ copy_fds_to_connection(struct wl_closure *closure,
const char *signature = message->signature;
int fd;

- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
if (arg.type != 'h')
@@ -1083,7 +1130,7 @@ buffer_size_for_closure(struct wl_closure *closure)
uint32_t size, buffer_size = 0;

signature = message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);

@@ -1140,7 +1187,7 @@ serialize_closure(struct wl_closure *closure, uint32_t *buffer,
end = buffer + buffer_count;

signature = message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);

diff --git a/src/wayland-client.c b/src/wayland-client.c
index eea634d..c681458 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -242,7 +242,7 @@ validate_closure_objects(struct wl_closure *closure)
struct wl_proxy *proxy;

signature = closure->message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
@@ -270,7 +270,7 @@ destroy_queued_closure(struct wl_closure *closure)
int i, count;

signature = closure->message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
@@ -354,7 +354,7 @@ message_count_fds(const char *signature)
unsigned int count, i, fds = 0;
struct argument_details arg;

- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
if (arg.type == 'h')
@@ -638,7 +638,7 @@ create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
struct wl_proxy *new_proxy = NULL;

signature = message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);

@@ -1285,7 +1285,7 @@ create_proxies(struct wl_proxy *sender, struct wl_closure *closure)
int count;

signature = closure->message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
@@ -1318,7 +1318,7 @@ increase_closure_args_refcount(struct wl_closure *closure)
struct wl_proxy *proxy;

signature = closure->message->signature;
- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
diff --git a/src/wayland-private.h b/src/wayland-private.h
index ba183fc..9c8f3c7 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -50,6 +50,7 @@
#define WL_MAP_CLIENT_SIDE 1
#define WL_SERVER_ID_START 0xff000000
#define WL_CLOSURE_MAX_ARGS 20
+#define WL_CLOSURE_MAX_FD_ARGS 3

struct wl_object {
const struct wl_interface *interface;
@@ -160,7 +161,7 @@ const char *
get_next_argument(const char *signature, struct argument_details *details);

int
-arg_count_for_signature(const char *signature);
+arg_count_for_signature(const char *signature, int *out_fd_count);

int
wl_message_count_arrays(const struct wl_message *message);
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 43e4099..51eda78 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -176,7 +176,7 @@ verify_objects(struct wl_resource *resource, uint32_t opcode,
struct wl_resource *res;
int count, i;

- count = arg_count_for_signature(signature);
+ count = arg_count_for_signature(signature, NULL);
for (i = 0; i < count; i++) {
signature = get_next_argument(signature, &arg);
switch (arg.type) {
diff --git a/tests/connection-test.c b/tests/connection-test.c
index 4248f4a..4e5fc38 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -43,6 +43,30 @@

static const char message[] = "Hello, world";

+static int
+create_test_fd(void)
+{
+ char f[] = "/tmp/wayland-tests-XXXXXX";
+ int fd = mkstemp(f);
+ assert(fd >= 0);
+ unlink(f);
+ return fd;
+}
+
+static void
+validate_fds_same_and_close(int expected, int actual)
+{
+ struct stat expected_buf, actual_buf;
+
+ assert(actual != expected);
+ fstat(actual, &actual_buf);
+ fstat(expected, &expected_buf);
+ assert(actual_buf.st_dev == expected_buf.st_dev);
+ assert(actual_buf.st_ino == expected_buf.st_ino);
+ close(actual);
+ close(expected);
+}
+
static struct wl_connection *
setup(int *s)
{
@@ -368,15 +392,7 @@ static void
validate_demarshal_h(struct marshal_data *data,
struct wl_object *object, int fd)
{
- struct stat buf1, buf2;
-
- assert(fd != data->value.h);
- fstat(fd, &buf1);
- fstat(data->value.h, &buf2);
- assert(buf1.st_dev == buf2.st_dev);
- assert(buf1.st_ino == buf2.st_ino);
- close(fd);
- close(data->value.h);
+ validate_fds_same_and_close(data->value.h, fd);
}

static void
@@ -492,7 +508,6 @@ marshal_demarshal(struct marshal_data *data,
TEST(connection_marshal_demarshal)
{
struct marshal_data data;
- char f[] = "/tmp/wayland-tests-XXXXXX";

setup_marshal_data(&data);

@@ -512,9 +527,7 @@ TEST(connection_marshal_demarshal)
marshal_demarshal(&data, (void *) validate_demarshal_s,
28, "?s", data.value.s);

- data.value.h = mkstemp(f);
- assert(data.value.h >= 0);
- unlink(f);
+ data.value.h = create_test_fd();
marshal_demarshal(&data, (void *) validate_demarshal_h,
8, "h", data.value.h);

@@ -608,8 +621,7 @@ TEST(connection_marshal_alot)
* for both regular data an fds. */

for (i = 0; i < 2000; i++) {
- strcpy(f, "/tmp/wayland-tests-XXXXXX");
- data.value.h = mkstemp(f);
+ data.value.h = create_test_fd();
assert(data.value.h >= 0);
unlink(f);
marshal_demarshal(&data, (void *) validate_demarshal_h,
@@ -637,6 +649,105 @@ TEST(connection_marshal_too_big)
free(big_string);
}

+static void
+validate_demarshal_hhh(int* expected_fds, struct wl_object *object, int fd1,
+ int fd2, int fd3)
+{
+ validate_fds_same_and_close(expected_fds[0], fd1);
+ validate_fds_same_and_close(expected_fds[1], fd2);
+ validate_fds_same_and_close(expected_fds[2], fd3);
+}
+
+
+
+TEST(connection_marshal_alot_of_fds)
+{
+ struct marshal_data data;
+ struct wl_closure *closure;
+ static const int opcode = 4444;
+ static struct wl_object sender = { NULL, NULL, 1234 };
+ struct wl_message message = { "test", "hhh", NULL };
+ struct wl_map objects;
+ void (*func)(int *, struct wl_object *, int, int, int) = validate_demarshal_hhh;
+ struct wl_object object = { NULL, &func, 0 };
+ uint32_t msg[1] = { 1234 };
+ static const int one_message_bytes = 8;
+ static const int total_bytes = one_message_bytes * 100;
+ int fds[300];
+ int *next_fd;
+ union wl_argument args[WL_CLOSURE_MAX_ARGS];
+ int size;
+
+ setup_marshal_data(&data);
+
+ /* This test enqueues 100 messages that each use 3 fd's, and exercises
+ * the ability of the code to deal with more than MAX_FDS_OUT (the
+ * sendmsg() limit)
+ *
+ * It verifies that all 300 fd's
+ */
+
+ wl_map_init(&objects, WL_MAP_SERVER_SIDE);
+ object.id = msg[0];
+
+ for (int i = 0; i < 300; i++) {
+ fds[i] = create_test_fd();
+ }
+
+ next_fd = fds;
+ for (int i = 0; i < 100; i++) {
+ args[0].h = *next_fd++;
+ args[1].h = *next_fd++;
+ args[2].h = *next_fd++;
+ closure = wl_closure_marshal(&sender, opcode, args, &message);
+ assert(closure);
+ assert(wl_closure_send(closure, data.write_connection) == 0);
+ wl_closure_destroy(closure);
+ }
+
+ assert(next_fd == &fds[300]);
+
+ assert(wl_connection_flush(data.write_connection, 0) == total_bytes);
+
+ next_fd = fds;
+ size = 0;
+ for (int i = 0; i < 100; i++) {
+ if (size == 0)
+ size = wl_connection_read(data.read_connection);
+ closure = wl_connection_demarshal(data.read_connection,
+ one_message_bytes, &objects,
+ &message);
+ assert(closure);
+ wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object,
+ 0, next_fd);
+ wl_closure_destroy(closure);
+
+ size -= one_message_bytes;
+ assert(size >= 0);
+
+ next_fd += 3;
+ }
+
+ assert(next_fd == &fds[300]);
+
+ assert(wl_connection_read(data.read_connection) == -1);
+
+ release_marshal_data(&data);
+}
+
+TEST(connection_marshal_too_many_fds_in_one_message)
+{
+ struct marshal_data data;
+ int fd = create_test_fd();
+
+ setup_marshal_data(&data);
+
+ expected_fail_marshal(EINVAL, "hhhh", fd, fd, fd, fd);
+
+ release_marshal_data(&data);
+ close(fd);
+}
+
static void
marshal_helper(const char *format, void *handler, ...)
{