[PATCH2/2] winmm: PlaySound concurrency cleanup.
eric.pouech at orange.fr
Mon Mar 28 06:14:32 CDT 2011
sharing a global variable between threads without proper sync protection or
atomic operation is the wrong thing to do
moreover you're doing several unrelated changes in the same patch, please
2011/3/28 <Joerg-Cyril.Hoehle at t-systems.com>
> - PlaySound_Alloc does not call _Free anymore, so it does not use WINMM_cs
> anymore, which makes it easier to reason about use of critical sections.
> - psStopEvent was never used like an event rather than a boolean.
> - No need to muck with WHDR_DONE.
> - No need to keep the unused thread handle object around.
> - Abort the playloop in case of error, like I did for mciwave. The error
> may not be transient.
> - Free waveHdr and hEvent after closing hWave. Either you believe that
> WAVERR_STILLPLAYING can happen, then obviously waveHdr is in use and must
> not yet be freed, or you believe checking for STILLPLAYING is a waste of
> code because waveOutReset must have done its job.
> I should have added an ERR() message.
> One should think again about what to do in such a case. Why/when would
> sleep help? User-friendlier might be not to wait, not to free WaveHdr
> and accept to leak resources in such a case.
> Actually, waveOutClose can fail with STILLPLAYING because waveOutReset
> can fail. If you look at winealsa, you'll see it uses CreateEvent to
> synchronously wait for acknowledge of the message. Now guess what
> happens if CreateEvent fails because memory is tight?
> What happens in the app when waveOutClose's CreateEvent fails?
> It's always tough to reason about the many error paths.
> Jörg Höhle
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the wine-devel