Allow xwayland to start lazily

Makes the xwayland startup process two phased.
The first phase just initialises the X11 sockets.
The second phase starts the Xwayland server itself.
When starting xwayland lazily the second phase will be postponed until
a client has connected to the X11 socket.

Changes in behaviour:
The DISPLAY environment is now set immediately after the X11 sockets
are created.
When the Xwayland server is killed or crashes, the sockets will not be
recreated, but reused.

Fixes #849: Start up Xwayland lazily

include/wlr/xwayland.h

@@ -15,10 +15,12 @@ struct wlr_xwayland {
 	pid_t pid;
 	int display;
 	int x_fd[2], wl_fd[2], wm_fd[2];
+	struct wl_event_source *x_fd_read_event[2];
 	struct wl_client *client;
 	struct wl_display *wl_display;
 	struct wlr_compositor *compositor;
 	time_t server_start;
+	bool lazy;
 
 	struct wl_event_source *sigusr1_source;
 	struct wl_listener client_destroy;
@@ -168,7 +170,7 @@ struct wlr_xwayland_resize_event {
 };
 
 struct wlr_xwayland *wlr_xwayland_create(struct wl_display *wl_display,
-	struct wlr_compositor *compositor);
+	struct wlr_compositor *compositor, bool lazy);
 
 void wlr_xwayland_destroy(struct wlr_xwayland *wlr_xwayland);
 

rootston/desktop.c

@@ -835,7 +835,7 @@ struct roots_desktop *desktop_create(struct roots_server *server,
 
 	if (config->xwayland) {
 		desktop->xwayland = wlr_xwayland_create(server->wl_display,
-			desktop->compositor);
+			desktop->compositor, false);
 		wl_signal_add(&desktop->xwayland->events.new_surface,
 			&desktop->xwayland_surface);
 		desktop->xwayland_surface.notify = handle_xwayland_surface;

xwayland/xwayland.c

@@ -128,7 +128,7 @@ static void exec_xwayland(struct wlr_xwayland *wlr_xwayland) {
 	execvp("Xwayland", argv);
 }
 
-static void xwayland_finish(struct wlr_xwayland *wlr_xwayland) {
+static void xwayland_finish_server(struct wlr_xwayland *wlr_xwayland) {
 	if (!wlr_xwayland || wlr_xwayland->display == -1) {
 		return;
 	}
@@ -147,27 +147,33 @@ static void xwayland_finish(struct wlr_xwayland *wlr_xwayland) {
 		wl_event_source_remove(wlr_xwayland->sigusr1_source);
 	}
 
-	safe_close(wlr_xwayland->x_fd[0]);
-	safe_close(wlr_xwayland->x_fd[1]);
 	safe_close(wlr_xwayland->wl_fd[0]);
 	safe_close(wlr_xwayland->wl_fd[1]);
 	safe_close(wlr_xwayland->wm_fd[0]);
 	safe_close(wlr_xwayland->wm_fd[1]);
+	/* We do not kill the Xwayland process, it dies to broken pipe
+	 * after we close our side of the wm/wl fds. This is more reliable
+	 * than trying to kill something that might no longer be Xwayland.
+	 */
+}
+
+static void xwayland_finish_display(struct wlr_xwayland *wlr_xwayland) {
+	safe_close(wlr_xwayland->x_fd[0]);
+	safe_close(wlr_xwayland->x_fd[1]);
 
 	wl_list_remove(&wlr_xwayland->display_destroy.link);
 
 	unlink_display_sockets(wlr_xwayland->display);
 	wlr_xwayland->display = -1;
 	unsetenv("DISPLAY");
-	/* We do not kill the Xwayland process, it dies to broken pipe
-	 * after we close our side of the wm/wl fds. This is more reliable
-	 * than trying to kill something that might no longer be Xwayland.
-	 */
 }
 
-static bool xwayland_start(struct wlr_xwayland *wlr_xwayland,
+static bool xwayland_start_display(struct wlr_xwayland *wlr_xwayland,
 	struct wl_display *wl_display, struct wlr_compositor *compositor);
 
+static bool xwayland_start_server(struct wlr_xwayland *wlr_xwayland);
+static bool xwayland_start_server_lazy(struct wlr_xwayland *wlr_xwayland);
+
 static void handle_client_destroy(struct wl_listener *listener, void *data) {
 	struct wlr_xwayland *wlr_xwayland =
 		wl_container_of(listener, wlr_xwayland, client_destroy);
@@ -176,12 +182,13 @@ static void handle_client_destroy(struct wl_listener *listener, void *data) {
 	wlr_xwayland->client = NULL;
 	wl_list_remove(&wlr_xwayland->client_destroy.link);
 
-	xwayland_finish(wlr_xwayland);
+	xwayland_finish_server(wlr_xwayland);
 
-	if (time(NULL) - wlr_xwayland->server_start > 5) {
+	if (wlr_xwayland->lazy) {
+		xwayland_start_server_lazy(wlr_xwayland);
+	} else if (time(NULL) - wlr_xwayland->server_start > 5) {
 		wlr_log(L_INFO, "Restarting Xwayland");
-		xwayland_start(wlr_xwayland, wlr_xwayland->wl_display,
+		xwayland_start_server(wlr_xwayland);
-			wlr_xwayland->compositor);
 	}
 }
 
@@ -217,7 +224,7 @@ static int xserver_handle_ready(int signal_number, void *data) {
 
 	wlr_xwayland->xwm = xwm_create(wlr_xwayland);
 	if (!wlr_xwayland->xwm) {
-		xwayland_finish(wlr_xwayland);
+		xwayland_finish_server(wlr_xwayland);
 		return 1;
 	}
 
@@ -236,9 +243,6 @@ static int xserver_handle_ready(int signal_number, void *data) {
 		wlr_xwayland->cursor = NULL;
 	}
 
-	char display_name[16];
-	snprintf(display_name, sizeof(display_name), ":%d", wlr_xwayland->display);
-	setenv("DISPLAY", display_name, true);
 
 	wlr_signal_emit_safe(&wlr_xwayland->events.ready, wlr_xwayland);
 	/* ready is a one-shot signal, fire and forget */
@@ -247,48 +251,62 @@ static int xserver_handle_ready(int signal_number, void *data) {
 	return 1; /* wayland event loop dispatcher's count */
 }
 
-static bool xwayland_start(struct wlr_xwayland *wlr_xwayland,
+static int xwayland_socket_connected(int fd, uint32_t mask, void* data){
+	struct wlr_xwayland *wlr_xwayland = data;
+
+	wl_event_source_remove(wlr_xwayland->x_fd_read_event[0]);
+	wl_event_source_remove(wlr_xwayland->x_fd_read_event[1]);
+
+	xwayland_start_server(wlr_xwayland);
+
+	return 0;
+}
+
+static bool xwayland_start_display(struct wlr_xwayland *wlr_xwayland,
 		struct wl_display *wl_display, struct wlr_compositor *compositor) {
-	memset(wlr_xwayland, 0, offsetof(struct wlr_xwayland, seat));
-	wlr_xwayland->wl_display = wl_display;
-	wlr_xwayland->compositor = compositor;
 	wlr_xwayland->x_fd[0] = wlr_xwayland->x_fd[1] = -1;
-	wlr_xwayland->wl_fd[0] = wlr_xwayland->wl_fd[1] = -1;
-	wlr_xwayland->wm_fd[0] = wlr_xwayland->wm_fd[1] = -1;
 
 	wlr_xwayland->display_destroy.notify = handle_display_destroy;
 	wl_display_add_destroy_listener(wl_display, &wlr_xwayland->display_destroy);
 
 	wlr_xwayland->display = open_display_sockets(wlr_xwayland->x_fd);
 	if (wlr_xwayland->display < 0) {
-		xwayland_finish(wlr_xwayland);
+		xwayland_finish_display(wlr_xwayland);
 		return false;
 	}
+
+	char display_name[16];
+	snprintf(display_name, sizeof(display_name), ":%d", wlr_xwayland->display);
+	setenv("DISPLAY", display_name, true);
+
+	return true;
+}
+
+static bool xwayland_start_server(struct wlr_xwayland *wlr_xwayland) {
+	wlr_xwayland->wl_fd[0] = wlr_xwayland->wl_fd[1] = -1;
+	wlr_xwayland->wm_fd[0] = wlr_xwayland->wm_fd[1] = -1;
+
 	if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, wlr_xwayland->wl_fd) != 0 ||
 			socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, wlr_xwayland->wm_fd) != 0) {
 		wlr_log_errno(L_ERROR, "failed to create socketpair");
-		xwayland_finish(wlr_xwayland);
+		xwayland_finish_server(wlr_xwayland);
 		return false;
 	}
-
 	wlr_xwayland->server_start = time(NULL);
 
-	if (!(wlr_xwayland->client = wl_client_create(wl_display, wlr_xwayland->wl_fd[0]))) {
+	if (!(wlr_xwayland->client = wl_client_create(wlr_xwayland->wl_display, wlr_xwayland->wl_fd[0]))) {
 		wlr_log_errno(L_ERROR, "wl_client_create failed");
-		xwayland_finish(wlr_xwayland);
+		xwayland_finish_server(wlr_xwayland);
 		return false;
 	}
 
-	// unset $DISPLAY while XWayland starts
-	unsetenv("DISPLAY");
-
 	wlr_xwayland->wl_fd[0] = -1; /* not ours anymore */
 
 	wlr_xwayland->client_destroy.notify = handle_client_destroy;
 	wl_client_add_destroy_listener(wlr_xwayland->client,
 		&wlr_xwayland->client_destroy);
 
-	struct wl_event_loop *loop = wl_display_get_event_loop(wl_display);
+	struct wl_event_loop *loop = wl_display_get_event_loop(wlr_xwayland->wl_display);
 	wlr_xwayland->sigusr1_source = wl_event_loop_add_signal(loop, SIGUSR1,
 		xserver_handle_ready, wlr_xwayland);
 
@@ -323,36 +341,74 @@ static bool xwayland_start(struct wlr_xwayland *wlr_xwayland,
 	}
 	if (wlr_xwayland->pid < 0) {
 		wlr_log_errno(L_ERROR, "fork failed");
-		xwayland_finish(wlr_xwayland);
+		xwayland_finish_server(wlr_xwayland);
 		return false;
 	}
 
 	/* close child fds */
-	close(wlr_xwayland->x_fd[0]);
+	/* remain managing x sockets for lazy start */
-	close(wlr_xwayland->x_fd[1]);
 	close(wlr_xwayland->wl_fd[1]);
 	close(wlr_xwayland->wm_fd[1]);
-	wlr_xwayland->x_fd[0] = wlr_xwayland->x_fd[1] = -1;
 	wlr_xwayland->wl_fd[1] = wlr_xwayland->wm_fd[1] = -1;
 
 	return true;
 }
 
+static bool xwayland_start_server_lazy(struct wlr_xwayland *wlr_xwayland) {
+	struct wl_event_loop *loop = wl_display_get_event_loop(wlr_xwayland->wl_display);
+	wlr_xwayland->x_fd_read_event[0] =
+		wl_event_loop_add_fd(loop, wlr_xwayland->x_fd[0], WL_EVENT_READABLE,
+				xwayland_socket_connected, wlr_xwayland);
+	wlr_xwayland->x_fd_read_event[1] =
+		wl_event_loop_add_fd(loop, wlr_xwayland->x_fd[1], WL_EVENT_READABLE,
+				xwayland_socket_connected, wlr_xwayland);
+
+	return true;
+}
+
 void wlr_xwayland_destroy(struct wlr_xwayland *wlr_xwayland) {
 	wlr_xwayland_set_seat(wlr_xwayland, NULL);
-	xwayland_finish(wlr_xwayland);
+	xwayland_finish_server(wlr_xwayland);
+	xwayland_finish_display(wlr_xwayland);
 	free(wlr_xwayland);
 }
 
 struct wlr_xwayland *wlr_xwayland_create(struct wl_display *wl_display,
-		struct wlr_compositor *compositor) {
+		struct wlr_compositor *compositor, bool lazy) {
 	struct wlr_xwayland *wlr_xwayland = calloc(1, sizeof(struct wlr_xwayland));
+	memset(wlr_xwayland, 0, offsetof(struct wlr_xwayland, seat));
+
+	wlr_xwayland->wl_display = wl_display;
+	wlr_xwayland->compositor = compositor;
+	wlr_xwayland->lazy = lazy;
+
+	wlr_xwayland->x_fd[0] = wlr_xwayland->x_fd[1] = -1;
+	wlr_xwayland->wl_fd[0] = wlr_xwayland->wl_fd[1] = -1;
+	wlr_xwayland->wm_fd[0] = wlr_xwayland->wm_fd[1] = -1;
 
 	wl_signal_init(&wlr_xwayland->events.new_surface);
 	wl_signal_init(&wlr_xwayland->events.ready);
-	if (xwayland_start(wlr_xwayland, wl_display, compositor)) {
+
-		return wlr_xwayland;
+	if (!xwayland_start_display(wlr_xwayland, wl_display, compositor)) {
+		goto error_alloc;
 	}
+
+	if (wlr_xwayland->lazy) {
+		if (!xwayland_start_server_lazy(wlr_xwayland)) {
+			goto error_display;
+		}
+	} else {
+		if (!xwayland_start_server(wlr_xwayland)) {
+			goto error_display;
+		}
+	}
+
+	return wlr_xwayland;
+
+error_display:
+	xwayland_finish_display(wlr_xwayland);
+
+error_alloc:
 	free(wlr_xwayland);
 	return NULL;
 }