[pulseaudio-discuss] [PATCH v1 2/2] reserve: Fix leaking NameLost signals after release+acquire

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz at bmw-carit.de>

The use of the pseudo-blocking D-Bus calls leads to the problem that
NameLost signals are received after the reply to ReleaseName().

The problem with this is that a later acquisition of the same audio
device can potentially receive the NameLost signal corresponding to
the previous instance, due to the fact that the signal hasn't been
popped from the D-Bus message queue.

The simplest approach to solve this problem is to use private
connections. In this case the DBusConnection will not be reused across
different device reservations, avoiding any possible ambiguous D-Bus
messages.
---
 src/modules/reserve-wrap.c | 64 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/src/modules/reserve-wrap.c b/src/modules/reserve-wrap.c
index 1411d27..8ab2d18 100644
--- a/src/modules/reserve-wrap.c
+++ b/src/modules/reserve-wrap.c
@@ -34,6 +34,7 @@
 
 #ifdef HAVE_DBUS
 #include <pulsecore/dbus-shared.h>
+#include <pulsecore/dbus-util.h>
 #include "reserve.h"
 #include "reserve-monitor.h"
 #endif
@@ -44,9 +45,10 @@ struct pa_reserve_wrapper {
     PA_REFCNT_DECLARE;
     pa_core *core;
     pa_hook hook;
+    char *device_name;
     char *shared_name;
 #ifdef HAVE_DBUS
-    pa_dbus_connection *connection;
+    pa_dbus_wrap_connection *connection;
     struct rd_device *device;
 #endif
 };
@@ -62,6 +64,8 @@ struct pa_reserve_monitor_wrapper {
 #endif
 };
 
+static pa_reserve_monitor_wrapper* find_reserve_monitor_wrapper(pa_core *c, const char *device_name);
+
 static void reserve_wrapper_free(pa_reserve_wrapper *r) {
     pa_assert(r);
 
@@ -69,8 +73,14 @@ static void reserve_wrapper_free(pa_reserve_wrapper *r) {
     if (r->device)
         rd_release(r->device);
 
-    if (r->connection)
-        pa_dbus_connection_unref(r->connection);
+    if (r->connection) {
+        struct pa_reserve_monitor_wrapper *w;
+
+        if ((w = find_reserve_monitor_wrapper(r->core, r->device_name)))
+            rm_set_rd_private_bus(w->monitor, NULL);
+
+        pa_dbus_wrap_connection_free(r->connection);
+    }
 #endif
 
     pa_hook_done(&r->hook);
@@ -80,6 +90,7 @@ static void reserve_wrapper_free(pa_reserve_wrapper *r) {
         pa_xfree(r->shared_name);
     }
 
+    pa_xfree(r->device_name);
     pa_xfree(r);
 }
 
@@ -103,10 +114,27 @@ static int request_cb(rd_device *d, int forced) {
 }
 #endif
 
+static pa_reserve_wrapper* find_reserve_wrapper(pa_core *c, const char *device_name) {
+    pa_reserve_wrapper *r;
+    char *t;
+
+    pa_assert(c);
+    pa_assert(device_name);
+
+    t = pa_sprintf_malloc("reserve-wrapper@%s", device_name);
+    r = pa_shared_get(c, t);
+
+    pa_xfree(t);
+
+    return r;
+}
+
 pa_reserve_wrapper* pa_reserve_wrapper_get(pa_core *c, const char *device_name) {
     pa_reserve_wrapper *r;
     char *t;
 #ifdef HAVE_DBUS
+    pa_reserve_monitor_wrapper *w;
+    DBusConnection *conn;
     int k;
     DBusError error;
 
@@ -134,9 +162,10 @@ pa_reserve_wrapper* pa_reserve_wrapper_get(pa_core *c, const char *device_name)
     r->shared_name = t;
 
     pa_assert_se(pa_shared_set(c, r->shared_name, r) >= 0);
+    pa_assert_se((r->device_name = strdup(device_name)));
 
 #ifdef HAVE_DBUS
-    if (!(r->connection = pa_dbus_bus_get(c, DBUS_BUS_SESSION, &error)) || dbus_error_is_set(&error)) {
+    if (!(conn = dbus_bus_get_private(DBUS_BUS_SESSION, &error))) {
         pa_log_debug("Unable to contact D-Bus session bus: %s: %s", error.name, error.message);
 
         /* We don't treat this as error here because we want allow PA
@@ -144,9 +173,14 @@ pa_reserve_wrapper* pa_reserve_wrapper_get(pa_core *c, const char *device_name)
         return r;
     }
 
+    pa_assert_se(r->connection = pa_dbus_wrap_connection_new_from_existing(c->mainloop, FALSE, conn));
+
+    if ((w = find_reserve_monitor_wrapper(c, device_name)))
+        rm_set_rd_private_bus(w->monitor, conn);
+
     if ((k = rd_acquire(
                  &r->device,
-                 pa_dbus_connection_get(r->connection),
+                 conn,
                  device_name,
                  _("PulseAudio Sound Server"),
                  0,
@@ -246,10 +280,26 @@ static void change_cb(rm_monitor *m) {
 }
 #endif
 
+static pa_reserve_monitor_wrapper* find_reserve_monitor_wrapper(pa_core *c, const char *device_name) {
+    pa_reserve_monitor_wrapper *w;
+    char *t;
+
+    pa_assert(c);
+    pa_assert(device_name);
+
+    t = pa_sprintf_malloc("reserve-monitor-wrapper@%s", device_name);
+    w = pa_shared_get(c, t);
+
+    pa_xfree(t);
+
+    return w;
+}
+
 pa_reserve_monitor_wrapper* pa_reserve_monitor_wrapper_get(pa_core *c, const char *device_name) {
     pa_reserve_monitor_wrapper *w;
     char *t;
 #ifdef HAVE_DBUS
+    pa_reserve_wrapper *r;
     int k;
     DBusError error;
 
@@ -301,6 +351,10 @@ pa_reserve_monitor_wrapper* pa_reserve_monitor_wrapper_get(pa_core *c, const cha
     pa_log_debug("Successfully create reservation lock monitor for device '%s'", device_name);
 
     rm_set_userdata(w->monitor, w);
+
+    if ((r = find_reserve_wrapper(c, device_name)))
+        rm_set_rd_private_bus(w->monitor, pa_dbus_wrap_connection_get(r->connection));
+
     return w;
 
 fail:
-- 
1.7.11.7