This behaviour is relied upon by Teardown, see https://github.com/ValveSoftware/Proton/issues/4332.
Signed-off-by: Liam Murphy liampm32@gmail.com --- v2: Add `todo_wine` to avoid test failure --- dlls/winmm/tests/wave.c | 159 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+)
diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c index 2f0c8443363..05e5e3b2d71 100644 --- a/dlls/winmm/tests/wave.c +++ b/dlls/winmm/tests/wave.c @@ -865,6 +865,163 @@ EXIT: HeapFree(GetProcessHeap(), 0, frags); }
+struct reentrancy_callback_data { + HANDLE hevent; + // Which thread is currently running the callback, or 0 if it isn't. + DWORD running_thread; + // How many times the callback's been called. + BOOL call_num; +}; + +static void CALLBACK reentrancy_callback_func(HWAVEOUT hwo, UINT uMsg, + DWORD_PTR dwInstance, + DWORD_PTR dwParam1, DWORD_PTR dwParam2) +{ + struct reentrancy_callback_data *data = (struct reentrancy_callback_data *)dwInstance; + + data->call_num += 1; + + todo_wine_if(data->call_num == 3) + ok(data->running_thread != GetCurrentThreadId(), "winmm callback called reentrantly, with message %u\n", uMsg); + if (data->running_thread) { + if (data->running_thread != GetCurrentThreadId()) trace("Callback running on two threads simultaneously\n"); + + // Don't mess with what the original call was doing. + return; + } + data->running_thread = GetCurrentThreadId(); + + // We have to wait for a `WOM_DONE` event rather than just using the `WOM_OPEN` event + // so that we're running on the winmm audio thread, so that we can block it + // to prevent the next `WOM_DONE` event just getting fired in the background + // (and then presumably calling the callback on another thread?) + if (uMsg == WOM_DONE) { + if (data->call_num == 2) { + MMRESULT rc; + // Since this is the first `WOM_DONE`, and the first frame is always `frags[0]`, we know that this is actually a pointer to `frags`. + WAVEHDR *frags = (WAVEHDR *)dwParam1; + + rc = waveOutWrite(hwo, &frags[0], sizeof(frags[0])); + ok(rc==MMSYSERR_NOERROR,"waveOutWrite(): rc=%s\n", wave_out_error(rc)); + + // Each frame is supposed to last 100ms, so 150ms should be enough to guarantee that the last one has finished. + Sleep(150); + + // This is needed to trigger the bug in wine, + // since winmm checks for `WOM_DONE` within `WINMM_PushData`, which this calls. + rc = waveOutWrite(hwo, &frags[1], sizeof(frags[0])); + ok(rc==MMSYSERR_NOERROR,"waveOutWrite(): rc=%s\n", wave_out_error(rc)); + } + } + + data->running_thread = 0; + // Only do this at the end of the function so that it isn't possible for the main thread + // to mistakenly call `waveOutClose` while the test is running. + SetEvent(data->hevent); +} + +// Test whether the callback gets called reentrantly when `waveOutWrite` is called +// from within the callback passed to `waveOutOpen`. +static void wave_out_test_no_reentrancy(int device) +{ + HWAVEOUT wout; + HANDLE hevent = CreateEventW(NULL, FALSE, FALSE, NULL); + WAVEHDR *frags = 0; + MMRESULT rc; + int headers = 2; + int loops = 0; + WAVEFORMATEX format; + WAVEFORMATEX* pwfx = &format; + DWORD flags = CALLBACK_FUNCTION; + double duration = 0.2; + DWORD_PTR callback = 0; + struct reentrancy_callback_data callback_data; + DWORD_PTR callback_instance = 0; + char * buffer; + DWORD length; + DWORD frag_length; + int i; + BOOL done; + + format.wFormatTag=WAVE_FORMAT_PCM; + format.nChannels=1; + format.wBitsPerSample=8; + format.nSamplesPerSec=22050; + format.nBlockAlign=format.nChannels*format.wBitsPerSample/8; + format.nAvgBytesPerSec=format.nSamplesPerSec*format.nBlockAlign; + format.cbSize=0; + + callback = (DWORD_PTR)reentrancy_callback_func; + callback_data.hevent = hevent; + callback_data.running_thread = 0; + callback_data.call_num = 0; + callback_instance = (DWORD_PTR)(&callback_data); + + wout=NULL; + rc=waveOutOpen(&wout,device,pwfx,callback,callback_instance,flags); + if (rc!=MMSYSERR_NOERROR) { + trace("`waveOutOpen` failed with rc %s, skipping", mmsys_error(rc)); + goto EXIT; + } + + rc=WaitForSingleObject(hevent,9000); + ok(rc==WAIT_OBJECT_0, "missing WOM_OPEN notification\n"); + + frags = HeapAlloc(GetProcessHeap(), 0, headers * sizeof(WAVEHDR)); + + buffer=wave_generate_silence(pwfx,duration / (loops + 1),&length); + + /* make sure fragment length is a multiple of block size */ + frag_length = ((length / headers) / pwfx->nBlockAlign) * pwfx->nBlockAlign; + + for (i = 0; i < headers; i++) { + frags[i].lpData=buffer + (i * frag_length); + if (i != (headers-1)) + frags[i].dwBufferLength=frag_length; + else { + /* use remainder of buffer for last fragment */ + frags[i].dwBufferLength=length - (i * frag_length); + } + frags[i].dwFlags=0; + frags[i].dwLoops=0; + rc=waveOutPrepareHeader(wout, &frags[i], sizeof(frags[0])); + ok(rc==MMSYSERR_NOERROR, + "waveOutPrepareHeader(%s): rc=%s\n",dev_name(device),wave_out_error(rc)); + } + + if (rc==MMSYSERR_NOERROR) { + // Queue up the first fragment. + rc=waveOutWrite(wout, &frags[0], sizeof(frags[0])); + ok(rc==MMSYSERR_NOERROR,"waveOutWrite(%s): rc=%s\n", + dev_name(device),wave_out_error(rc)); + } + + done = FALSE; + while (!done) { + rc=WaitForSingleObject(hevent,8000); + ok(rc==WAIT_OBJECT_0, "missing WOM_DONE notification\n"); + + done = TRUE; + for (i = 0; i < headers; i++) { + if (!(frags[i].dwFlags & WHDR_DONE)) { + done = FALSE; + } + } + } + + // Calling `waveOutClose` from within the callback hangs on Windows, so we have to make sure to do it here instead. + rc = waveOutClose(wout); + ok(rc==MMSYSERR_NOERROR,"waveOutClose(): rc=%s\n", wave_out_error(rc)); + + rc=WaitForSingleObject(hevent,1500); + ok(rc==WAIT_OBJECT_0, "missing WOM_CLOSE notification\n"); + + HeapFree(GetProcessHeap(), 0, buffer); +EXIT: + CloseHandle(hevent); + HeapFree(GetProcessHeap(), 0, frags); +} + static void wave_out_test_device(UINT_PTR device) { WAVEOUTCAPSA capsA; @@ -984,6 +1141,8 @@ static void wave_out_test_device(UINT_PTR device) trace(" %s\n",wave_out_caps(capsA.dwSupport)); HeapFree(GetProcessHeap(), 0, nameA);
+ wave_out_test_no_reentrancy(device); + if (winetest_interactive && (device != WAVE_MAPPER)) { trace("Playing a 5 seconds reference tone.\n");
The events are then fired after the callback completes.
Signed-off-by: Liam Murphy liampm32@gmail.com --- dlls/winmm/tests/wave.c | 1 - dlls/winmm/waveform.c | 165 ++++++++++++++++++++++++++++++---------- 2 files changed, 124 insertions(+), 42 deletions(-)
diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c index 05e5e3b2d71..3e09ebcebb0 100644 --- a/dlls/winmm/tests/wave.c +++ b/dlls/winmm/tests/wave.c @@ -881,7 +881,6 @@ static void CALLBACK reentrancy_callback_func(HWAVEOUT hwo, UINT uMsg,
data->call_num += 1;
- todo_wine_if(data->call_num == 3) ok(data->running_thread != GetCurrentThreadId(), "winmm callback called reentrantly, with message %u\n", uMsg); if (data->running_thread) { if (data->running_thread != GetCurrentThreadId()) trace("Callback running on two threads simultaneously\n"); diff --git a/dlls/winmm/waveform.c b/dlls/winmm/waveform.c index 1159b48b483..55b18b3e4dd 100644 --- a/dlls/winmm/waveform.c +++ b/dlls/winmm/waveform.c @@ -67,12 +67,20 @@ WINE_DEFAULT_DEBUG_CHANNEL(winmm); #define AC_BUFLEN (10 * 100000) #define MAX_DEVICES 256 #define MAPPER_INDEX 0x3F +#define CB_RUNNING 0x10
typedef struct _WINMM_CBInfo { DWORD_PTR callback; DWORD_PTR user; + // Layout: 0b00000000_000r0ttt + // where t: callback type (`DCB_*` constants) + // r: Whether the callback is currently running, to avoid reentrancy. + // The 0 in the middle is `DCB_NOSWITCH`. DWORD flags; HWAVE hwave; + // Used to buffer data from `WIM_DATA` or `WOM_DONE` if the callback is still running, + // depending on whether this is an input or output device respectively. + WAVEHDR *first, *last; } WINMM_CBInfo;
struct _WINMM_MMDevice; @@ -169,6 +177,12 @@ typedef struct _WINMM_OpenInfo { WAVEFORMATEX *format; DWORD_PTR callback; DWORD_PTR cb_user; + // The high half of the bits are the same as `WINMM_CBInfo.flags`, + // and the low half have the layout 0b00000000_0000dmaq, + // where d: `WAVE_FORMAT_DIRECT`, + // m: `WAVE_MAPPED`, + // a: `WAVE_ALLOWSYNC`, + // q: `WAVE_FORMAT_QUERY`. DWORD flags; BOOL reset; } WINMM_OpenInfo; @@ -370,13 +384,103 @@ static WINMM_Device *WINMM_GetDeviceFromHWAVE(HWAVE hwave) return device; }
+// Note: the lock on `device` must already have been acquired before calling this function. +static inline void WINMM_SendBufferedMessages(WINMM_Device *device) { + WINMM_CBInfo* info = &device->cb_info; + // Make a copy of this to pass to the callback while within the critical section, + // since it can be mutated once we leave the critical section. + DWORD cb_type = info->flags; + + while (info->first) { + WAVEHDR* hdr; + WORD new_msg; + + if (device->render) { + new_msg = WOM_DONE; + } else { + new_msg = WIM_DATA; + } + + hdr = info->first; + info->first = hdr->lpNext; + hdr->lpNext = NULL; + + LeaveCriticalSection(&device->lock); + + DriverCallback(info->callback, cb_type, (HDRVR)info->hwave, + new_msg, info->user, (DWORD_PTR)hdr, 0); + + EnterCriticalSection(&device->lock); + } +} + /* Note: NotifyClient should never be called while holding the device lock - * since the client may call wave* functions from within the callback. */ -static inline void WINMM_NotifyClient(WINMM_CBInfo *info, WORD msg, DWORD_PTR param1, - DWORD_PTR param2) + * since the client may call wave* functions from within the callback. + * + * Any `WAVEHDR`s passed to this function must no longer be referenced by any other `WAVEHDR`s, + * so that their linked list can be reused to buffer them when this is called from within the callback. */ +static inline void WINMM_NotifyClient(WINMM_Device *device, WORD msg, WAVEHDR* param1, + WAVEHDR* param2) { - DriverCallback(info->callback, info->flags, (HDRVR)info->hwave, - msg, info->user, param1, param2); + WINMM_CBInfo *info; + DWORD cb_type; + + EnterCriticalSection(&device->lock); + info = &device->cb_info; + // Make a copy of this to pass to the callback while within the critical section, + // since it can be mutated once we leave the critical section. + cb_type = info->flags; + + if (info->flags & CB_RUNNING) { + switch (msg) { + case WIM_DATA: + case WOM_DONE: + param1->lpNext = NULL; + if (!info->first) { + info->first = info->last = param1; + } else { + info->last->lpNext = param1; + info->last = param1; + } + break; + case WIM_CLOSE: + case WOM_CLOSE: + // If this device is closing, it might get reopened as another kind of device + // with all the buffered messages wiped, causing them to never get sent. + // So we need to send them all now. + // + // On Windows, calling `waveOutClose` from within `DriverCallback` hangs anyway, + // so this causing reentrancy isn't really a problem. + WINMM_SendBufferedMessages(device); + + LeaveCriticalSection(&device->lock); + + DriverCallback(info->callback, cb_type, (HDRVR)info->hwave, + msg, info->user, (DWORD_PTR)param1, (DWORD_PTR)param2); + + EnterCriticalSection(&device->lock); + break; + } + } else { + if ((info->flags & DCB_TYPEMASK) == DCB_FUNCTION) { + info->flags |= CB_RUNNING; + } + + LeaveCriticalSection(&device->lock); + + DriverCallback(info->callback, cb_type, (HDRVR)info->hwave, + msg, info->user, (DWORD_PTR)param1, (DWORD_PTR)param2); + + EnterCriticalSection(&device->lock); + + if ((info->flags & DCB_TYPEMASK) == DCB_FUNCTION) { + WINMM_SendBufferedMessages(device); + + info->flags &= ~CB_RUNNING; + } + } + + LeaveCriticalSection(&device->lock); }
static MMRESULT hr2mmr(HRESULT hr) @@ -1202,6 +1306,8 @@ static LRESULT WINMM_OpenDevice(WINMM_Device *device, WINMM_OpenInfo *info, device->cb_info.callback = info->callback; device->cb_info.user = info->cb_user; device->cb_info.hwave = device->handle; + device->cb_info.first = NULL; + device->cb_info.last = NULL;
info->handle = device->handle;
@@ -1623,7 +1729,6 @@ static WAVEHDR *WOD_MarkDoneHeaders(WINMM_Device *device)
static void WOD_PushData(WINMM_Device *device) { - WINMM_CBInfo cb_info; HRESULT hr; UINT32 pad, bufsize, avail_frames, queue_frames, written, ofs; UINT32 queue_bytes, nloops; @@ -1752,15 +1857,13 @@ static void WOD_PushData(WINMM_Device *device) device->played_frames += avail_frames;
exit: - cb_info = device->cb_info; - LeaveCriticalSection(&device->lock);
while(first){ WAVEHDR *next = first->lpNext; first->dwFlags &= ~WHDR_INQUEUE; first->dwFlags |= WHDR_DONE; - WINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0); + WINMM_NotifyClient(device, WOM_DONE, first, 0); first = next; } } @@ -1860,7 +1963,6 @@ static void WID_PullACMData(WINMM_Device *device)
static void WID_PullData(WINMM_Device *device) { - WINMM_CBInfo cb_info; WAVEHDR *queue, *first = NULL, *last = NULL; HRESULT hr;
@@ -1926,8 +2028,6 @@ static void WID_PullData(WINMM_Device *device) }
exit: - cb_info = device->cb_info; - LeaveCriticalSection(&device->lock);
if(last){ @@ -1936,7 +2036,7 @@ exit: WAVEHDR *next = first->lpNext; first->dwFlags &= ~WHDR_INQUEUE; first->dwFlags |= WHDR_DONE; - WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0); + WINMM_NotifyClient(device, WIM_DATA, first, 0); first = next; } } @@ -1985,7 +2085,6 @@ static LRESULT WINMM_Pause(WINMM_Device *device)
static LRESULT WINMM_Reset(HWAVE hwave) { - WINMM_CBInfo cb_info; WINMM_Device *device = WINMM_GetDeviceFromHWAVE(hwave); BOOL is_out; WAVEHDR *first; @@ -2012,7 +2111,6 @@ static LRESULT WINMM_Reset(HWAVE hwave) device->last_clock_pos = 0; IAudioClient_Reset(device->client);
- cb_info = device->cb_info; is_out = device->render != NULL;
LeaveCriticalSection(&device->lock); @@ -2022,9 +2120,9 @@ static LRESULT WINMM_Reset(HWAVE hwave) first->dwFlags &= ~WHDR_INQUEUE; first->dwFlags |= WHDR_DONE; if(is_out) - WINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0); + WINMM_NotifyClient(device, WOM_DONE, first, 0); else - WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0); + WINMM_NotifyClient(device, WIM_DATA, first, 0); first = next; }
@@ -2732,7 +2830,7 @@ MMRESULT WINAPI waveOutOpen(LPHWAVEOUT lphWaveOut, UINT uDeviceID, { LRESULT res; WINMM_OpenInfo info; - WINMM_CBInfo cb_info; + WINMM_Device* device;
TRACE("(%p, %u, %p, %lx, %lx, %08x)\n", lphWaveOut, uDeviceID, lpFormat, dwCallback, dwInstance, dwFlags); @@ -2763,12 +2861,9 @@ MMRESULT WINAPI waveOutOpen(LPHWAVEOUT lphWaveOut, UINT uDeviceID, if(lphWaveOut) *lphWaveOut = (HWAVEOUT)info.handle;
- cb_info.flags = HIWORD(dwFlags & CALLBACK_TYPEMASK); - cb_info.callback = dwCallback; - cb_info.user = dwInstance; - cb_info.hwave = info.handle; + device = WINMM_GetDeviceFromHWAVE(info.handle);
- WINMM_NotifyClient(&cb_info, WOM_OPEN, 0, 0); + WINMM_NotifyClient(device, WOM_OPEN, 0, 0);
return res; } @@ -2780,7 +2875,6 @@ UINT WINAPI waveOutClose(HWAVEOUT hWaveOut) { UINT res; WINMM_Device *device; - WINMM_CBInfo cb_info;
TRACE("(%p)\n", hWaveOut);
@@ -2789,14 +2883,12 @@ UINT WINAPI waveOutClose(HWAVEOUT hWaveOut) if(!WINMM_ValidateAndLock(device)) return MMSYSERR_INVALHANDLE;
- cb_info = device->cb_info; - LeaveCriticalSection(&device->lock);
res = SendMessageW(g_devices_hwnd, WODM_CLOSE, (WPARAM)hWaveOut, 0);
if(res == MMSYSERR_NOERROR) - WINMM_NotifyClient(&cb_info, WOM_CLOSE, 0, 0); + WINMM_NotifyClient(device, WOM_CLOSE, 0, 0);
return res; } @@ -3388,7 +3480,7 @@ MMRESULT WINAPI waveInOpen(HWAVEIN* lphWaveIn, UINT uDeviceID, { LRESULT res; WINMM_OpenInfo info; - WINMM_CBInfo cb_info; + WINMM_Device* device;
TRACE("(%p, %x, %p, %lx, %lx, %08x)\n", lphWaveIn, uDeviceID, lpFormat, dwCallback, dwInstance, dwFlags); @@ -3419,12 +3511,9 @@ MMRESULT WINAPI waveInOpen(HWAVEIN* lphWaveIn, UINT uDeviceID, if(lphWaveIn) *lphWaveIn = (HWAVEIN)info.handle;
- cb_info.flags = HIWORD(dwFlags & CALLBACK_TYPEMASK); - cb_info.callback = dwCallback; - cb_info.user = dwInstance; - cb_info.hwave = info.handle; + device = WINMM_GetDeviceFromHWAVE(info.handle);
- WINMM_NotifyClient(&cb_info, WIM_OPEN, 0, 0); + WINMM_NotifyClient(device, WIM_OPEN, 0, 0);
return res; } @@ -3435,7 +3524,6 @@ MMRESULT WINAPI waveInOpen(HWAVEIN* lphWaveIn, UINT uDeviceID, UINT WINAPI waveInClose(HWAVEIN hWaveIn) { WINMM_Device *device; - WINMM_CBInfo cb_info; UINT res;
TRACE("(%p)\n", hWaveIn); @@ -3445,14 +3533,12 @@ UINT WINAPI waveInClose(HWAVEIN hWaveIn) if(!WINMM_ValidateAndLock(device)) return MMSYSERR_INVALHANDLE;
- cb_info = device->cb_info; - LeaveCriticalSection(&device->lock);
res = SendMessageW(g_devices_hwnd, WIDM_CLOSE, (WPARAM)hWaveIn, 0);
if(res == MMSYSERR_NOERROR) - WINMM_NotifyClient(&cb_info, WIM_CLOSE, 0, 0); + WINMM_NotifyClient(device, WIM_CLOSE, 0, 0);
return res; } @@ -3568,7 +3654,6 @@ UINT WINAPI waveInStart(HWAVEIN hWaveIn) */ UINT WINAPI waveInStop(HWAVEIN hWaveIn) { - WINMM_CBInfo cb_info; WINMM_Device *device; WAVEHDR *buf; HRESULT hr; @@ -3593,14 +3678,12 @@ UINT WINAPI waveInStop(HWAVEIN hWaveIn) }else buf = NULL;
- cb_info = device->cb_info; - LeaveCriticalSection(&device->lock);
if(buf){ buf->dwFlags &= ~WHDR_INQUEUE; buf->dwFlags |= WHDR_DONE; - WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)buf, 0); + WINMM_NotifyClient(device, WIM_DATA, buf, 0); }
return MMSYSERR_NOERROR;