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
A+ 2011/3/28 <joerg-cyril.hoe...@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
