[pulseaudio-commits] Branch 'stable-3.x' - 2 commits - src/modules
Tanu Kaskinen
tanuk at kemper.freedesktop.org
Mon Feb 18 09:40:31 PST 2013
src/modules/reserve-monitor.c | 59 +--------------------------------
src/modules/reserve.c | 73 ++++++++++++++++++++++++++++++++++++++++++
src/modules/reserve.h | 9 +++++
3 files changed, 84 insertions(+), 57 deletions(-)
New commits:
commit 1db09d30138c54de64f8e197eb6d60b8a04d31c8
Author: Mikel Astiz <mikel.astiz at bmw-carit.de>
Date: Wed Jan 30 09:30:31 2013 +0100
reserve: Fix leaking NameLost signals after release+acquire
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 poll the actual name
owner from the D-Bus daemon, in order to make sure that we did really
lose the name.
The proposal uses a blocking call to GetNameOwner to avoid incosistent
states in the internal APIs: it would otherwise be possible to have a
"busy" device before the reservation has been lost, in the unlikely
case if some other process acquires the name before we got the
confirmation that the NameLost was actually true.
diff --git a/src/modules/reserve.c b/src/modules/reserve.c
index bbb6773..f78805e 100644
--- a/src/modules/reserve.c
+++ b/src/modules/reserve.c
@@ -293,6 +293,7 @@ static DBusHandlerResult filter_handler(
rd_device *d;
DBusError error;
+ char *name_owner = NULL;
dbus_error_init(&error);
@@ -310,6 +311,21 @@ static DBusHandlerResult filter_handler(
goto invalid;
if (strcmp(name, d->service_name) == 0 && d->owning) {
+ /* Verify the actual owner of the name to avoid leaked NameLost
+ * signals from previous reservations. The D-Bus daemon will send
+ * all messages asynchronously in the correct order, but we could
+ * potentially process them too late due to the pseudo-blocking
+ * call mechanism used during both acquisition and release. This
+ * can happen if we release the device and immediately after
+ * reacquire it before NameLost is processed. */
+ if (!d->gave_up) {
+ const char *un;
+
+ if ((un = dbus_bus_get_unique_name(c)) && rd_dbus_get_name_owner(c, d->service_name, &name_owner, &error) == 0)
+ if (strcmp(name_owner, un) == 0)
+ goto invalid; /* Name still owned by us */
+ }
+
d->owning = 0;
if (!d->gave_up) {
@@ -326,6 +342,7 @@ static DBusHandlerResult filter_handler(
}
invalid:
+ free(name_owner);
dbus_error_free(&error);
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
commit 88a7b31ca7d1ec38b68e7bfd99e2e53afe96cfcf
Author: Mikel Astiz <mikel.astiz at bmw-carit.de>
Date: Wed Jan 30 09:30:30 2013 +0100
reserve: Move get_name_owner() to the public rd_device API
The function is interesting for both rd_device and rd_monitor so make
it part of the rd_device public API to avoid duplicated code.
The decision to move the function to reserve.c is motivated by the fact
that other projects (i.e. jack) use reserve.c only. Therefore, adding a
reserve->reserve-monitor dependency should be avoided.
diff --git a/src/modules/reserve-monitor.c b/src/modules/reserve-monitor.c
index 4aa4a2b..70de870 100644
--- a/src/modules/reserve-monitor.c
+++ b/src/modules/reserve-monitor.c
@@ -32,6 +32,7 @@
#include <assert.h>
#include "reserve-monitor.h"
+#include "reserve.h"
struct rm_monitor {
int ref;
@@ -120,62 +121,6 @@ invalid:
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
-static int get_name_owner(
- DBusConnection *connection,
- const char *name,
- char **name_owner,
- DBusError *error) {
-
- DBusMessage *msg, *reply;
- int r;
-
- *name_owner = NULL;
-
- if (!(msg = dbus_message_new_method_call(DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "GetNameOwner"))) {
- r = -ENOMEM;
- goto fail;
- }
-
- if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, &name, DBUS_TYPE_INVALID)) {
- r = -ENOMEM;
- goto fail;
- }
-
- reply = dbus_connection_send_with_reply_and_block(connection, msg, DBUS_TIMEOUT_USE_DEFAULT, error);
- dbus_message_unref(msg);
- msg = NULL;
-
- if (reply) {
- if (!dbus_message_get_args(reply, error, DBUS_TYPE_STRING, name_owner, DBUS_TYPE_INVALID)) {
- dbus_message_unref(reply);
- r = -EIO;
- goto fail;
- }
-
- *name_owner = strdup(*name_owner);
- dbus_message_unref(reply);
-
- if (!*name_owner) {
- r = -ENOMEM;
- goto fail;
- }
-
- } else if (dbus_error_has_name(error, "org.freedesktop.DBus.Error.NameHasNoOwner"))
- dbus_error_free(error);
- else {
- r = -EIO;
- goto fail;
- }
-
- return 0;
-
-fail:
- if (msg)
- dbus_message_unref(msg);
-
- return r;
-}
-
int rm_watch(
rm_monitor **_m,
DBusConnection *connection,
@@ -243,7 +188,7 @@ int rm_watch(
m->matching = 1;
- if ((r = get_name_owner(m->connection, m->service_name, &name_owner, error)) < 0)
+ if ((r = rd_dbus_get_name_owner(m->connection, m->service_name, &name_owner, error)) < 0)
goto fail;
m->busy = get_busy(m->connection, name_owner);
diff --git a/src/modules/reserve.c b/src/modules/reserve.c
index b4c168c..bbb6773 100644
--- a/src/modules/reserve.c
+++ b/src/modules/reserve.c
@@ -606,3 +606,59 @@ void* rd_get_userdata(rd_device *d) {
return d->userdata;
}
+
+int rd_dbus_get_name_owner(
+ DBusConnection *connection,
+ const char *name,
+ char **name_owner,
+ DBusError *error) {
+
+ DBusMessage *msg, *reply;
+ int r;
+
+ *name_owner = NULL;
+
+ if (!(msg = dbus_message_new_method_call(DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "GetNameOwner"))) {
+ r = -ENOMEM;
+ goto fail;
+ }
+
+ if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, &name, DBUS_TYPE_INVALID)) {
+ r = -ENOMEM;
+ goto fail;
+ }
+
+ reply = dbus_connection_send_with_reply_and_block(connection, msg, DBUS_TIMEOUT_USE_DEFAULT, error);
+ dbus_message_unref(msg);
+ msg = NULL;
+
+ if (reply) {
+ if (!dbus_message_get_args(reply, error, DBUS_TYPE_STRING, name_owner, DBUS_TYPE_INVALID)) {
+ dbus_message_unref(reply);
+ r = -EIO;
+ goto fail;
+ }
+
+ *name_owner = strdup(*name_owner);
+ dbus_message_unref(reply);
+
+ if (!*name_owner) {
+ r = -ENOMEM;
+ goto fail;
+ }
+
+ } else if (dbus_error_has_name(error, "org.freedesktop.DBus.Error.NameHasNoOwner"))
+ dbus_error_free(error);
+ else {
+ r = -EIO;
+ goto fail;
+ }
+
+ return 0;
+
+fail:
+ if (msg)
+ dbus_message_unref(msg);
+
+ return r;
+}
diff --git a/src/modules/reserve.h b/src/modules/reserve.h
index bc50870..6527bd7 100644
--- a/src/modules/reserve.h
+++ b/src/modules/reserve.h
@@ -72,6 +72,15 @@ void rd_set_userdata(rd_device *d, void *userdata);
* userdata was set. */
void* rd_get_userdata(rd_device *d);
+/* Helper function to get the unique connection name owning a given
+ * name. Returns 0 on success, a negative errno style return value on
+ * error. */
+int rd_dbus_get_name_owner(
+ DBusConnection *connection,
+ const char *name,
+ char **name_owner,
+ DBusError *error);
+
#ifdef __cplusplus
}
#endif
More information about the pulseaudio-commits
mailing list