Bug#708760: libsdl1.2debian: darkplaces doesn't quit when using PulseAudio 3.0

Sat May 18 12:23:41 UTC 2013

Package: libsdl1.2debian
Version: 1.2.15-5
Severity: important
Tags: patch
Control: affects -1 darkplaces

--- Steps to reproduce ---

Run a game that uses the darkplaces engine (nexuiz, or quake if you have
the right proprietary data). Exit the game (either use the GUI, or pull down
the console with Shift+Escape and type "quit").

I recommend testing this under gdb and in windowed mode, so that it's
easier to recover from this bug, with one of these:

    NEXUIZ_DEBUGGER="gdb --args" nexuiz -window
    quake --engine="gdb --args darkplaces" -window

but the bug is also reproducible without gdb and in full-screen.

--- Expected result ---

The game quits.

--- Actual result ---

The game hangs. It isn't using CPU time, but doesn't quit. When not using
gdb, Ctrl+C doesn't work, killing the engine with SIGTERM
(the process' name is "darkplaces") doesn't work either, but kill -9 works.

I've filed this as "important" since if you're running the game in
fullscreen, it's difficult to kill it in this situation (you would have to
switch to a text console).

So far I've only checked this on a mixed unstable/experimental system
with PulseAudio 3 from experimental; I'll try to reproduce it on a wheezy
system with PulseAudio 2.

--- Analysis ---

Under gdb, interrupting the game while in this stage produces:

> (gdb) thread apply all bt
> 
> Thread 3 (Thread 0x7fffe4f56700 (LWP 16985)):
> #0  0x00007ffff4f8827f in __GI_ppoll (fds=<optimized out>, 
>     nfds=<optimized out>, timeout=<optimized out>, sigmask=sigmask at entry=0x0)
>     at ../sysdeps/unix/sysv/linux/ppoll.c:57
> #1  0x00007ffff2dfc4f7 in pa_mainloop_poll (m=m at entry=0x3047340)
>     at pulse/mainloop.c:862
> #2  0x00007ffff2dfcab9 in pa_mainloop_iterate (m=0x3047340, 
>     block=block at entry=1, retval=retval at entry=0x0) at pulse/mainloop.c:936
> #3  0x00007ffff58a0199 in PULSE_WaitAudio (this=0x30a6cf0)
>     at ../../src/audio/pulse/SDL_pulseaudio.c:310
> #4  0x00007ffff58730d0 in SDL_RunAudio (audiop=audiop at entry=0x30a6cf0)
>     at ../../src/audio/SDL_audio.c:222
> #5  0x00007ffff587b406 in SDL_RunThread (data=0x30fed20)
>     at ../../src/thread/SDL_thread.c:204
> #6  0x00007ffff58be899 in RunThread (data=<optimized out>)
>     at ../../src/thread/pthread/SDL_systhread.c:47
> #7  0x00007ffff3a43e0e in start_thread (arg=0x7fffe4f56700)
>     at pthread_create.c:311
> #8  0x00007ffff4f9395d in clone ()
>     at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
> 
> Thread 1 (Thread 0x7ffff7ee3780 (LWP 16981)):
> #0  0x00007ffff3a44ff8 in pthread_join (threadid=140737034675968, 
>     thread_return=thread_return at entry=0x0) at pthread_join.c:92
> #1  0x00007ffff58be9bb in SDL_SYS_WaitThread (thread=thread at entry=0x2e2f310)
>     at ../../src/thread/pthread/SDL_systhread.c:107
> #2  0x00007ffff587b61e in SDL_WaitThread (thread=0x2e2f310, status=0x0)
>     at ../../src/thread/SDL_thread.c:272
> #3  0x00007ffff587384a in SDL_AudioQuit () at ../../src/audio/SDL_audio.c:629
> #4  0x00007ffff5872775 in SDL_QuitSubSystem (flags=flags at entry=16)
>     at ../../src/SDL.c:190
> #5  0x00007ffff58732ca in SDL_CloseAudio () at ../../src/audio/SDL_audio.c:619
> #6  0x0000000000416229 in SndSys_Shutdown () at snd_sdl.c:181
> #7  0x000000000040eb88 in S_Shutdown () at snd_main.c:775
> #8  S_Terminate () at snd_main.c:931
> #9  0x0000000000514f5a in Host_Shutdown () at host.c:1411
> #10 0x000000000064bc90 in Sys_Quit (returnvalue=0) at sys_shared.c:51
> #11 0x0000000000484f82 in Cmd_ExecuteString (
>     text=text at entry=0x7fffffff9880 "quit", src=src at entry=src_command, 
>     lockmutex=lockmutex at entry=false) at cmd.c:1757
> #12 0x00000000004852c9 in Cbuf_Execute () at cmd.c:364
> #13 0x0000000000485601 in Cbuf_Frame () at cmd.c:386
> #14 0x000000000051462b in Host_Main () at host.c:773
> #15 0x0000000000409443 in main (argc=5, argv=0x7fffffffde98) at sys_sdl.c:201

I believe the problem here is that the audio thread basically takes this
form:

    while (audio->enabled) {
        ... play audio for a short time ...
    }

and when the main thread calls SDL_AudioQuit, this consists of setting
audio->enabled to 0 and then waiting for the audio thread to terminate.
Unfortunately, memory writes on CPU 1 are not guaranteed to become visible
to CPU 2 in the absence of a memory barrier, so there is no guarantee that
the audio thread will see audio->enabled changing. Perhaps this used to work
because something else was issuing a memory barrier (e.g. locking a mutex),
but that no longer happens?

The attached patch works as a proof-of-concept, but is neither portable nor
efficient: __sync_*() is gcc-specific, and the audio thread will flush
the CPU cache once per iteration (which appears to be once per millisecond
when using the PulseAudio driver).

It might be enough to declare SDL_AudioDevice.enabled as "volatile int",
and only have the memory barrier in the main thread, but I haven't tried that
version yet.

If API changes in SDL 2 are still OK, a better solution upstream for
SDL 2 might be to move the audio-thread loop into the driver: then instead
of repeatedly running pa_mainloop_iterate(), the PulseAudio driver could
block in pa_mainloop_run() in the audio thread, and call a "wake up" function
(commonly implemented as a pipe-to-self) from the main thread?

Regards,
    S

-- System Information:
Debian Release: jessie/sid
  APT prefers proposed-updates
  APT policy: (500, 'proposed-updates'), (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.8-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.utf8, LC_CTYPE=en_GB.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages libsdl1.2debian depends on:
ii  libasound2         1.0.27-4
ii  libc6              2.17-3
ii  libcaca0           0.99.beta18-1
ii  libdirectfb-1.2-9  1.2.10.0-5
ii  libpulse0          3.0-1
ii  libts-0.0-0        1.0-11
ii  libx11-6           2:1.5.0-1
ii  libxext6           2:1.3.1-2
ii  multiarch-support  2.17-3

libsdl1.2debian recommends no packages.

libsdl1.2debian suggests no packages.

-- no debconf information