Bug 26565 – Windows version of Mutex.lock() should ASSERT on re-entrance

• Status: NEW
• Component: New Bugs

[Jeremy Orlow]

Windows version of Mutex.lock() should ASSERT on re-entrance.  The pthread code does not use the flag that allows re-entrance, so we should protect against Windows developers writing code that depends on re-entrance.  An ASSERT seems like the right solution since doing so it not fatal, it's just something we want people to avoid.

I'll send out a patch soon.

Webkit-dev discussion on the topic:
http://lists.macosforge.org/pipermail/webkit-dev/2009-June/008338.html

[Jeremy Orlow]

I looked at the code, and it seems the only change necessary is adding the following ASSERT in JavaScriptCore/wtf/threadingwin.cpp's Mutex::lock() function:

void Mutex::lock()
{
    EnterCriticalSection(&m_mutex.m_internalMutex);
    ++m_mutex.m_recursionCount;
    ASSERT(m_mutex.m_recursionCount == 1);
}

Mutex::tryLock already has the correct behavior.

Unfortunately, it seems that safari.exe relies on the recursive behavior.  Obviously I have no way to fix these dependencies, so it seems that this is a non-starter (at least for someone outside of Apple).  I still think it's a good idea though.

If Safari's Windows code cannot possibly avoid its re-entrance assumption, maybe we could create a sub-class of Mutex called ReentrantMutex.  Only the constructor and lock would need to be overridden.  (No need for anything to be virtual...which would obviously not be practical anyway.)