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.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!