proxy: improve proxy cleanup

When we destroy a proxy, mark it as zombie until the server removes
the id. This way we can still keep the id locked with a valid entry
and remove it later.

4 files changed, 27 insertions, 17 deletions

diff --git a/src/modules/module-protocol-native.c b/src/modules/module-protocol-native.c
index 323e0591..cae6bbba 100644
--- a/src/modules/module-protocol-native.c
+++ b/src/modules/module-protocol-native.c
@@ -532,11 +532,15 @@ on_remote_data(void *data, int fd, uint32_t mask)
 			        spa_debug_pod(0, NULL, (struct spa_pod *)msg->data);
 			}
 
-                        proxy = pw_remote_find_proxy(this, msg->id);
-
-                        if (proxy == NULL) {
-                                pw_log_error("protocol-native %p: could not find proxy %u", this, msg->id);
-                                continue;
+			proxy = pw_remote_find_proxy(this, msg->id);
+			if (proxy == NULL || proxy->zombie) {
+				if (proxy->zombie)
+					pw_log_debug(NAME" %p: zombie proxy %u", this, msg->id);
+				else
+					pw_log_error(NAME" %p: could not find proxy %u", this, msg->id);
+
+				/* FIXME close fds */
+				continue;
                         }
 
 			marshal = pw_proxy_get_marshal(proxy);
diff --git a/src/pipewire/private.h b/src/pipewire/private.h
index 73cb809c..2f993dc1 100644
--- a/src/pipewire/private.h
+++ b/src/pipewire/private.h
@@ -655,6 +655,8 @@ struct pw_proxy {
 	struct spa_list link;		/**< link in the remote */
 
 	uint32_t id;			/**< client side id */
+	unsigned int zombie:1;		/**< proxy is removed locally and waiting to
+					  *  be removed from server */
 	unsigned int removed:1;		/**< proxy was removed from server */
 
 	struct spa_hook_list listener_list;
diff --git a/src/pipewire/proxy.c b/src/pipewire/proxy.c
index 6ef62ba4..95788fef 100644
--- a/src/pipewire/proxy.c
+++ b/src/pipewire/proxy.c
@@ -161,22 +161,27 @@ void pw_proxy_destroy(struct pw_proxy *proxy)
 	struct proxy *impl = SPA_CONTAINER_OF(proxy, struct proxy, this);
 	struct pw_remote *remote = proxy->remote;
 
-	pw_log_debug(NAME" %p: destroy %u", proxy, proxy->id);
-	pw_proxy_emit_destroy(proxy);
-
-	spa_list_remove(&proxy->link);
+	if (!proxy->zombie) {
+		pw_log_debug(NAME" %p: destroy %u", proxy, proxy->id);
+		pw_proxy_emit_destroy(proxy);
 
+		spa_list_remove(&proxy->link);
+	}
 	if (!proxy->removed) {
 		/* if the server did not remove this proxy, remove ourselves
-		 * from the proxy objects and schedule a destroy, if
-		 * we don't have a remote, we must keep the proxy id
-		 * locked because the server might be using it still. */
-		pw_map_insert_at(&remote->objects, proxy->id, NULL);
-		if (remote->core_proxy)
+		 * from the proxy objects and schedule a destroy. */
+		if (remote->core_proxy) {
+			proxy->zombie = true;
 			pw_core_proxy_destroy(remote->core_proxy, proxy);
+		} else {
+			proxy->removed = true;
+		}
+	}
+	if (proxy->removed) {
+		pw_map_remove(&remote->objects, proxy->id);
+		pw_log_debug(NAME" %p: free %u", proxy, proxy->id);
+		free(impl);
 	}
-	pw_log_debug(NAME" %p: free", proxy);
-	free(impl);
 }
 
 SPA_EXPORT
diff --git a/src/pipewire/remote.c b/src/pipewire/remote.c
index b30b4914..5192d5da 100644
--- a/src/pipewire/remote.c
+++ b/src/pipewire/remote.c
@@ -141,7 +141,6 @@ static void core_event_remove_id(void *data, uint32_t id)
 	if ((proxy = pw_map_lookup(&this->objects, id)) != NULL) {
 		proxy->removed = true;
 		pw_proxy_destroy(proxy);
-		pw_map_remove(&this->objects, id);
 	}
 }