Re: [Qemu-devel] paaudio race condition on close

[Markus Armbruster]

If Gerd (cc'ed) doesn't know, then I'm afraid nobody around here does.

Peter Maydell <address@hidden> writes:

> There seems to be a race condition in the pulseaudio backend on
> closing of a voice...
>
> There are two threads involved here. The first one is a worker
> thread that just sits executing our qpa_thread_in() function
> to get input audio from PA and feed it to us. The second thread
> is QEMU itself (the cpu thread in the case of TCG), which calls
> AUD_close_in():
>
> #0  0x00007ffff054466b in pthread_join (threadid=140736709240576,
> thread_return=0x7fffdfdcc130)
>     at pthread_join.c:92
> #1  0x00005555557def8f in audio_pt_join (p=0x7fffbc0195b8, arg=0x7fffdfdcc178,
>     cap=0x555555b0dfd5 <__FUNCTION__.21892> "qpa_fini_in")
>     at /home/petmay01/linaro/qemu-from-laptop/qemu/audio/audio_pt_int.c:166
> #2  0x00005555557ddec7 in qpa_fini_in (hw=0x7fffbc019520)
>     at /home/petmay01/linaro/qemu-from-laptop/qemu/audio/paaudio.c:689
> #3  0x00005555557d3cab in audio_pcm_hw_gc_in (hwp=0x7fffbc032c28)
>     at /home/petmay01/linaro/qemu-from-laptop/qemu/audio/audio_template.h:196
> #4  0x00005555557d432a in audio_close_in (sw=0x7fffbc032bd0)
>     at /home/petmay01/linaro/qemu-from-laptop/qemu/audio/audio_template.h:375
> #5  0x00005555557d43bc in AUD_close_in (card=0x555556a29a08, 
> sw=0x7fffbc032bd0)
>     at /home/petmay01/linaro/qemu-from-laptop/qemu/audio/audio_template.h:387
>
> qpa_fini_in() basically sets a "done" flag in the PAVoiceIn struct,
> and prods the worker thread to make sure it exits before continuing.
> Usually this works. Unfortunately if the input thread is in the
> middle of actually writing to the buffer then things can go pear
> shaped, because the sample buffer itself is freed earlier, by
> audio_pcm_hw_gc_in():
>
>         glue (s->nb_hw_voices_, TYPE) += 1;
>         glue (audio_pcm_hw_free_resources_ ,TYPE) (hw);
>         glue (hw->pcm_ops->fini_, TYPE) (hw);
>
> So we end up freeing hw resources (like the conv_buf) before we
> give the pulseaudio backend a chance to actually shut down its
> helper thread.
>
> Does anybody know how this is supposed to work? Is the fix as
> simple as just moving the free_resources call below the fini?
>
> thanks
> -- PMM