Bug 26565
| Summary: | Windows version of Mutex.lock() should ASSERT on re-entrance | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Jeremy Orlow <jorlow> |
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | ap, mjs |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
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
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
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.)