Disconnecting an audio device while playing on it causes a deadlock (PulseAudio)

[timoschwarzer]

Reproduction:

• miniaudio from current dev branch
• Pipewire v1.2.1 (unclear whether this happens only with PipeWire or not)
• Play a sound on a device, preferably a USB audio device to be able to disconnect it
• Disconnect the device while playing a sound

The worker thread gets stuck in ma_device_data_loop__pulse, specifically on the pa_mainloop_iterate invocation. ma_device_uninit will deadlock because it will join the worker thread. I have not found a solution or workaround to prevent that yet.

[timoschwarzer]

Making the call to pa_mainloop_iterate non-blocking resolves this issue for me. Since you are way more experienced with these APIs, maybe you know about other consequences this change might have.

- resultPA = ((ma_pa_mainloop_iterate_proc)pDevice->pContext->pulse.pa_mainloop_iterate)((ma_pa_mainloop*)pDevice->pulse.pMainLoop, 1, NULL);
+ resultPA = ((ma_pa_mainloop_iterate_proc)pDevice->pContext->pulse.pa_mainloop_iterate)((ma_pa_mainloop*)pDevice->pulse.pMainLoop, 0, NULL);

I found this solution in a similar issue happening to the people at SDL: libsdl-org/SDL#3996 (comment)

They fixed it by making the call non-blocking and adding a short sleep to not eat all the CPU power.

[timoschwarzer]

This is my current solution inside ma_device_data_loop__pulse:

    while (ma_device_get_state(pDevice) == ma_device_state_started) {
-       resultPA = ((ma_pa_mainloop_iterate_proc)pDevice->pContext->pulse.pa_mainloop_iterate)((ma_pa_mainloop*)pDevice->pulse.pMainLoop, 1, NULL);
+       resultPA = ((ma_pa_mainloop_iterate_proc)pDevice->pContext->pulse.pa_mainloop_iterate)((ma_pa_mainloop*)pDevice->pulse.pMainLoop, 0, NULL);
        if (resultPA < 0) {
            break;
        }

+       if (((ma_pa_stream_readable_size_proc)pDevice->pContext->pulse.pa_stream_readable_size)((ma_pa_stream*)pDevice->pulse.pStreamPlayback) == 0) {
+           ma_sleep(5);
+       }
    }

[mackron]

I had another recent report about a deadlock with the PulseAudio backend (#877). I bet this would also fix that one. I can't think of any other solution so I guess I'll have to implement your proposal and see how it goes. Though I will say that the loop-with-sleep pattern is a very untidy solution and is very disappointing on the part of PipeWire's PulseAudio emulation layer.

Everything has been going well with the PulseAudio backend for a while now, and now all of a sudden I get two PipeWire related bug reports about deadlocks in the space of 2 weeks or so. Makes me wonder if PipeWire has introduced a bug recently. But in any case, I'll implement your proposed fix and report back.

[mackron]

I've pushed a potential fix to the dev branch. My change is slightly different to yours. I don't use pa_stream_readable_size() at all and instead just go straight to the sleep. The reason is that pa_stream_readable_size() would be used for a capture stream, whereas pa_stream_writable_size() is used for a playback stream, and I figured it was simpler to just go straight to the sleep rather than having to worry about branching based on the device type. I also had to make the timeout 1ms because any longer than that and I was getting glitching.

I also added a variable to the device config to enable a blocking PulseAudio loop in case people want to use the previous functionality.

I was unable to reproduce this on my system so I've not been able to verify the fix. Are you able to try the dev branch and see if that works for you?

[mackron]

Sorry for the message spam. I had to make it so the device config defaults to a blocking loop. To test this you would need to do this in the device config: deviceConfig.pulse.blockingMainLoop = MA_FALSE;

The problem is that when using a non-blocking loop with a sleep I get horrendous glitching with smaller period sizes. I can't be introducing a regression like that into the library so this will need to be considered an experimental feature that you need to opt in to. I don't know how to address this right now without causing other problems elsewhere. Even 1ms is causing glitching, which is interesting to me because that should be more than enough resolution. If I don't sleep at all it works, but then a CPU core is pinned at 100% which is unacceptable.

[timoschwarzer]

@mackron Thank you for looking into this! I would assume the glitching from lower period sizes comes from the fact that, by default, sleeping has a lower bound when not running with realtime scheduling. So I would assume that ma_sleep(1); actually sleeps longer than 1 millisecond. I will test that later.

[timoschwarzer]

Nope, wasn't that. I'm out of ideas then for now, too.

[mackron]

I doubt it would make any difference, but for what it's worth I'm using PipeWire version 1.2.2. It feels like this is just a bug in PipeWire, or maybe something hardware specific, but it would still be nice to figure out a workaround that works nicely across the board. I might have to review how SDL is using PulseAudio to get an idea on how it's working for them but not miniaudio. Certainly there's nothing obviously wrong with miniaudio.

[timoschwarzer]

I will try reproducing the bug with a blocking main loop on 1.2.2 tomorrow 👍

[mackron]

Just FYI on this, I've reverted those changes regarding the deviceConfig.pulse.blockingMainLoop thing. I've had a report about a possible PulseAudio regression and I'm just eliminating these commits as a possible cause. I'll consider re-adding this config option later.

[timoschwarzer]

@mackron thanks for letting me know! 🙌

[mackron]

Just checking in to see if by chance you've been able to upgrade PipeWire and if this issue is still happening? No rush or anything.