Modified: trunk/Source/WTF/ChangeLog (229514 => 229515)
--- trunk/Source/WTF/ChangeLog 2018-03-11 07:20:29 UTC (rev 229514)
+++ trunk/Source/WTF/ChangeLog 2018-03-11 08:03:30 UTC (rev 229515)
@@ -1,3 +1,35 @@
+2018-03-11 Yusuke Suzuki <utatane....@gmail.com>
+
+ [Win] Use SRWLOCK and CONDITION_VARIABLE to simplify implementation
+ https://bugs.webkit.org/show_bug.cgi?id=183541
+
+ Reviewed by Darin Adler.
+
+ After Windows Vista, Windows offers SRWLOCK and CONDITION_VARIABLE.
+ They can simplify the implementation of our WTF::Mutex and WTF::ThreadCondition.
+
+ C++ std::mutex and std::condition_variable uses std::chrono for their timed
+ functions. Since std::chrono is not overflow-aware, we cannot reliably use
+ this functionalities. This is why we still keep WTF::Mutex and WTF::ThreadCondition.
+ They are used for ParkingLot.
+
+ * wtf/ThreadingPrimitives.h:
+ * wtf/ThreadingWin.cpp:
+ (WTF::Mutex::Mutex):
+ (WTF::Mutex::~Mutex):
+ (WTF::Mutex::lock):
+ (WTF::Mutex::tryLock):
+ (WTF::Mutex::unlock):
+ (WTF::absoluteTimeToWaitTimeoutInterval):
+ (WTF::ThreadCondition::ThreadCondition):
+ (WTF::ThreadCondition::~ThreadCondition):
+ (WTF::ThreadCondition::wait):
+ (WTF::ThreadCondition::timedWait):
+ (WTF::ThreadCondition::signal):
+ (WTF::ThreadCondition::broadcast):
+ (WTF::PlatformCondition::timedWait): Deleted.
+ (WTF::PlatformCondition::signal): Deleted.
+
2018-03-10 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r229436.
Modified: trunk/Source/WTF/wtf/ThreadingPrimitives.h (229514 => 229515)
--- trunk/Source/WTF/wtf/ThreadingPrimitives.h 2018-03-11 07:20:29 UTC (rev 229514)
+++ trunk/Source/WTF/wtf/ThreadingPrimitives.h 2018-03-11 08:03:30 UTC (rev 229515)
@@ -55,28 +55,15 @@
#elif OS(WINDOWS)
using ThreadIdentifier = uint32_t;
using PlatformThreadHandle = HANDLE;
-struct PlatformMutex {
- CRITICAL_SECTION m_internalMutex;
- size_t m_recursionCount;
-};
-struct PlatformCondition {
- size_t m_waitersGone;
- size_t m_waitersBlocked;
- size_t m_waitersToUnblock;
- HANDLE m_blockLock;
- HANDLE m_blockQueue;
- HANDLE m_unblockLock;
-
- bool timedWait(PlatformMutex&, DWORD durationMilliseconds);
- void signal(bool unblockAll);
-};
+using PlatformMutex = SRWLOCK;
+using PlatformCondition = CONDITION_VARIABLE;
#else
-typedef void* PlatformMutex;
-typedef void* PlatformCondition;
+#error "Not supported platform"
#endif
-
+
class Mutex {
- WTF_MAKE_NONCOPYABLE(Mutex); WTF_MAKE_FAST_ALLOCATED;
+ WTF_MAKE_NONCOPYABLE(Mutex);
+ WTF_MAKE_FAST_ALLOCATED;
public:
WTF_EXPORT_PRIVATE Mutex();
WTF_EXPORT_PRIVATE ~Mutex();
@@ -85,8 +72,8 @@
WTF_EXPORT_PRIVATE bool tryLock();
WTF_EXPORT_PRIVATE void unlock();
-public:
PlatformMutex& impl() { return m_mutex; }
+
private:
PlatformMutex m_mutex;
};
@@ -95,6 +82,7 @@
class ThreadCondition {
WTF_MAKE_NONCOPYABLE(ThreadCondition);
+ WTF_MAKE_FAST_ALLOCATED;
public:
WTF_EXPORT_PRIVATE ThreadCondition();
WTF_EXPORT_PRIVATE ~ThreadCondition();
@@ -109,11 +97,6 @@
PlatformCondition m_condition;
};
-#if OS(WINDOWS)
-// Returns an interval in milliseconds suitable for passing to one of the Win32 wait functions (e.g., ::WaitForSingleObject).
-WTF_EXPORT_PRIVATE DWORD absoluteTimeToWaitTimeoutInterval(WallTime absoluteTime);
-#endif
-
} // namespace WTF
using WTF::Mutex;
@@ -120,8 +103,4 @@
using WTF::MutexLocker;
using WTF::ThreadCondition;
-#if OS(WINDOWS)
-using WTF::absoluteTimeToWaitTimeoutInterval;
-#endif
-
#endif // ThreadingPrimitives_h
Modified: trunk/Source/WTF/wtf/ThreadingWin.cpp (229514 => 229515)
--- trunk/Source/WTF/wtf/ThreadingWin.cpp 2018-03-11 07:20:29 UTC (rev 229514)
+++ trunk/Source/WTF/wtf/ThreadingWin.cpp 2018-03-11 08:03:30 UTC (rev 229515)
@@ -367,194 +367,62 @@
Mutex::Mutex()
{
- m_mutex.m_recursionCount = 0;
- InitializeCriticalSection(&m_mutex.m_internalMutex);
+ InitializeSRWLock(&m_mutex);
}
Mutex::~Mutex()
{
- DeleteCriticalSection(&m_mutex.m_internalMutex);
}
void Mutex::lock()
{
- EnterCriticalSection(&m_mutex.m_internalMutex);
- ++m_mutex.m_recursionCount;
+ AcquireSRWLockExclusive(&m_mutex);
}
-
-#pragma warning(suppress: 26115)
+
bool Mutex::tryLock()
{
- // This method is modeled after the behavior of pthread_mutex_trylock,
- // which will return an error if the lock is already owned by the
- // current thread. Since the primitive Win32 'TryEnterCriticalSection'
- // treats this as a successful case, it changes the behavior of several
- // tests in WebKit that check to see if the current thread already
- // owned this mutex (see e.g., IconDatabase::getOrCreateIconRecord)
- DWORD result = TryEnterCriticalSection(&m_mutex.m_internalMutex);
-
- if (result != 0) { // We got the lock
- // If this thread already had the lock, we must unlock and
- // return false so that we mimic the behavior of POSIX's
- // pthread_mutex_trylock:
- if (m_mutex.m_recursionCount > 0) {
- LeaveCriticalSection(&m_mutex.m_internalMutex);
- return false;
- }
-
- ++m_mutex.m_recursionCount;
- return true;
- }
-
- return false;
+ return TryAcquireSRWLockExclusive(&m_mutex);
}
void Mutex::unlock()
{
- ASSERT(m_mutex.m_recursionCount);
- --m_mutex.m_recursionCount;
- LeaveCriticalSection(&m_mutex.m_internalMutex);
+ ReleaseSRWLockExclusive(&m_mutex);
}
-bool PlatformCondition::timedWait(PlatformMutex& mutex, DWORD durationMilliseconds)
+// Returns an interval in milliseconds suitable for passing to one of the Win32 wait functions (e.g., ::WaitForSingleObject).
+static DWORD absoluteTimeToWaitTimeoutInterval(WallTime absoluteTime)
{
- // Enter the wait state.
- DWORD res = WaitForSingleObject(m_blockLock, INFINITE);
- ASSERT_UNUSED(res, res == WAIT_OBJECT_0);
- ++m_waitersBlocked;
- res = ReleaseSemaphore(m_blockLock, 1, 0);
- ASSERT_UNUSED(res, res);
+ WallTime currentTime = WallTime::now();
- --mutex.m_recursionCount;
- LeaveCriticalSection(&mutex.m_internalMutex);
+ // Time is in the past - return immediately.
+ if (absoluteTime < currentTime)
+ return 0;
- // Main wait - use timeout.
- bool timedOut = (WaitForSingleObject(m_blockQueue, durationMilliseconds) == WAIT_TIMEOUT);
+ // Time is too far in the future (and would overflow unsigned long) - wait forever.
+ if ((absoluteTime - currentTime) > Seconds::fromMilliseconds(INT_MAX))
+ return INFINITE;
- res = WaitForSingleObject(m_unblockLock, INFINITE);
- ASSERT_UNUSED(res, res == WAIT_OBJECT_0);
-
- int signalsLeft = m_waitersToUnblock;
-
- if (m_waitersToUnblock)
- --m_waitersToUnblock;
- else if (++m_waitersGone == (INT_MAX / 2)) { // timeout/canceled or spurious semaphore
- // timeout or spurious wakeup occured, normalize the m_waitersGone count
- // this may occur if many calls to wait with a timeout are made and
- // no call to notify_* is made
- res = WaitForSingleObject(m_blockLock, INFINITE);
- ASSERT_UNUSED(res, res == WAIT_OBJECT_0);
- m_waitersBlocked -= m_waitersGone;
- res = ReleaseSemaphore(m_blockLock, 1, 0);
- ASSERT_UNUSED(res, res);
- m_waitersGone = 0;
- }
-
- res = ReleaseMutex(m_unblockLock);
- ASSERT_UNUSED(res, res);
-
- if (signalsLeft == 1) {
- res = ReleaseSemaphore(m_blockLock, 1, 0); // Open the gate.
- ASSERT_UNUSED(res, res);
- }
-
- EnterCriticalSection (&mutex.m_internalMutex);
- ++mutex.m_recursionCount;
-
- return !timedOut;
+ return static_cast<DWORD>((absoluteTime - currentTime).milliseconds());
}
-void PlatformCondition::signal(bool unblockAll)
-{
- unsigned signalsToIssue = 0;
-
- DWORD res = WaitForSingleObject(m_unblockLock, INFINITE);
- ASSERT_UNUSED(res, res == WAIT_OBJECT_0);
-
- if (m_waitersToUnblock) { // the gate is already closed
- if (!m_waitersBlocked) { // no-op
- res = ReleaseMutex(m_unblockLock);
- ASSERT_UNUSED(res, res);
- return;
- }
-
- if (unblockAll) {
- signalsToIssue = m_waitersBlocked;
- m_waitersToUnblock += m_waitersBlocked;
- m_waitersBlocked = 0;
- } else {
- signalsToIssue = 1;
- ++m_waitersToUnblock;
- --m_waitersBlocked;
- }
- } else if (m_waitersBlocked > m_waitersGone) {
- res = WaitForSingleObject(m_blockLock, INFINITE); // Close the gate.
- ASSERT_UNUSED(res, res == WAIT_OBJECT_0);
- if (m_waitersGone != 0) {
- m_waitersBlocked -= m_waitersGone;
- m_waitersGone = 0;
- }
- if (unblockAll) {
- signalsToIssue = m_waitersBlocked;
- m_waitersToUnblock = m_waitersBlocked;
- m_waitersBlocked = 0;
- } else {
- signalsToIssue = 1;
- m_waitersToUnblock = 1;
- --m_waitersBlocked;
- }
- } else { // No-op.
- res = ReleaseMutex(m_unblockLock);
- ASSERT_UNUSED(res, res);
- return;
- }
-
- res = ReleaseMutex(m_unblockLock);
- ASSERT_UNUSED(res, res);
-
- if (signalsToIssue) {
- res = ReleaseSemaphore(m_blockQueue, signalsToIssue, 0);
- ASSERT_UNUSED(res, res);
- }
-}
-
-static const long MaxSemaphoreCount = static_cast<long>(~0UL >> 1);
-
ThreadCondition::ThreadCondition()
{
- m_condition.m_waitersGone = 0;
- m_condition.m_waitersBlocked = 0;
- m_condition.m_waitersToUnblock = 0;
- m_condition.m_blockLock = CreateSemaphore(0, 1, 1, 0);
- m_condition.m_blockQueue = CreateSemaphore(0, 0, MaxSemaphoreCount, 0);
- m_condition.m_unblockLock = CreateMutex(0, 0, 0);
-
- if (!m_condition.m_blockLock || !m_condition.m_blockQueue || !m_condition.m_unblockLock) {
- if (m_condition.m_blockLock)
- CloseHandle(m_condition.m_blockLock);
- if (m_condition.m_blockQueue)
- CloseHandle(m_condition.m_blockQueue);
- if (m_condition.m_unblockLock)
- CloseHandle(m_condition.m_unblockLock);
- }
+ InitializeConditionVariable(&m_condition);
}
ThreadCondition::~ThreadCondition()
{
- CloseHandle(m_condition.m_blockLock);
- CloseHandle(m_condition.m_blockQueue);
- CloseHandle(m_condition.m_unblockLock);
}
void ThreadCondition::wait(Mutex& mutex)
{
- m_condition.timedWait(mutex.impl(), INFINITE);
+ SleepConditionVariableSRW(&m_condition, &mutex.impl(), INFINITE, 0);
}
bool ThreadCondition::timedWait(Mutex& mutex, WallTime absoluteTime)
{
+ // https://msdn.microsoft.com/en-us/library/windows/desktop/ms686304(v=vs.85).aspx
DWORD interval = absoluteTimeToWaitTimeoutInterval(absoluteTime);
-
if (!interval) {
// Consider the wait to have timed out, even if our condition has already been signaled, to
// match the pthreads implementation.
@@ -561,34 +429,22 @@
return false;
}
- return m_condition.timedWait(mutex.impl(), interval);
+ if (SleepConditionVariableSRW(&m_condition, &mutex.impl(), interval, 0))
+ return true;
+ ASSERT(GetLastError() == ERROR_TIMEOUT);
+ return false;
}
void ThreadCondition::signal()
{
- m_condition.signal(false); // Unblock only 1 thread.
+ WakeConditionVariable(&m_condition);
}
void ThreadCondition::broadcast()
{
- m_condition.signal(true); // Unblock all threads.
+ WakeAllConditionVariable(&m_condition);
}
-DWORD absoluteTimeToWaitTimeoutInterval(WallTime absoluteTime)
-{
- WallTime currentTime = WallTime::now();
-
- // Time is in the past - return immediately.
- if (absoluteTime < currentTime)
- return 0;
-
- // Time is too far in the future (and would overflow unsigned long) - wait forever.
- if ((absoluteTime - currentTime) > Seconds::fromMilliseconds(INT_MAX))
- return INFINITE;
-
- return static_cast<DWORD>((absoluteTime - currentTime).milliseconds());
-}
-
// Remove this workaround code when <rdar://problem/31793213> is fixed.
ThreadIdentifier createThread(ThreadFunction function, void* data, const char* threadName)
{
