std::mutex lock hangs when overriding the new operator

Question

We have an internal memory manager that we use with one of our products. The memory manager overrides the new and delete operators, and works fine in single-threaded appliations. However, I'm now tasked to make it work with multi-threaded applications too. To my understanding the following pseudo code should work, but it hangs on a spin, even with try_lock(). Any ideas?

Update #1

Causes "Access Violation":

#include <mutex>

std::mutex g_mutex;

/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   g_mutex.lock(); // Access violation exception
   ...   
}

Causes thread to hang forever in a spin:

#include <mutex>

std::mutex g_mutex;
bool g_systemInitiated = false;


/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   if (g_systemInitiated == false) return malloc(size);
   g_mutex.lock(); // Thread hangs forever here. g_mutex.try_lock() also hangs
   ...   
}

int main(int argc, const char* argv[])
{
   // Tell the new() operator that the system has initiated
   g_systemInitiated = true;
   ...
}

Update #2

A recursive mutex also causes the thread to hang forever in a spin:

#include <mutex>

std::recursive_mutex g_mutex;
bool g_systemInitiated = false;


/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   if (g_systemInitiated == false) return malloc(size);
   g_mutex.lock(); // Thread hangs forever here. g_mutex.try_lock() also hangs
   ...   
}

int main(int argc, const char* argv[])
{
   // Tell the new() operator that the system has initiated
   g_systemInitiated = true;
   ...
}

Update #3

Jonathan Wakely suggested I should try unique_lock and/or lock_guard, but the lock still hangs in a spin.

unique_lock test:

#include <mutex>

std::mutex g_mutex;
std::unique_lock<std::mutex> g_lock1(g_mutex, std::defer_lock);
bool g_systemInitiated = false;

/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   if (g_systemInitiated == false) return malloc(size);
   g_lock1.lock(); // Thread hangs forever here the first time it is called
   ...   
}

int main(int argc, const char* argv[])
{
   // Tell the new() operator that the system has initiated
   g_systemInitiated = true;
   ...
}

lock_guard test:

#include <mutex>

std::recursive_mutex g_mutex;
bool g_systemInitiated = false;


/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   if (g_systemInitiated == false) return malloc(size);
   std::lock_guard<std::mutex> g_lock_guard1(g_mutex); // Thread hangs forever here the first time it is called
   ...   
}

int main(int argc, const char* argv[])
{
   // Tell the new() operator that the system has initiated
   g_systemInitiated = true;
   ...
}

I think my problem is that delete is called by the C++ 11 mutex library when locking. delete is also overridden like so:

/*!
\brief Overrides the Standard C++ new operator
\param p [in] The pointer to memory to free
*/
void operator delete(void *p)
{
    if (g_systemInitiated == false)
    {
       free(p); 
    }
    else
    {
       std::lock_guard<std::mutex> g_lock_guard1(g_mutex);
       ...
    }
}

This causes deadlocking situation that I can't see any good solution to except for making my own locking that does not spawn any calls to new or delete while locking or unlocking.

Update #4

I've implemented my own custom recursive mutex that have no calls to new or delete, also, it allows the same thread to enter a locked block.

#include <thread>

std::thread::id g_lockedByThread;
bool g_isLocked = false;
bool g_systemInitiated = false;

/*!
\brief Overrides the Standard C++ new operator
\param size [in] Number of bytes to allocate
*/
void *operator new(size_t size)
{
   if (g_systemInitiated == false) return malloc(size);

   while (g_isLocked && g_lockedByThread != std::this_thread::get_id());
   g_isLocked = true; // Atomic operation
   g_lockedByThread = std::this_thread::get_id();
   ...   
   g_isLocked = false;
}

/*!
\brief Overrides the Standard C++ new operator
\param p [in] The pointer to memory to free
*/
void operator delete(void *p)
{
    if (g_systemInitiated == false)
    {
       free(p); 
    }
    else
    {
       while (g_isLocked && g_lockedByThread != std::this_thread::get_id());
       g_isLocked = true; // Atomic operation
       g_lockedByThread = std::this_thread::get_id();
       ...   
       g_isLocked = false;
    }
}

int main(int argc, const char* argv[])
{
   // Tell the new() operator that the system has initiated
   g_systemInitiated = true;
   ...
}

Update #5

Tried Jonathan Wakely suggestion, and found that it definitely seems that there is something wrong with Microsoft's implementation of C++ 11 Mutexes; his example hangs if compiled with the /MTd (Multi-threaded Debug) compiler flag, but works fine if compiled with the /MDd (Multi-threaded Debug DLL) compiler flag. As Jonathan rightly pointed out std::mutex implementations are supposed to be constexpr's. Here is the VS 2012 C++ code I used to test the implementation issue:

#include "stdafx.h"

#include <mutex>
#include <iostream>

bool g_systemInitiated = false;
std::mutex g_mutex;

void *operator new(size_t size)
{
    if (g_systemInitiated == false) return malloc(size);
    std::lock_guard<std::mutex> lock(g_mutex);
    std::cout << "Inside new() critical section" << std::endl;
    // <-- Memory manager would be called here, dummy call to malloc() in stead
    return malloc(size);
}

void operator delete(void *p)
{
    if (g_systemInitiated == false) free(p); 
    else
    {
        std::lock_guard<std::mutex> lock(g_mutex);
        std::cout << "Inside delete() critical section" << std::endl;
        // <-- Memory manager would be called here, dummy call to free() in stead
        free(p);
    }
}

int _tmain(int argc, _TCHAR* argv[])
{
    g_systemInitiated = true;

    char *test = new char[100];
    std::cout << "Allocated" << std::endl;
    delete test;
    std::cout << "Deleted" << std::endl;

    return 0;
}

Update #6

Submitted a bug report to Microsoft: https://connect.microsoft.com/VisualStudio/feedback/details/776596/std-mutex-not-a-constexpr-with-mtd-compiler-flag#details

Answer 1

The mutex library uses new, and std::mutex is not recursive (i.e. reentrant) by default. A chicken-and-egg problem.

UPDATE As has been pointed out in the comments below, using std::recursive_mutex may work. But the classic C++ problem of the order of static initialization of globals being not well defined remains, as does the danger of outside access to the global mutex (best to put it inside an anonymous namespace.)

UPDATE 2 You may be switching g_systemInitiated to true too early, i.e. before the mutex has had a chance to complete its initialization, and so the "first-time-through" call to malloc() never happens. To force this, you could try replacing the assignment in main() with a call to an initialization function in the allocator module:

namespace {
    std::recursive_mutex    g_mutex;
    bool                    g_initialized = false;
}
void initialize()
{
    g_mutex.lock();
    g_initialized = true;
    g_mutex.unlock();
}

Answer 2

All of your examples are broken, except the first one, which is just very bad practice for not using a scoped lock type.

This should work, if it doesn't your compiler or standard library is broken:

#include <mutex>

std::mutex g_mutex;

void *operator new(size_t size)
{
   std::lock_guard<std::mutex> lock(g_mutex);
   ...   
}