Merge pull request #959 from VincentVanlaer/xwayland-lazy

Add the option to start Xwayland only when a client connects

include/rootston/config.h

@@ -60,6 +60,7 @@ struct roots_cursor_config {
 
 struct roots_config {
 	bool xwayland;
+	bool xwayland_lazy;
 
 	struct wl_list outputs;
 	struct wl_list devices;

include/rootston/desktop.h

@@ -65,7 +65,6 @@ struct roots_desktop {
 #ifdef WLR_HAS_XWAYLAND
 	struct wlr_xwayland *xwayland;
 	struct wl_listener xwayland_surface;
-	struct wl_listener xwayland_ready;
 #endif
 };
 

include/wlr/xwayland.h

@@ -13,20 +13,26 @@ struct wlr_xwayland_cursor;
 
 struct wlr_xwayland {
 	pid_t pid;
-	int display;
-	int x_fd[2], wl_fd[2], wm_fd[2];
 	struct wl_client *client;
-	struct wl_display *wl_display;
-	struct wlr_compositor *compositor;
-	time_t server_start;
-
 	struct wl_event_source *sigusr1_source;
 	struct wl_listener client_destroy;
 	struct wl_listener display_destroy;
 	struct wlr_xwm *xwm;
 	struct wlr_xwayland_cursor *cursor;
+	int wm_fd[2], wl_fd[2];
+
+	time_t server_start;
 
-	/* Anything above seat is reset on Xwayland restart, rest is conserved */
+	/* Anything above display is reset on Xwayland restart, rest is conserved */
+
+	int display;
+	int x_fd[2];
+	struct wl_event_source *x_fd_read_event[2];
+
+	bool lazy;
+
+	struct wl_display *wl_display;
+	struct wlr_compositor *compositor;
 	struct wlr_seat *seat;
 	struct wl_listener seat_destroy;
 
@@ -169,7 +175,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/config.c

@@ -240,6 +240,9 @@ static int config_ini_handler(void *user, const char *section, const char *name,
 		if (strcmp(name, "xwayland") == 0) {
 			if (strcasecmp(value, "true") == 0) {
 				config->xwayland = true;
+			} else if (strcasecmp(value, "immediate") == 0) {
+				config->xwayland = true;
+				config->xwayland_lazy = false;
 			} else if (strcasecmp(value, "false") == 0) {
 				config->xwayland = false;
 			} else {
@@ -389,6 +392,7 @@ struct roots_config *roots_config_create_from_args(int argc, char *argv[]) {
 	}
 
 	config->xwayland = true;
+	config->xwayland_lazy = true;
 	wl_list_init(&config->outputs);
 	wl_list_init(&config->devices);
 	wl_list_init(&config->keyboards);

rootston/desktop.c

@@ -822,7 +822,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, config->xwayland_lazy);
 		wl_signal_add(&desktop->xwayland->events.new_surface,
 			&desktop->xwayland_surface);
 		desktop->xwayland_surface.notify = handle_xwayland_surface;

rootston/main.c

@@ -14,18 +14,6 @@
 
 struct roots_server server = { 0 };
 
-static void ready(struct wl_listener *listener, void *data) {
-	if (server.config->startup_cmd != NULL) {
-		const char *cmd = server.config->startup_cmd;
-		pid_t pid = fork();
-		if (pid < 0) {
-			wlr_log(L_ERROR, "cannot execute binding command: fork() failed");
-		} else if (pid == 0) {
-			execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL);
-		}
-	}
-}
-
 int main(int argc, char **argv) {
 	wlr_log_init(L_DEBUG, NULL);
 	server.config = roots_config_create_from_args(argc, argv);
@@ -65,21 +53,24 @@ int main(int argc, char **argv) {
 	}
 
 	setenv("WAYLAND_DISPLAY", socket, true);
-#ifndef WLR_HAS_XWAYLAND
+#ifdef WLR_HAS_XWAYLAND
-	ready(NULL, NULL);
-#else
 	if (server.desktop->xwayland != NULL) {
 		struct roots_seat *xwayland_seat =
 			input_get_seat(server.input, ROOTS_CONFIG_DEFAULT_SEAT_NAME);
 		wlr_xwayland_set_seat(server.desktop->xwayland, xwayland_seat->seat);
-		wl_signal_add(&server.desktop->xwayland->events.ready,
-			&server.desktop->xwayland_ready);
-		server.desktop->xwayland_ready.notify = ready;
-	} else {
-		ready(NULL, NULL);
 	}
 #endif
 
+	if (server.config->startup_cmd != NULL) {
+		const char *cmd = server.config->startup_cmd;
+		pid_t pid = fork();
+		if (pid < 0) {
+			wlr_log(L_ERROR, "cannot execute binding command: fork() failed");
+		} else if (pid == 0) {
+			execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL);
+		}
+	}
+
 	wl_display_run(server.wl_display);
 	wl_display_destroy(server.wl_display);
 	return 0;

rootston/rootston.ini.example

@@ -1,5 +1,8 @@
 [core]
-# Disable X11 support. Enabled by default.
+# X11 support
+#  - true: enables X11, xwayland is started only when an X11 client connects
+#  - immediate: enables X11, xwayland is started immediately
+#  - false: disables xwayland
 xwayland=false
 
 # Single output configuration. String after colon must match output's name.

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,41 +147,61 @@ 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]);
+	memset(wlr_xwayland, 0, offsetof(struct wlr_xwayland, display));
+	wlr_xwayland->wl_fd[0] = wlr_xwayland->wl_fd[1] = -1;
+	wlr_xwayland->wm_fd[0] = wlr_xwayland->wm_fd[1] = -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]);
+	wlr_xwayland->x_fd[0] = wlr_xwayland->x_fd[1] = -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);
 
+	if (wlr_xwayland->sigusr1_source) {
+		// Xwayland failed to start, let the sigusr1 handler deal with it
+		return;
+	}
+
 	// Don't call client destroy: it's being destroyed already
 	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) {
-		wlr_log(L_INFO, "Restarting Xwayland");
+		if (wlr_xwayland->lazy) {
-		xwayland_start(wlr_xwayland, wlr_xwayland->wl_display,
+			wlr_log(L_INFO, "Restarting Xwayland (lazy)");
-			wlr_xwayland->compositor);
+			xwayland_start_server_lazy(wlr_xwayland);
+		} else  {
+			wlr_log(L_INFO, "Restarting Xwayland");
+			xwayland_start_server(wlr_xwayland);
+		}
 	}
 }
 
@@ -217,7 +237,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 +256,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 +264,61 @@ 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]);
+
+	wlr_xwayland->x_fd_read_event[0] = wlr_xwayland->x_fd_read_event[1] = NULL;
+
+	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) {
+
 	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 +353,76 @@ 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));
+	if (!wlr_xwayland) {
+		return NULL;
+	}
+
+	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;
 }