ipc: Add a stable ID for clients

Co-authored-by: Lubosz Sarnecki <lubosz.sarnecki@collabora.com>

src/xrt/ipc/server/ipc_server.h

@@ -342,6 +342,9 @@ struct ipc_server
 
 	volatile uint32_t current_slot_index;
 
+	//! Generator for IDs.
+	uint32_t id_generator;
+
 	struct
 	{
 		int active_client_index;
@@ -377,13 +380,29 @@ int
 ipc_server_main_android(struct ipc_server **ps, void (*startup_complete_callback)(void *data), void *data);
 #endif
 
+/*!
+ * Get the current state of a client.
+ *
+ * @ingroup ipc_server
+ */
+xrt_result_t
+ipc_server_get_client_app_state(struct ipc_server *s, uint32_t client_id, struct ipc_app_state *out_ias);
+
 /*!
  * Set the new active client.
  *
  * @ingroup ipc_server
  */
-void
-ipc_server_set_active_client(struct ipc_server *s, int client_id);
+xrt_result_t
+ipc_server_set_active_client(struct ipc_server *s, uint32_t client_id);
+
+/*!
+ * Toggle the io for this client.
+ *
+ * @ingroup ipc_server
+ */
+xrt_result_t
+ipc_server_toggle_io_client(struct ipc_server *s, uint32_t client_id);
 
 /*!
  * Called by client threads to set a session to active.

src/xrt/ipc/server/ipc_server_handler.c

@@ -937,49 +937,53 @@ ipc_handle_compositor_poll_events(volatile struct ipc_client_state *ics, union x
 	return xrt_comp_poll_events(ics->xc, out_xce);
 }
 
-xrt_result_t
-ipc_handle_system_get_client_info(volatile struct ipc_client_state *_ics,
-                                  uint32_t id,
-                                  struct ipc_app_state *out_client_desc)
-{
-	if (id >= IPC_MAX_CLIENTS) {
-		return XRT_ERROR_IPC_FAILURE;
-	}
-	volatile struct ipc_client_state *ics = &_ics->server->threads[id].ics;
-
-	if (!xrt_ipc_handle_is_valid(ics->imc.ipc_handle)) {
-		return XRT_ERROR_IPC_FAILURE;
-	}
-
-	*out_client_desc = ics->client_state;
-	out_client_desc->io_active = ics->io_active;
-
-	//@todo: track this data in the ipc_client_state struct
-	out_client_desc->primary_application = false;
-	if (ics->server->global_state.active_client_index == (int)id) {
-		out_client_desc->primary_application = true;
-	}
-
-	return XRT_SUCCESS;
-}
-
 xrt_result_t
 ipc_handle_system_get_clients(volatile struct ipc_client_state *_ics, struct ipc_client_list *list)
 {
+	struct ipc_server *s = _ics->server;
+
+	// Look client list.
+	os_mutex_lock(&s->global_state.lock);
+
+	uint32_t count = 0;
 	for (uint32_t i = 0; i < IPC_MAX_CLIENTS; i++) {
-		list->ids[i] = _ics->server->threads[i].ics.server_thread_index;
+
+		volatile struct ipc_client_state *ics = &s->threads[i].ics;
+
+		// Is this thread running?
+		if (ics->server_thread_index < 0) {
+			continue;
+		}
+
+		list->ids[count++] = ics->client_state.id;
 	}
+
+	list->id_count = count;
+
+	// Unlock now.
+	os_mutex_unlock(&s->global_state.lock);
+
 	return XRT_SUCCESS;
 }
 
 xrt_result_t
-ipc_handle_system_set_primary_client(volatile struct ipc_client_state *ics, uint32_t client_id)
+ipc_handle_system_get_client_info(volatile struct ipc_client_state *_ics,
+                                  uint32_t client_id,
+                                  struct ipc_app_state *out_ias)
 {
-	IPC_INFO(ics->server, "System setting active client to %d.", client_id);
+	struct ipc_server *s = _ics->server;
 
-	ipc_server_set_active_client(ics->server, client_id);
+	return ipc_server_get_client_app_state(s, client_id, out_ias);
+}
 
-	return XRT_SUCCESS;
+xrt_result_t
+ipc_handle_system_set_primary_client(volatile struct ipc_client_state *_ics, uint32_t client_id)
+{
+	struct ipc_server *s = _ics->server;
+
+	IPC_INFO(s, "System setting active client to %d.", client_id);
+
+	return ipc_server_set_active_client(s, client_id);
 }
 
 xrt_result_t
@@ -993,21 +997,11 @@ ipc_handle_system_set_focused_client(volatile struct ipc_client_state *ics, uint
 xrt_result_t
 ipc_handle_system_toggle_io_client(volatile struct ipc_client_state *_ics, uint32_t client_id)
 {
-	volatile struct ipc_client_state *ics = NULL;
+	struct ipc_server *s = _ics->server;
 
-	if (client_id >= IPC_MAX_CLIENTS) {
-		return XRT_ERROR_IPC_FAILURE;
-	}
+	IPC_INFO(s, "System toggling io for client %u.", client_id);
 
-	ics = &_ics->server->threads[client_id].ics;
-
-	if (!xrt_ipc_handle_is_valid(ics->imc.ipc_handle)) {
-		return XRT_ERROR_IPC_FAILURE;
-	}
-
-	ics->io_active = !ics->io_active;
-
-	return XRT_SUCCESS;
+	return ipc_server_toggle_io_client(s, client_id);
 }
 
 xrt_result_t

src/xrt/ipc/server/ipc_server_per_client_thread.c

@@ -70,7 +70,7 @@ client_loop(volatile struct ipc_client_state *ics)
 {
 	U_TRACE_SET_THREAD_NAME("IPC Client");
 
-	IPC_INFO(ics->server, "Client connected");
+	IPC_INFO(ics->server, "Client %u connected", ics->client_state.id);
 
 	// Claim the client fd.
 	int epoll_fd = setup_epoll(ics);

src/xrt/ipc/server/ipc_server_process.c

@@ -40,6 +40,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <assert.h>
+#include <limits.h>
 
 /*
  *
@@ -431,10 +432,21 @@ ipc_server_handle_client_connected(struct ipc_server *vs, xrt_ipc_handle_t ipc_h
 	}
 
 	it->state = IPC_THREAD_STARTING;
+
+	// Allocate a new ID, avoid zero.
+	//! @todo validate ID.
+	uint32_t id = ++vs->id_generator;
+
+	// Reset everything.
+	U_ZERO((struct ipc_client_state *)ics);
+
+	// Set state.
+	ics->client_state.id = id;
 	ics->imc.ipc_handle = ipc_handle;
 	ics->server = vs;
 	ics->server_thread_index = cs_index;
 	ics->io_active = true;
+
 	os_thread_start(&it->thread, ipc_server_client_thread, (void *)ics);
 
 	// Unlock when we are done.
@@ -695,26 +707,130 @@ update_server_state_locked(struct ipc_server *s)
 	s->global_state.last_active_client_index = s->global_state.active_client_index;
 }
 
-static void
-set_active_client_locked(struct ipc_server *s, int client_id)
+static volatile struct ipc_client_state *
+find_client_locked(struct ipc_server *s, uint32_t client_id)
 {
-	if (client_id != s->global_state.active_client_index) {
-		s->global_state.active_client_index = client_id;
+	// Check for invalid IDs.
+	if (client_id == 0 || client_id > INT_MAX) {
+		IPC_WARN(s, "Invalid ID '%u', failing operation.", client_id);
+		return NULL;
 	}
+
+	for (uint32_t i = 0; i < IPC_MAX_CLIENTS; i++) {
+		volatile struct ipc_client_state *ics = &s->threads[i].ics;
+
+		// Is this the client we are looking for?
+		if (ics->client_state.id != client_id) {
+			continue;
+		}
+
+		// Just in case of state data.
+		if (!xrt_ipc_handle_is_valid(ics->imc.ipc_handle)) {
+			IPC_WARN(s, "Encountered invalid state while searching for client with ID '%d'", client_id);
+			return NULL;
+		}
+
+		return ics;
+	}
+
+	IPC_WARN(s, "No client with ID '%u', failing operation.", client_id);
+
+	return NULL;
 }
 
+static xrt_result_t
+get_client_app_state_locked(struct ipc_server *s, uint32_t client_id, struct ipc_app_state *out_ias)
+{
+	volatile struct ipc_client_state *ics = find_client_locked(s, client_id);
+	if (ics == NULL) {
+		return XRT_ERROR_IPC_FAILURE;
+	}
+
+	struct ipc_app_state ias = ics->client_state;
+	ias.io_active = ics->io_active;
+
+	// @todo: track this data in the ipc_client_state struct
+	ias.primary_application = false;
+
+	// The active client is decided by index, so get that from the ics.
+	int index = ics->server_thread_index;
+
+	if (s->global_state.active_client_index == index) {
+		ias.primary_application = true;
+	}
+
+	*out_ias = ias;
+
+	return XRT_SUCCESS;
+}
+
+static xrt_result_t
+set_active_client_locked(struct ipc_server *s, uint32_t client_id)
+{
+	volatile struct ipc_client_state *ics = find_client_locked(s, client_id);
+	if (ics == NULL) {
+		return XRT_ERROR_IPC_FAILURE;
+	}
+
+	// The active client is decided by index, so get that from the ics.
+	int index = ics->server_thread_index;
+
+	if (index != s->global_state.active_client_index) {
+		s->global_state.active_client_index = index;
+	}
+
+	return XRT_SUCCESS;
+}
+
+static xrt_result_t
+toggle_io_client_locked(struct ipc_server *s, uint32_t client_id)
+{
+	volatile struct ipc_client_state *ics = find_client_locked(s, client_id);
+	if (ics == NULL) {
+		return XRT_ERROR_IPC_FAILURE;
+	}
+
+	ics->io_active = !ics->io_active;
+
+	return XRT_SUCCESS;
+}
+
+
 /*
  *
  * Exported functions.
  *
  */
 
-void
-ipc_server_set_active_client(struct ipc_server *s, int client_id)
+
+xrt_result_t
+ipc_server_get_client_app_state(struct ipc_server *s, uint32_t client_id, struct ipc_app_state *out_ias)
 {
 	os_mutex_lock(&s->global_state.lock);
-	set_active_client_locked(s, client_id);
+	xrt_result_t xret = get_client_app_state_locked(s, client_id, out_ias);
 	os_mutex_unlock(&s->global_state.lock);
+
+	return xret;
+}
+
+xrt_result_t
+ipc_server_set_active_client(struct ipc_server *s, uint32_t client_id)
+{
+	os_mutex_lock(&s->global_state.lock);
+	xrt_result_t xret = set_active_client_locked(s, client_id);
+	os_mutex_unlock(&s->global_state.lock);
+
+	return xret;
+}
+
+xrt_result_t
+ipc_server_toggle_io_client(struct ipc_server *s, uint32_t client_id)
+{
+	os_mutex_lock(&s->global_state.lock);
+	xrt_result_t xret = toggle_io_client_locked(s, client_id);
+	os_mutex_unlock(&s->global_state.lock);
+
+	return xret;
 }
 
 void
@@ -742,7 +858,8 @@ ipc_server_activate_session(volatile struct ipc_client_state *ics)
 		                             s->global_state.last_active_client_index);
 	} else {
 		// Update active client
-		set_active_client_locked(s, ics->server_thread_index);
+		set_active_client_locked(s, ics->client_state.id);
+
 		// For new active regular sessions update all clients.
 		update_server_state_locked(s);
 	}

src/xrt/ipc/shared/ipc_protocol.h

@@ -285,7 +285,8 @@ struct ipc_client_description
 
 struct ipc_client_list
 {
-	int32_t ids[IPC_MAX_CLIENTS];
+	uint32_t ids[IPC_MAX_CLIENTS];
+	uint32_t id_count;
 };
 
 /*!
@@ -295,6 +296,9 @@ struct ipc_client_list
  */
 struct ipc_app_state
 {
+	// Stable and unique ID of the client, only unique within this instance.
+	uint32_t id;
+
 	bool primary_application;
 	bool session_active;
 	bool session_visible;

src/xrt/ipc/shared/proto.json

@@ -16,7 +16,7 @@
 			{"name": "id", "type": "uint32_t"}
 		],
 		"out": [
-			{"name": "desc", "type": "struct ipc_app_state"}
+			{"name": "ias", "type": "struct ipc_app_state"}
 		]
 	},
 

src/xrt/targets/ctl/main.c

@@ -43,15 +43,13 @@ get_mode(struct ipc_connection *ipc_c)
 	}
 
 	P("Clients:\n");
-	for (uint32_t i = 0; i < IPC_MAX_CLIENTS; i++) {
-		if (clients.ids[i] < 0) {
-			continue;
-		}
+	for (uint32_t i = 0; i < clients.id_count; i++) {
+		uint32_t id = clients.ids[i];
 
 		struct ipc_app_state cs;
-		r = ipc_call_system_get_client_info(ipc_c, i, &cs);
+		r = ipc_call_system_get_client_info(ipc_c, id, &cs);
 		if (r != XRT_SUCCESS) {
-			PE("Failed to get client info for client %d.\n", i);
+			PE("Failed to get client info for client %d.\n", id);
 			return 1;
 		}
 
@@ -145,21 +143,15 @@ main(int argc, char *argv[])
 		switch (c) {
 		case 'p':
 			s_val = atoi(optarg);
-			if (s_val >= 0 && s_val < IPC_MAX_CLIENTS) {
-				op_mode = MODE_SET_PRIMARY;
-			}
+			op_mode = MODE_SET_PRIMARY;
 			break;
 		case 'f':
 			s_val = atoi(optarg);
-			if (s_val >= 0 && s_val < IPC_MAX_CLIENTS) {
-				op_mode = MODE_SET_FOCUSED;
-			}
+			op_mode = MODE_SET_FOCUSED;
 			break;
 		case 'i':
 			s_val = atoi(optarg);
-			if (s_val >= 0 && s_val < IPC_MAX_CLIENTS) {
-				op_mode = MODE_TOGGLE_IO;
-			}
+			op_mode = MODE_TOGGLE_IO;
 			break;
 		case '?':
 			if (optopt == 's') {