seat: Deactivate devices before sending disable

Previously, seatd would not deactivate devices until the client had
acked the disable. In once instance, this lead to libinput spending
significant time checking and closing each input device.

As a workaround, mimick logind's behavior of deactivating devices first.
The original behavior can be reintroduced if the client-side problem is
fixed.

Closes: https://todo.sr.ht/~kennylevinsen/seatd/5

include/client.h

@@ -21,6 +21,7 @@ struct client {
 
 	struct seat *seat;
 	int seat_vt;
+	bool pending_disable;
 
 	struct list devices;
 };

include/seat.h

@@ -39,15 +39,16 @@ struct seat *seat_create(const char *name, bool vt_bound);
 void seat_destroy(struct seat *seat);
 
 int seat_add_client(struct seat *seat, struct client *client);
-int seat_remove_client(struct seat *seat, struct client *client);
+int seat_remove_client(struct client *client);
 int seat_open_client(struct seat *seat, struct client *client);
-int seat_close_client(struct seat *seat, struct client *client);
+int seat_close_client(struct client *client);
+int seat_ack_disable_client(struct client *client);
 
 struct seat_device *seat_open_device(struct client *client, const char *path);
 int seat_close_device(struct client *client, struct seat_device *seat_device);
 struct seat_device *seat_find_device(struct client *client, int device_id);
 
-int seat_set_next_session(struct seat *seat, int session);
+int seat_set_next_session(struct client *client, int session);
 int seat_activate(struct seat *seat);
 int seat_prepare_vt_switch(struct seat *seat);
 

seatd/client.c

@@ -79,7 +79,7 @@ void client_kill(struct client *client) {
 		client->connection.fd = -1;
 	};
 	if (client->seat != NULL) {
-		seat_remove_client(client->seat, client);
+		seat_remove_client(client);
 		client->seat = NULL;
 	}
 }
@@ -89,7 +89,7 @@ void client_destroy(struct client *client) {
 	client->server = NULL;
 	if (client->seat != NULL) {
 		// This should also close and remove all devices
-		seat_remove_client(client->seat, client);
+		seat_remove_client(client);
 		client->seat = NULL;
 	}
 	if (client->event_source != NULL) {
@@ -185,7 +185,7 @@ static int handle_close_seat(struct client *client) {
 		return -1;
 	}
 
-	if (seat_remove_client(client->seat, client) == -1) {
+	if (seat_remove_client(client) == -1) {
 		log_error("unable to remove client from seat");
 		return -1;
 	}
@@ -291,24 +291,7 @@ static int handle_switch_session(struct client *client, int session) {
 		return -1;
 	}
 
-	struct seat *seat = client->seat;
+	if (seat_set_next_session(client, session) == -1) {
-	if (seat->active_client != client) {
-		log_info("refusing to switch session: client requesting switch is not active");
-		errno = EPERM;
-		goto error;
-	}
-	if (session <= 0) {
-		log_errorf("invalid session: %d", session);
-		errno = EINVAL;
-		goto error;
-	}
-
-	if (client_get_session(client) == session) {
-		return 0;
-	}
-
-	if (seat_set_next_session(seat, session) == -1) {
-		log_infof("could not queue session switch: %s", strerror(errno));
 		goto error;
 	}
 
@@ -331,7 +314,7 @@ static int handle_disable_seat(struct client *client) {
 		goto error;
 	}
 
-	if (seat_close_client(seat, client) == -1) {
+	if (seat_ack_disable_client(client) == -1) {
 		goto error;
 	}
 

seatd/seat.c

@@ -70,10 +70,11 @@ int seat_add_client(struct seat *seat, struct client *client) {
 	return 0;
 }
 
-int seat_remove_client(struct seat *seat, struct client *client) {
+int seat_remove_client(struct client *client) {
-	assert(seat);
 	assert(client);
-	assert(client->seat == seat);
+	assert(client->seat);
+
+	struct seat *seat = client->seat;
 
 	// We must first remove the client to avoid reactivation
 	bool found = false;
@@ -95,12 +96,12 @@ int seat_remove_client(struct seat *seat, struct client *client) {
 	}
 
 	while (client->devices.length > 0) {
-		struct seat_device *device = client->devices.items[client->devices.length - 1];
+		struct seat_device *device = list_pop_back(&client->devices);
 		seat_close_device(client, device);
 	}
 
 	if (seat->active_client == client) {
-		seat_close_client(seat, client);
+		seat_close_client(client);
 	}
 
 	client->seat = NULL;
@@ -135,6 +136,11 @@ struct seat_device *seat_open_device(struct client *client, const char *path) {
 		return NULL;
 	}
 
+	if (client->pending_disable) {
+		errno = EPERM;
+		return NULL;
+	}
+
 	char sanitized_path[PATH_MAX];
 	if (realpath(path, sanitized_path) == NULL) {
 		log_errorf("invalid path '%s': %s", path, strerror(errno));
@@ -375,7 +381,7 @@ int seat_open_client(struct seat *seat, struct client *client) {
 
 	seat->active_client = client;
 	if (client_enable_seat(client) == -1) {
-		seat_remove_client(seat, client);
+		seat_remove_client(client);
 		return -1;
 	}
 
@@ -383,9 +389,37 @@ int seat_open_client(struct seat *seat, struct client *client) {
 	return 0;
 }
 
-int seat_close_client(struct seat *seat, struct client *client) {
+int seat_close_client(struct client *client) {
-	assert(seat);
 	assert(client);
+	assert(client->seat);
+
+	struct seat *seat = client->seat;
+
+	if (seat->active_client != client) {
+		log_error("client not active");
+		errno = EBUSY;
+		return -1;
+	}
+
+	while (client->devices.length > 0) {
+		struct seat_device *device = list_pop_back(&client->devices);
+		if (seat_close_device(client, device) == -1) {
+			log_errorf("unable to close '%s': %s", device->path, strerror(errno));
+		}
+	}
+
+	client->pending_disable = false;
+	seat->active_client = NULL;
+	seat_activate(seat);
+	log_debug("closed client");
+	return 0;
+}
+
+static int seat_disable_client(struct client *client) {
+	assert(client);
+	assert(client->seat);
+
+	struct seat *seat = client->seat;
 
 	if (seat->active_client != client) {
 		log_error("client not active");
@@ -406,37 +440,62 @@ int seat_close_client(struct seat *seat, struct client *client) {
 
 	log_debugf("deactivated %zd devices", client->devices.length);
 
-	seat->active_client = NULL;
+	client->pending_disable = true;
-
+	if (client_disable_seat(seat->active_client) == -1) {
-	if (seat->vt_bound && seat->vt_pending_ack) {
+		seat_remove_client(client);
-		log_debug("acking pending VT switch");
+		return -1;
-		seat->vt_pending_ack = false;
-		assert(seat->curttyfd != -1);
-		terminal_set_process_switching(seat->curttyfd, true);
-		terminal_set_keyboard(seat->curttyfd, true);
-		terminal_set_graphics(seat->curttyfd, false);
-		terminal_ack_switch(seat->curttyfd);
-		close(seat->curttyfd);
-		seat->curttyfd = -1;
-		return 0;
 	}
 
-	seat_activate(seat);
+	log_debug("disabling client");
-	log_debug("closed client");
 	return 0;
 }
 
-int seat_set_next_session(struct seat *seat, int session) {
+int seat_ack_disable_client(struct client *client) {
-	assert(seat);
+	assert(client);
+	assert(client->seat);
+
+	struct seat *seat = client->seat;
+
+	if (seat->active_client != client || !client->pending_disable) {
+		log_error("client not active or not pending disable");
+		errno = EBUSY;
+		return -1;
+	}
+
+	client->pending_disable = false;
+	seat->active_client = NULL;
+	seat_activate(seat);
+	log_debug("disabled client");
+	return 0;
+}
+
+int seat_set_next_session(struct client *client, int session) {
+	assert(client);
+	assert(client->seat);
+
+	struct seat *seat = client->seat;
+
+	if (seat->active_client != client || client->pending_disable) {
+		log_error("client not active or pending disable");
+		errno = EPERM;
+		return -1;
+	}
+
+	if (session == client_get_session(client)) {
+		log_info("requested session is already active");
+		return 0;
+	}
 
 	// Check if the session number is valid
 	if (session <= 0) {
+		log_errorf("invalid session value: %d", session);
 		errno = EINVAL;
 		return -1;
 	}
 
 	// Check if a switch is already queued
 	if (seat->next_vt > 0 || seat->next_client != NULL) {
+		log_info("switch is already queued");
 		return 0;
 	}
 
@@ -463,10 +522,7 @@ int seat_set_next_session(struct seat *seat, int session) {
 		return -1;
 	}
 
-	if (client_disable_seat(seat->active_client) == -1) {
+	seat_disable_client(seat->active_client);
-		seat_remove_client(seat, seat->active_client);
-	}
-
 	return 0;
 }
 
@@ -486,6 +542,20 @@ int seat_activate(struct seat *seat) {
 			return -1;
 		}
 
+		// If we need to ack a switch, do that
+		if (seat->vt_pending_ack) {
+			log_info("acking pending VT switch");
+			seat->vt_pending_ack = false;
+			if (seat->curttyfd != -1) {
+				terminal_set_process_switching(seat->curttyfd, false);
+				terminal_set_keyboard(seat->curttyfd, true);
+				terminal_set_graphics(seat->curttyfd, false);
+				close(seat->curttyfd);
+				seat->curttyfd = -1;
+			}
+			return 0;
+		}
+
 		// If we're asked to do a simple VT switch, do that
 		if (seat->next_vt > 0) {
 			log_info("executing VT switch");
@@ -568,16 +638,13 @@ int seat_prepare_vt_switch(struct seat *seat) {
 
 	if (seat->vt_pending_ack) {
 		log_info("impatient user, killing session to force pending switch");
-		seat_close_client(seat, seat->active_client);
+		seat_close_client(seat->active_client);
 		return 0;
 	}
 
 	log_debug("delaying VT switch acknowledgement");
 
 	seat->vt_pending_ack = true;
-	if (client_disable_seat(seat->active_client) == -1) {
+	seat_disable_client(seat->active_client);
-		seat_remove_client(seat, seat->active_client);
-	}
-
 	return 0;
 }