CodeSOD: Overloaded Loop

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 autopodoplo- 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.

[Advertisement] Onsite, remote, bare-metal or cloud - create, configure and orchestrate 1,000s of servers, all from the same dashboard while continually monitoring for drift and allowing for instantaneous remediation. Download Otter today!