[pulseaudio-discuss] [PATCH 1/2] dbus: Fix connection idxset freeing when unloading the module.

[Tanu Kaskinen]

If u->connections isn't empty when module-dbus-protocol is
unloaded, then connection_free() is called for the
remaining connections when the idxset is freed.
connection_free() tries to remove the connection from the
idxset, but that fails, because the item has already been
removed from the idxset in this scenario.

The problem is solved by not trying to remove the connection
from the idxset in connection_free(). Instead, whoever wants
to delete connections, has to remove the connection from the
idxset in addition to calling connection_free().
---
 src/modules/dbus/module-dbus-protocol.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/modules/dbus/module-dbus-protocol.c b/src/modules/dbus/module-dbus-protocol.c
index aa11cdf..276c6ba 100644
--- a/src/modules/dbus/module-dbus-protocol.c
+++ b/src/modules/dbus/module-dbus-protocol.c
@@ -105,7 +105,6 @@ static void connection_free(struct connection *c) {
     pa_assert_se(pa_dbus_protocol_unregister_connection(c->server->userdata->dbus_protocol, pa_dbus_wrap_connection_get(c->wrap_conn)) >= 0);
 
     pa_client_free(c->client);
-    pa_assert_se(pa_idxset_remove_by_data(c->server->userdata->connections, c, NULL));
     pa_dbus_wrap_connection_free(c->wrap_conn);
     pa_xfree(c);
 }
@@ -514,8 +513,10 @@ static void cleanup_cb(pa_mainloop_api *a, pa_defer_event *e, void *userdata) {
     uint32_t idx;
 
     PA_IDXSET_FOREACH(conn, u->connections, idx) {
-        if (!dbus_connection_get_is_connected(pa_dbus_wrap_connection_get(conn->wrap_conn)))
+        if (!dbus_connection_get_is_connected(pa_dbus_wrap_connection_get(conn->wrap_conn))) {
+            pa_idxset_remove_by_data(u->connections, conn, NULL);
             connection_free(conn);
+        }
     }
 
     u->module->core->mainloop->defer_enable(e, 0);
@@ -579,17 +580,9 @@ fail:
     return -1;
 }
 
-/* Called by idxset when the connection set is freed. */
-static void connection_free_cb(void *p, void *userdata) {
-    struct connection *conn = p;
-
-    pa_assert(conn);
-
-    connection_free(conn);
-}
-
 void pa__done(pa_module *m) {
     struct userdata *u;
+    struct connection *c;
 
     pa_assert(m);
 
@@ -602,8 +595,10 @@ void pa__done(pa_module *m) {
     if (u->cleanup_event)
         m->core->mainloop->defer_free(u->cleanup_event);
 
-    if (u->connections)
-        pa_idxset_free(u->connections, connection_free_cb, NULL);
+    while ((c = pa_idxset_steal_first(u->connections, NULL)))
+        connection_free(c);
+
+    pa_idxset_free(u->connections, NULL, NULL);
 
     if (u->tcp_server)
         server_free(u->tcp_server);
-- 
1.7.4.2