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

[David Henningsson]

On 01/29/2013 06:04 PM, Mikel Astiz wrote:
> 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 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 inconsistent
> 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.
> ---
>   src/modules/reserve.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/src/modules/reserve.c b/src/modules/reserve.c
> index bc2d697..6c315fe 100644
> --- a/src/modules/reserve.c
> +++ b/src/modules/reserve.c
> @@ -310,6 +310,20 @@ static DBusHandlerResult filter_handler(
>   			goto invalid;
>
>   		if (strcmp(name, d->service_name) == 0 && d->owning) {
> +			/* Request the actual owner of the name to avoid leaked
> +			 * NameLost signals from previous reservations. These could
> +			 * potentially remain unprocessed due to the pseudo-blocking
> +			 * call mechanism used during both acquisition and
> +			 * release. */

What do you think of this text?

/* Since signals are delivered asynchronously by the dBus daemon, we 
might get the NameLost signal too late. This can happen if we release 
the device and immediately after reacquire it. Therefore verify that the 
name is still lost. */


> +			if (!d->gave_up) {
> +				const char *un;
> +				char *name_owner;
> +
> +				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)

Name_owner needs to be freed.

> +						goto invalid; /* Name still owned by us */
> +			}
> +
>   			d->owning = 0;
>
>   			if (!d->gave_up)  {
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic