Short-circuit in pm-utils hook can cause incorrect mixer state to be (re)stored

[Chow Loong Jin]

-- Lucid SRU report follows --

Impact: A short-circuit in PulseAudio's pm-utils hook to check for the ConsoleKit active seat when (re)storing sink and source states can result in the wrong states being stored. (This change was introduced for suspend-to-* and resume optimization.)

Bug resolution: Prior to suspend-to-*, save all PA users' sink & source states and then mute only the active ConsoleKit seat's sink & source. After resume, restore all PA users' pre-suspend sink & source states. This change has been made in revision 292: http://bazaar.launchpad.net/~ubuntu-core-dev/pulseaudio/ubuntu.2a/revision/292?remember=284&compare_revid=284 .

Minimal patch: http://bazaar.launchpad.net/%7Eubuntu-core-dev/pulseaudio/ubuntu.2a/diff/292/284?remember=284

TEST CASE: Detailed below in the original defect report

Regression potential: Suspend-to-* and resume will require [negligibly] more time. Otherwise, there are no identified regressions.

-- Original defect report follows --

Binary package hint: pulseaudio

I noticed this the other day after accidentally going into "Switch User" mode: I had my own pulseaudio instance, and gdm had its own. I then suspending my machine, and upon resuming, sound was muted.

The reason for this happening is due to a bug in the 01Pulseaudio pm-utils hook.
1. First, it checks the status of gdm's pulseaudio, stores it, then mutes it.
2. Then, it checks the status of my pulseaudio, stores it, and also mutes it. Due to #1, the sink is already muted, as both sinks were pointing to the same physical device. Hence, the stored states are as follows: gdm's sink is not muted, but mine is.

Upon resuming:
1. It reads gdm's state, and unmutes the sink.
2. Then it reads my pulseaudio's saved state, and mutes the sink again.

Result: The sink is muted after resuming from suspend.

N.B.:
This seems to be a corner case that only occurs when multiple pulseaudio instances are running, and both contain sinks pointing to the same underlying device. It can probably be fixed by changing the pm-utils hook order to read and store all the states of the sinks prior to muting them.

[Daniel T Chen (crimsun)]

Actually, the proper fix is to rip out the pm-utils hook altogether. It's merely a workaround for broken HDA drivers (sadly, nearly all of them) anyhow.

[Chow Loong Jin (hyperair)]

On Saturday 24,April,2010 08:26 PM, Daniel T Chen wrote:
> Actually, the proper fix is to rip out the pm-utils hook altogether.
> It's merely a workaround for broken HDA drivers (sadly, nearly all of
> them) anyhow.
>

But are all the drivers fixed yet? From my memory, the HDA drivers had issues
with pulseaudio holding onto the sound card while suspending, causing them to
have issues resuming later on, and resulting in a hung pulseaudio. At least,
that is what prompted me to write that script. On my system, currently, (custom
2.6.34-rc5 + sound-2.6.git) it works fine without the script.

--
Kind regards,
Chow Loong Jin

[Daniel T Chen (crimsun)]

As discussed on irc, the way forward is twofold:

On the driver side, we need to fix the remaining controller/codec combos. This is fairly limited to Cirrus and some Conexant (on NVidia).

On the userspace side, the suspend and resume states need to be exposed over dbus.

[Daniel T Chen (crimsun)]

This is an ideal lucid-proposed candidate. We just need to remove the ck check and conditional (and break) from suspend_pulse() and resume_pulse(). If someone can prepare the description of this bug and generate a debdiff, that would be great.

[Daniel T Chen (crimsun)]

I've made this change in revision 290.

[Chow Loong Jin (hyperair)]

Looking at the minimal patch linked in the description, I actually think it
would still cause issues, as the issue was the order of muting/saving the
sessions. Basically:

1. Save user A's state
2. Mute user A's sink+source
3. Save user B's state
4. Mute user B's sink+source
---suspend and resume---
5. Read and restore user A's sink+source state (not muted)
6. Read and restore user B's sink+state (muted)

But step #2 caused step #3 to be saved as muted rather than not muted, since the
physical sinks are the same.

I think the solution would be to loop twice through the pulse users, saving the
states the first round, and muting+suspending the sinks and sources the second
round.

Also, I am still using Ubuntu Karmic, and the 01Pulseaudio hook in Karmic does
not have the ConsoleKit bits.

--
Kind regards,
Chow Loong Jin

[Launchpad Janitor (janitor)]

This bug was fixed in the package pulseaudio - 1:0.9.22~0.9.21+stable-queue-32-g8478-0ubuntu15

---------------
pulseaudio (1:0.9.22~0.9.21+stable-queue-32-g8478-0ubuntu15) maverick; urgency=low

  * 0093-backport-fixes-stable-queue-head.patch: Backport the following
    changesets from the stable-queue branch:
    + bc7314f (name all threads for /proc/$PID/task/$TID/comm)
    + d519ca4 (prevent crash on jack server shutdown) (LP: #538815)
    + 65f89dc (fix bracketing in pa_rtp_recv())
  * debian/01PulseAudio: modify the ConsoleKit active seat checks in the
    pm-utils hook so that:
    - pre-suspend: all users' sink & source states are stored, then only
      the active seat's sinks and sources are muted
    - post-resume: all users' sink & source pre-suspend states are
      restored
    This resolves the issue where users' post-resume sink & source
    states don't correspond with their pre-suspend ones.
    (LP: #569395)
  * debian/01PulseAudio: fix broken quoting and misuse of su -l. Merged
    from lp:~hyperair/pulseaudio/fix-01Pulseaudio-quoting. Thanks,
    Chow Loong Jin! (LP: #572391)
  * debian/control: Update bzr branch for maverick.
 -- Daniel T Chen <email address hidden> Sat, 08 May 2010 11:08:56 -0400

[Luke Yelavich (themuso)]

The code for this is sitting in bzr, but it doesn't seem this was uploaded to lucid-proposed. Should we push this to lucid-proposed for woder testing?

[Luke Yelavich (themuso)]

Is there anybody still using lucid, who is affected by this bug?

[Rolf Leggewie (r0lf)]

lucid has seen the end of its life and is no longer receiving any updates. Marking the lucid task for this ticket as "Won't Fix".