What is wrong with this tiny piece of mutex code?

Question

// A Mutex allows threads mutually exclusive access to a resource.
//-----------------------------------------------------------------------

class Mutex
{
private:
    CRITICAL_SECTION m_mutex;

public:
     Mutex() { InitializeCriticalSection(&m_mutex); }
    ~Mutex() { DeleteCriticalSection(&m_mutex);     }

    void acquire() { EnterCriticalSection(&m_mutex); }
    void release() { LeaveCriticalSection(&m_mutex); }
};

Using the Entrek Codesnitch software to debug and test for any memory leaks, etc., it reports the following error:

InitializeCriticalSection Error: lpCriticalSection (0x000387d4) points to an invalid 
  memory location (0x00018984) Mutex::Mutex in lockmutex.h, line 29

Maybe all my sleepless nights are finally getting to me. But I don't understand what it's exactly complaining about. Any ideas?

Answer 1

I don't see anything wrong with your class definition. Where is it being used though? A particular instance could still be used incorrectly.

The Win32 definition of a CRITICAL_SECTION includes a pointer to a 'struct _RTL_CRITICAL_SECTION *'. The OS could be being clever in its handling of this struct in ways that confuse the tool.

Answer 2

CodeSnitch is apparently not smart enough to know that InitializeCriticalSection() expects to be working on a structure containing an uninitialized pointer.

Think of it from CodeSnitch's point of view. What's the difference between what you're doing, and this:

struct Customer {
    char * name;
};

extern void greetCustomer(Customer* c);

class CheckoutLine {
  private:
    Customer m_customer;
  public CheckoutLine() {
    greetCustomer(&m_customer);
  }
};

This looks way more fishy to the human eye, because we infer that greetCustomer is probably going to rely on m_customer being initialized, which it obviously is not. But semantically, this exactly the same as your code.

It's probably worth filing a bug with Entrek; InitializeCriticalSection() is a reasonable exception to the "structures should be initialized before passing them to a function" rule.

Answer 3

I'll bet you can fake out the snitch with ::memset ( & m_mutex, 0, sizeof ( m_mutex ) ); before the call to init it.