ScopedMutex in Zero Hour looks dangerous, has timeout of 500 milliseconds

[xezon]

Generals

		ScopedMutex(HANDLE mutex) : m_mutex(mutex)
		{
			WaitForSingleObject(m_mutex, INFINITE);
		}

Zero Hour

		ScopedMutex(HANDLE mutex) : m_mutex(mutex)
		{
			DWORD status = WaitForSingleObject(m_mutex, 500);
			if (status != WAIT_OBJECT_0) {
				DEBUG_LOG(("ScopedMutex WaitForSingleObject timed out - status %d\n", status));
			}
		}

Using a timeout of 500 milliseconds for a mutex is bonkers. Unclear what the rationale here was but this looks like a bug, albeit an unlikely one.

The scoped mutex is used exclusively in MilesAudioManager.cpp

[Mauller]

Although the generals version waits for uint32 Max milliseconds.

I wonder if there is an issue with acquiring resources from the miles DLL and things lock up at times.

[xezon]

I think INFINITE actually stands for infinite. It makes no sense to timeout a mutex like this and then still go ahead and run the code that is supposed to be locked. Typically a timeout should bail out, not proceed.

I think the fix here is to delete the ScopedMutex and simply use a critical section for the locking.

[Mauller]

Ah yeah i get what you mean now, yeah the return should be getting handled properly and exiting the code path instead of continuing.