Brian found himself digging through some C++ code, trying to figure out a cross-thread synchronization bug. It had something to do with the NetLockWait
function, based on his debugging, so he dug into the code.
bool CMyDatastore::NetLockWait(DWORD dwEvent, long nRecord, CMySignal& Signal, DWORD dwTieout)
{
bool retBool;
long value;
DWORD reason;
DWORD currentAttach;
CTimerElapsed timeout;
timeout.SetTime(dwTimeout);
retBool = false;
while (timeout)
{
if (::WaitForSingleObject(Signal.GetEvent(), timeout) != WAIT_OBJECT_0)
{
break;
}
ReserveSynch();
Signal.Pop(reason, value);
ReleaseSynch();
if (reason == dwEvent && value == nRecord)
{
retBool = true;
break;
}
}
return (retBool);
}
While reading this, something leapt out to Brian- the variable timeout
was never changed inside the loop. So, this is a simple bug, right? The timeout is basically ignored, and the loop keeps retrying until it hits one of the break
statements?
Well… no. In fact, the loop did successfully time out, and timeouts had nothing to do with the synchronization bug. Brian couldn’t help but be curious- how did the timeout actually get decremented?
CTimerElapsed::operator DWORD(void)
{
DWORD wait = 0;
DWORD dwCurrentTick;
DWORD dwElapsed;
if (m_dwTimeout == INFINITE)
{
wait = INFINITE;
}
else
{
dwCurrentTick = ::GetTickCount();
if (dwCurrentTick < m_dwStartTicks)
dwElapsed = (UINT_MAX - m_dwStartTicks) + dwCurrentTick;
else
dwElapsed = dwCurrentTick - m_dwStartTicks;
if (dwElapsed < m_dwTimeout)
wait = m_dwTimeout - dwElapsed;
}
return (wait);
}
A number of languages prohibit operator overloading, for good reasons. C++ is highly optimized for autopodóplo- shooting ones own foot. Thus, C++ cheerfully lets you create this monstrosity.
This block here overrides the DWORD
type cast- whenever a CTimerElapsed
object is used where a DWORD
is expected, this code is called to convert the CTimerElapsed
instance into a DWORD
, decrementing the count, and pushing the boundaries of what operator overloading is for.
This code was not the source of Brian’s bug. This code works. It’s not wrong, but it’s wrong.
![](https://thedailywtf.com/images/inedo/buildmaster-icon.png)