Skip to content

Commit 369c696

Browse files
mfleiszakallabeth
authored andcommitted
rdpsnd: Fix possible crash and deadlock in winmm backend
1 parent 53f7927 commit 369c696

File tree

1 file changed

+37
-55
lines changed

1 file changed

+37
-55
lines changed

channels/rdpsnd/client/winmm/rdpsnd_winmm.c

Lines changed: 37 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,6 @@
4141

4242
#include "rdpsnd_main.h"
4343

44-
#if defined(WITH_WINMM_SEMAPHORE)
45-
#define SEM_COUNT_MAX 6
46-
#endif
47-
4844
typedef struct rdpsnd_winmm_plugin rdpsndWinmmPlugin;
4945

5046
struct rdpsnd_winmm_plugin
@@ -56,9 +52,8 @@ struct rdpsnd_winmm_plugin
5652
UINT32 volume;
5753
wLog* log;
5854
UINT32 latency;
59-
#if defined(WITH_WINMM_SEMAPHORE)
60-
HANDLE semaphore;
61-
#endif
55+
HANDLE hThread;
56+
DWORD threadId;
6257
};
6358

6459
static BOOL rdpsnd_winmm_convert_format(const AUDIO_FORMAT* in, WAVEFORMATEX* out)
@@ -98,28 +93,27 @@ static BOOL rdpsnd_winmm_set_format(rdpsndDevicePlugin* device, const AUDIO_FORM
9893
return TRUE;
9994
}
10095

101-
static void CALLBACK waveOutProc(HWAVEOUT hwo, UINT uMsg, DWORD_PTR dwInstance, DWORD_PTR dwParam1,
102-
DWORD_PTR dwParam2)
96+
static DWORD WINAPI waveOutProc(LPVOID lpParameter)
10397
{
104-
rdpsndWinmmPlugin* winmm = (rdpsndWinmmPlugin*)dwInstance;
105-
LPWAVEHDR lpWaveHdr = (LPWAVEHDR)dwParam1;
106-
107-
switch (uMsg)
98+
MSG msg;
99+
while (GetMessage(&msg, NULL, 0, 0))
108100
{
109-
case WOM_OPEN:
110-
case WOM_CLOSE:
111-
break;
112-
case WOM_DONE:
113-
waveOutUnprepareHeader(hwo, lpWaveHdr, sizeof(WAVEHDR));
114-
free(lpWaveHdr->lpData);
115-
free(lpWaveHdr);
116-
#if defined(WITH_WINMM_SEMAPHORE)
117-
ReleaseSemaphore(winmm->semaphore, 1, NULL);
118-
#endif
119-
break;
120-
default:
101+
if (msg.message == MM_WOM_CLOSE)
102+
{
103+
/* device was closed - exit thread */
121104
break;
105+
}
106+
else if (msg.message == MM_WOM_DONE)
107+
{
108+
/* free buffer */
109+
LPWAVEHDR waveHdr = (LPWAVEHDR)msg.lParam;
110+
waveOutUnprepareHeader((HWAVEOUT)msg.wParam, waveHdr, sizeof(WAVEHDR));
111+
free(waveHdr->lpData);
112+
free(waveHdr);
113+
}
122114
}
115+
116+
return 0;
123117
}
124118

125119
static BOOL rdpsnd_winmm_open(rdpsndDevicePlugin* device, const AUDIO_FORMAT* format,
@@ -134,19 +128,22 @@ static BOOL rdpsnd_winmm_open(rdpsndDevicePlugin* device, const AUDIO_FORMAT* fo
134128
if (!rdpsnd_winmm_set_format(device, format, latency))
135129
return FALSE;
136130

137-
mmResult = waveOutOpen(&winmm->hWaveOut, WAVE_MAPPER, &winmm->format, (DWORD_PTR)waveOutProc,
138-
(DWORD_PTR)winmm, CALLBACK_FUNCTION);
131+
winmm->hThread = CreateThread(NULL, 0, waveOutProc, NULL, 0, &winmm->threadId);
132+
if (!winmm->hThread)
133+
{
134+
WLog_Print(winmm->log, WLOG_ERROR, "CreateThread failed: %" PRIu32 "", GetLastError());
135+
return FALSE;
136+
}
137+
138+
mmResult = waveOutOpen(&winmm->hWaveOut, WAVE_MAPPER, &winmm->format,
139+
(DWORD_PTR)winmm->threadId, 0, CALLBACK_THREAD);
139140

140141
if (mmResult != MMSYSERR_NOERROR)
141142
{
142143
WLog_Print(winmm->log, WLOG_ERROR, "waveOutOpen failed: %" PRIu32 "", mmResult);
143144
return FALSE;
144145
}
145146

146-
#if defined(WITH_WINMM_SEMAPHORE)
147-
ReleaseSemaphore(winmm->semaphore, SEM_COUNT_MAX, NULL);
148-
#endif
149-
150147
mmResult = waveOutSetVolume(winmm->hWaveOut, winmm->volume);
151148

152149
if (mmResult != MMSYSERR_NOERROR)
@@ -165,11 +162,6 @@ static void rdpsnd_winmm_close(rdpsndDevicePlugin* device)
165162

166163
if (winmm->hWaveOut)
167164
{
168-
#if defined(WITH_WINMM_SEMAPHORE)
169-
size_t x;
170-
for (x = 0; x < SEM_COUNT_MAX; x++)
171-
WaitForSingleObject(winmm->semaphore, INFINITE);
172-
#endif
173165
mmResult = waveOutReset(winmm->hWaveOut);
174166
if (mmResult != MMSYSERR_NOERROR)
175167
WLog_Print(winmm->log, WLOG_ERROR, "waveOutReset failure: %" PRIu32 "", mmResult);
@@ -180,6 +172,13 @@ static void rdpsnd_winmm_close(rdpsndDevicePlugin* device)
180172

181173
winmm->hWaveOut = NULL;
182174
}
175+
176+
if (winmm->hThread)
177+
{
178+
WaitForSingleObject(winmm->hThread, INFINITE);
179+
CloseHandle(&winmm->hThread);
180+
winmm->hThread = NULL;
181+
}
183182
}
184183

185184
static void rdpsnd_winmm_free(rdpsndDevicePlugin* device)
@@ -189,9 +188,6 @@ static void rdpsnd_winmm_free(rdpsndDevicePlugin* device)
189188
if (winmm)
190189
{
191190
rdpsnd_winmm_close(device);
192-
#if defined(WITH_WINMM_SEMAPHORE)
193-
CloseHandle(winmm->semaphore);
194-
#endif
195191
free(winmm);
196192
}
197193
}
@@ -274,18 +270,13 @@ static UINT rdpsnd_winmm_play(rdpsndDevicePlugin* device, const BYTE* data, size
274270
lpWaveHdr->dwBufferLength = (DWORD)size;
275271

276272
mmResult = waveOutPrepareHeader(winmm->hWaveOut, lpWaveHdr, sizeof(WAVEHDR));
277-
278273
if (mmResult != MMSYSERR_NOERROR)
279274
{
280275
WLog_Print(winmm->log, WLOG_ERROR, "waveOutPrepareHeader failure: %" PRIu32 "", mmResult);
281276
goto fail;
282277
}
283278

284-
#if defined(WITH_WINMM_SEMAPHORE)
285-
WaitForSingleObject(winmm->semaphore, INFINITE);
286-
#endif
287279
mmResult = waveOutWrite(winmm->hWaveOut, lpWaveHdr, sizeof(WAVEHDR));
288-
289280
if (mmResult != MMSYSERR_NOERROR)
290281
{
291282
WLog_Print(winmm->log, WLOG_ERROR, "waveOutWrite failure: %" PRIu32 "", mmResult);
@@ -325,12 +316,11 @@ UINT freerdp_rdpsnd_client_subsystem_entry(PFREERDP_RDPSND_DEVICE_ENTRY_POINTS p
325316

326317
if (waveOutGetNumDevs() == 0)
327318
{
328-
WLog_Print(winmm->log, WLOG_ERROR, "No sound playback device available!");
319+
WLog_Print(WLog_Get(TAG), WLOG_ERROR, "No sound playback device available!");
329320
return ERROR_DEVICE_NOT_AVAILABLE;
330321
}
331322

332323
winmm = (rdpsndWinmmPlugin*)calloc(1, sizeof(rdpsndWinmmPlugin));
333-
334324
if (!winmm)
335325
return CHANNEL_RC_NO_MEMORY;
336326

@@ -342,18 +332,10 @@ UINT freerdp_rdpsnd_client_subsystem_entry(PFREERDP_RDPSND_DEVICE_ENTRY_POINTS p
342332
winmm->device.Close = rdpsnd_winmm_close;
343333
winmm->device.Free = rdpsnd_winmm_free;
344334
winmm->log = WLog_Get(TAG);
345-
#if defined(WITH_WINMM_SEMAPHORE)
346-
winmm->semaphore = CreateSemaphore(NULL, 0, SEM_COUNT_MAX, NULL);
347-
if (!winmm->semaphore)
348-
goto fail;
349-
#endif
335+
350336
args = pEntryPoints->args;
351337
rdpsnd_winmm_parse_addin_args((rdpsndDevicePlugin*)winmm, args);
352338
winmm->volume = 0xFFFFFFFF;
353339
pEntryPoints->pRegisterRdpsndDevice(pEntryPoints->rdpsnd, (rdpsndDevicePlugin*)winmm);
354340
return CHANNEL_RC_OK;
355-
356-
fail:
357-
rdpsnd_winmm_free((rdpsndDevicePlugin*)winmm);
358-
return ERROR_INTERNAL_ERROR;
359341
}

0 commit comments

Comments
 (0)