[PATCH v3 1/2] winmm: Test that a callback passed to `waveOutOpen` can call `waveOutWrite` without getting called reentrantly.
This behaviour is relied upon by Teardown, see https://github.com/ValveSoftware/Proton/issues/4332. Signed-off-by: Liam Murphy <liampm32(a)gmail.com> --- v2: Add `todo_wine` to avoid test failure --- v3: Use block comments instead of line comments --- 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..619c4f9339e 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 running. */ + 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"); -- 2.34.1
The events are then fired after the callback completes. Signed-off-by: Liam Murphy <liampm32(a)gmail.com> --- v3: Use block comments instead of line comments --- 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 619c4f9339e..61c8cae72a5 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..32104d5abbb 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; -- 2.34.1
On Fri, Jan 21, 2022 at 03:37:27PM +1100, Liam Murphy wrote:
The events are then fired after the callback completes.
I think a simpler way to solve this is to only process audio data during waveOutWrite if the device is stopped, see below patch. This should prevent us sending reentrant WOM_DONE messages. Can you try it out and see if that fixes your game? --- dlls/winmm/waveform.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dlls/winmm/waveform.c b/dlls/winmm/waveform.c index 1159b48b483..2813f9a103f 100644 --- a/dlls/winmm/waveform.c +++ b/dlls/winmm/waveform.c @@ -1948,11 +1948,10 @@ static MMRESULT WINMM_BeginPlaying(WINMM_Device *device) · TRACE("(%p)\n", device->handle); · - if(device->render) - /* prebuffer data before starting */ - WOD_PushData(device); - if(device->stopped){ + if(device->render) + WOD_PushData(device); + device->stopped = FALSE; · hr = IAudioClient_Start(device->client); --· 2.34.1
Hey Liam, Thanks very much for your analysis and work on fixing this. I'll try to get to reviewing it soon, but it's a big diff and we're in the middle of working on a major Wine upgrade for Proton, so I might not have time for it until sometime next week. BTW, if you're interested, I have a nest.py script in my tools repo which will auto-indent relay logs, similar to how you did on the GitHub issue: https://gitlab.com/mywinetools/mywinetools/-/blob/master/nest.py Will get back to you soon, Andrew On Fri, Jan 21, 2022 at 03:37:26PM +1100, Liam Murphy wrote:
This behaviour is relied upon by Teardown, see https://github.com/ValveSoftware/Proton/issues/4332.
Signed-off-by: Liam Murphy <liampm32(a)gmail.com> --- v2: Add `todo_wine` to avoid test failure --- v3: Use block comments instead of line comments --- 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..619c4f9339e 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 running. */ + 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"); -- 2.34.1
participants (2)
-
Andrew Eikum -
Liam Murphy