[Win] Use SRWLOCK and CONDITION_VARIABLE to simplify implementation
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Mar 2018 08:03:30 +0000 (08:03 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Mar 2018 08:03:30 +0000 (08:03 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229515 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/ThreadingPrimitives.h
Source/WTF/wtf/ThreadingWin.cpp

index 0e40c6e..a2b7cc8 100644 (file)
@@ -1,3 +1,35 @@
+2018-03-11  Yusuke Suzuki  <utatane.tea@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-queue@webkit.org>
 
         Unreviewed, rolling out r229436.
index 28f479b..fdf607f 100644 (file)
@@ -55,28 +55,15 @@ using PlatformCondition = pthread_cond_t;
 #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 @@ public:
     WTF_EXPORT_PRIVATE bool tryLock();
     WTF_EXPORT_PRIVATE void unlock();
 
-public:
     PlatformMutex& impl() { return m_mutex; }
+
 private:
     PlatformMutex m_mutex;
 };
@@ -95,6 +82,7 @@ typedef Locker<Mutex> MutexLocker;
 
 class ThreadCondition {
     WTF_MAKE_NONCOPYABLE(ThreadCondition);
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     WTF_EXPORT_PRIVATE ThreadCondition();
     WTF_EXPORT_PRIVATE ~ThreadCondition();
@@ -109,19 +97,10 @@ private:
     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;
 using WTF::MutexLocker;
 using WTF::ThreadCondition;
 
-#if OS(WINDOWS)
-using WTF::absoluteTimeToWaitTimeoutInterval;
-#endif
-
 #endif // ThreadingPrimitives_h
index 6eddfd6..9799fc1 100644 (file)
@@ -367,226 +367,82 @@ void Thread::destructTLS(void* data)
 
 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);
-
-    --mutex.m_recursionCount;
-    LeaveCriticalSection(&mutex.m_internalMutex);
-
-    // Main wait - use timeout.
-    bool timedOut = (WaitForSingleObject(m_blockQueue, durationMilliseconds) == WAIT_TIMEOUT);
-
-    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;
-}
-
-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;
-        }
+    WallTime currentTime = WallTime::now();
 
-        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;
-    }
+    // Time is in the past - return immediately.
+    if (absoluteTime < currentTime)
+        return 0;
 
-    res = ReleaseMutex(m_unblockLock);
-    ASSERT_UNUSED(res, res);
+    // Time is too far in the future (and would overflow unsigned long) - wait forever.
+    if ((absoluteTime - currentTime) > Seconds::fromMilliseconds(INT_MAX))
+        return INFINITE;
 
-    if (signalsToIssue) {
-        res = ReleaseSemaphore(m_blockQueue, signalsToIssue, 0);
-        ASSERT_UNUSED(res, res);
-    }
+    return static_cast<DWORD>((absoluteTime - currentTime).milliseconds());
 }
 
-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.
         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.
-}
-
-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());
+    WakeAllConditionVariable(&m_condition);
 }
 
 // Remove this workaround code when <rdar://problem/31793213> is fixed.