[PATCH2/2] winmm: PlaySound concurrency cleanup.

Eric Pouech 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
split up


2011/3/28 <Joerg-Cyril.Hoehle at t-systems.com>

> Hi,
> - 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.
> Regards,
>        Jörg Höhle

Eric Pouech
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110328/0d56e332/attachment.htm>

More information about the wine-devel mailing list