[PATCH 3/3] winmm: PlaySound concurrency cleanup.

Joerg-Cyril.Hoehle at t-systems.com Joerg-Cyril.Hoehle at t-systems.com
Tue Mar 29 06:59:48 CDT 2011


- 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.
- No need to keep the unused thread handle object around.
- More error checking.

- 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 to see if that ever happens to somebody.
  (I should have used WARN instead of ERR in patch 1/3 when aborting the loop,
   however mciwave already uses ERR and I want code to be consistent).

  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.

 One possibility would be to pre-allocate one event. What are the limits on the
 number of events and handles per thread in Wine?

        Jörg Höhle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-winmm-PlaySound-concurrency-cleanup.patch
Type: application/octet-stream
Size: 3923 bytes
Desc: 0003-winmm-PlaySound-concurrency-cleanup.patch
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20110329/585c5d4f/attachment-0001.obj>

More information about the wine-patches mailing list