niemiro
niemiro

Reputation: 1838

Why isn't the mutex being aquired?

I have been looking into all of the different syncronization primitives available in the WinAPI, but have been struggling with what should have been something simple. Why doesn't the following code work?

class MultiThreadedCounter
{
private:
    int count; HANDLE hMutex;

public:
    void IncrementCounter()
    {
        if (count == 0)
            hMutex = CreateMutex(NULL, TRUE, NULL);
        count++;
    }

    void DecrementCounter()
    {
        count--;
        if (count == 0)
            ReleaseMutex(hMutex);
    }

    void WaitForCounterToReachZero()
    {
        WaitForSingleObject(hMutex, INFINITE);
        CloseHandle(hMutex);
    }
};
MultiThreadedCounter extractionsInProgressCounter;

It's definitely getting called in the right order. First, IncrementCounter() is called by the main thread before the async task (here, a thread sleep). Then the main thread calls WaitForCounterToReachZero(). Finally, the background thread calls DecrementCounter() when it has completed its work, which should allow the main thread to proceed.

However, WaitForSingleObject is not waiting. It returns immediately, with WAIT_OBJECT_0. Why is it doing that? It's almost like the mutex was never initially aquired. However, in the call to CreateMutex, I set bInitialOwner to TRUE, which is why I don't understand why it doesn't seem to have been aquired. I guess I have misunderstood something.

Thank you.

EDIT 1:

OK, so to test, I changed IncrementCounter() to:

void IncrementCounter()
{
    if (count == 0)
    {
        hMutex = CreateMutex(NULL, TRUE, NULL);
        DWORD var1 = WaitForSingleObject(hMutex, INFINITE);
        DWORD var2 = WaitForSingleObject(hMutex, INFINITE);
    }
    count++;
}

That really, really should have deadlocked it, but no, both calls to WaitForSingleObject returned immediately with var1 and var2 both equal to 0 (which according to the headers is WAIT_OBJECT_0).

The call to CreateMutex can't be working, can it? Yet hMutex gets set to a sensible value and GetLastError() remains at 0. So confused...

EDIT 2: Thank you all for your help. I never got this to work, however, I now realise that I was doing this the wrong way anyway. So I switched everything over to an Event, at which point it worked, then added a few conditionals to deal with out of order increments & decrements, then a critical section to protect the count variable. And it works :)

class MultiThreadedCounter
{
private:
int count; HANDLE hEvent; CRITICAL_SECTION criticalSection;

public:
void IncrementCounter()
{
    EnterCriticalSection(&criticalSection);
    if (count == 0)
        ResetEvent(hEvent);
    count++;
    LeaveCriticalSection(&criticalSection);
}

void DecrementCounter()
{
    EnterCriticalSection(&criticalSection);
    if (count > 0)
        count--;
    if (count == 0)
        SetEvent(hEvent);
    LeaveCriticalSection(&criticalSection);
}

void WaitForCounterToReachZero()
{
    WaitForSingleObject(hEvent, INFINITE);
}

MultiThreadedCounter()
{
    hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
    InitializeCriticalSection(&criticalSection);
    count = 0;
}

~MultiThreadedCounter()
{
    CloseHandle(hEvent);
    DeleteCriticalSection(&criticalSection);
}
};

Upvotes: 2

Views: 165

Answers (1)

simonc
simonc

Reputation: 42165

You don't show a constructor for MultiThreadedCounter. Without this, there is no place to initialise count to 0, meaning that the first call to IncrementCounter almost certainly won't call CreateMutex

Your constructor should look something like

MultiThreadedCounter()
    : count(0)
    , hMutex(NULL)
{
}

As an aside, if you need a lock that is used between threads in a single process, you could consider using a critical section instead.

Upvotes: 2

Related Questions