[PATCH 10/25] mciseq: Limit concurrency when starting to play.

Joerg-Cyril.Hoehle at t-systems.com Joerg-Cyril.Hoehle at t-systems.com
Mon Oct 8 13:22:00 CDT 2012


Alexandre wrote:

>>   if (wmm->dwStatus == MCI_MODE_PAUSE) continue;
>>   if (wmm->dwStatus != MCI_MODE_PLAY)  break; but that's not the same.  
>> A concurrent thread might change dwStatus to PAUSE right between the 2 
>> lines.

>If the code depends on that sort of thing then it's broken.
>If another thread can change dwStatus you need a critical section
>or an interlocked function.

I believe it's not broken:
1. All writers of wmm->dwStatus are already within a critical section
  (except at mciOpen time where there's no threading to consider).
2. The player MUST NOT get stuck in a CS while playing.
3. But it must read that variable.
4. I believe Interlocked* is strictly superfluous when only reading, so
   I see no value in InterlockedExchange or InterlockedCompareExchange here.

The player must consider dwStatus conceptually "volatile" (without requiring
the broken C keyword). Therefore one has to carefully
think about possible values of (dwStatus == MCI_PLAY || dwStatus == MODE_PAUSE).
The value could switch from PAUSE to PLAY just when evaluating ||.

However, the statement
+	if (wmm->dwStatus == MCI_MODE_STOP || wmm->dwStatus == MCI_MODE_NOT_READY)
+	    break;
cannot be fooled by interleaved changes.

Regards,
	Jörg Höhle


More information about the wine-devel mailing list