Race in native_midi_win32.c

[henricj]

When a song ends, there appears to be a race between native_midi_stop()'s call to midiStreamClose() and the MidiProc() callback triggered by the call to midiStreamStop(), which happens on a thread-pool worker thread (at least, it does on Windows 11).

Builds with AddressSanitizer enabled fail every time the dunelegacy intro sequence runs to completion when in "Music Type = xmi" mode. (Repro: Do an MSVC build of the xmidi-import branch with the "windows-x64-asan-debug" CMake preset--which shows up as "Windows x64 ASAN Debug" in the VS configuration drop-down, set the "Music Type" to "xmi" and the "Play Intro" to "true" in "Dune Legacy.ini". Then run dunelegacy.exe in the debugger and wait for the intro to complete.)

Here are stack traces showing the SDL audio thread and the worker thread when the break happened.

This is where the debugger breaks the worker thread while it runs MidiProc():

ntdll.dll!RtlReportCriticalFailure()
ntdll.dll!RtlpHeapHandleError()
ntdll.dll!RtlpHpHeapHandleError()
ntdll.dll!RtlpLogHeapFailure()
ntdll.dll!RtlpAnalyzeHeapFailure()
ntdll.dll!RtlGetUserInfoHeap()
kernel32.dll!GlobalHandle()
winmmbase.dll!midiOutUnprepareHeader()
midimap.dll!modUnprepare()
midimap.dll!modMessage()
winmmbase.dll!midiStreamMessage()
winmmbase.dll!midiOutUnprepareHeader()
dunelegacy.exe!BlockOut(_NativeMidiSong * song=0x000012ac44eb4e80) Line 64
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\codecs\native_midi\native_midi_win32.c(64)
dunelegacy.exe!MidiProc(HMIDIIN__ * hMidi=0x000012aa45041550, unsigned int uMsg=0x000003c9, unsigned __int64 dwInstance=0x0000000000000000, unsigned __int64 dwParam1=0x000012ac44eb4e90, unsigned __int64 dwParam2=0x0000000000000000) Line 178
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\codecs\native_midi\native_midi_win32.c(178)
winmmbase.dll!DriverCallback()
winmmbase.dll!midiOutStreamCallback()
winmmbase.dll!DriverCallback()
midimap.dll!modmCallback()
winmmbase.dll!DriverCallback()
winmmbase.dll!midiOutStreamCallback()
winmmbase.dll!DriverCallback()
winmmbase.dll!midiOutRemovePMBuffer()
winmmbase.dll!mmWndProc(struct HWND__ *,unsigned int,unsigned __int64,__int64)
user32.dll!UserCallWinProcCheckWow()
user32.dll!DispatchMessageWorker()
winmmbase.dll!mciwindow(void *)
clang_rt.asan_dbg_dynamic-x86_64.dll!__asan::AsanThread::ThreadStart(unsigned __int64,struct __sanitizer::atomic_uintptr_t *)
kernel32.dll!BaseThreadInitThunk()
ntdll.dll!RtlUserThreadStart()

And here here is the audio thread part-way through midiStreamClose():

winmmbase.dll!midiStreamClose()
dunelegacy.exe!native_midi_stop() Line 299
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\codecs\native_midi\native_midi_win32.c(299)
dunelegacy.exe!NATIVEMIDI_Stop(void * context=0x000011cbbf911a80) Line 78
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\codecs\music_nativemidi.c(78)
dunelegacy.exe!music_internal_halt() Line 1153
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\music.c(1153)
dunelegacy.exe!music_mixer(void * udata=0x0000000000000000, unsigned char * stream=0x000011dfbf8c3200, int len=0x00000000) Line 359
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\music.c(359)
dunelegacy.exe!mix_channels(void * udata=0x0000000000000000, unsigned char * stream=0x000011dfbf8c3200, int len=0x00001000) Line 294
    at C:\src\game\dunelegacy\external\SDL_mixer\SDL_mixer\src\mixer.c(294)
SDL2d.dll!SDL_RunAudio(void * devicep=0x000011c9bf8efa00) Line 727
    at C:\src\game\dunelegacy\external\vcpkg\vcpkg\buildtrees\sdl2\src\e268dc78a1-c01e78abe3.clean\src\audio\SDL_audio.c(727)
SDL2d.dll!SDL_RunThread(SDL_Thread * thread=0x000011c9bf8fab00) Line 275
    at C:\src\game\dunelegacy\external\vcpkg\vcpkg\buildtrees\sdl2\src\e268dc78a1-c01e78abe3.clean\src\thread\SDL_thread.c(275)
SDL2d.dll!RunThread(void * data=0x000011c9bf8fab00) Line 83
    at C:\src\game\dunelegacy\external\vcpkg\vcpkg\buildtrees\sdl2\src\e268dc78a1-c01e78abe3.clean\src\thread\windows\SDL_systhread.c(83)
SDL2d.dll!RunThreadViaCreateThread(void * data=0x000011c9bf8fab00) Line 93
    at C:\src\game\dunelegacy\external\vcpkg\vcpkg\buildtrees\sdl2\src\e268dc78a1-c01e78abe3.clean\src\thread\windows\SDL_systhread.c(93)
clang_rt.asan_dbg_dynamic-x86_64.dll!__asan::AsanThread::ThreadStart(unsigned __int64,struct __sanitizer::atomic_uintptr_t *)
kernel32.dll!BaseThreadInitThunk()
ntdll.dll!RtlUserThreadStart()

I think this is caused by midiOutUnprepareHeader() trying to use the hMidiStream that was just destroyed by the in-progress call to midiStreamClose().

The above traces are from SDL_mixer's main branch (on 1d00166, but it looks pretty much the same with the 2.0.4 release (not much of that code has changed).

[sezero]

This we should fix before the new release.

[henricj]

This change lets dunelegacy's intro sequence run properly with XMI music.

Thanks.

[slouken]

You’re welcome!