[PATCH2/2] winmm: PlaySound concurrency cleanup.

Joerg-Cyril.Hoehle at t-systems.com Joerg-Cyril.Hoehle at t-systems.com
Mon Mar 28 04:03:06 CDT 2011


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-winmm-PlaySound-concurrency-cleanup.patch
Type: application/octet-stream
Size: 6673 bytes
Desc: 0002-winmm-PlaySound-concurrency-cleanup.patch
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20110328/8934a099/attachment.obj>


More information about the wine-patches mailing list