[2/3] client: Add wl_display_soft_flush()

Submitted by Lloyd Pique on Sept. 8, 2018, 1:14 a.m.

Details

Message ID 20180908011405.102768-2-lpique@google.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Lloyd Pique Sept. 8, 2018, 1:14 a.m.
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 <lpique@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(-)

Patch hide | download patch | download mbox

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);