[WTF] Use m_suspendCount instead of m_suspended flag in Thread
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 17:28:14 +0000 (17:28 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 17:28:14 +0000 (17:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180427

Reviewed by Carlos Garcia Campos.

When resuming the thread, signal handler is reinvoked once before `sigsuspend` is resumed.
But this handler should not do anything since it is just a signal for `sigsuspend`.
Previously, we use m_suspenedd flag to distinguish between suspending and resuming signal
handler invocations.

But this additional m_suspended flag is not necessary since we can use m_suspendCount instead.
This patch drops m_suspended and use m_suspendCount. Since semaphore operations emit full memory
barrier, m_suspendCount is loaded and stored as we expect.

* wtf/Threading.h:
* wtf/ThreadingPthreads.cpp:
(WTF::Thread::signalHandlerSuspendResume):
(WTF::Thread::suspend):
(WTF::Thread::resume):

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

Source/WTF/ChangeLog
Source/WTF/wtf/Threading.h
Source/WTF/wtf/ThreadingPthreads.cpp

index cef7ce0..c406bd7 100644 (file)
@@ -1,3 +1,25 @@
+2017-12-05  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Use m_suspendCount instead of m_suspended flag in Thread
+        https://bugs.webkit.org/show_bug.cgi?id=180427
+
+        Reviewed by Carlos Garcia Campos.
+
+        When resuming the thread, signal handler is reinvoked once before `sigsuspend` is resumed.
+        But this handler should not do anything since it is just a signal for `sigsuspend`.
+        Previously, we use m_suspenedd flag to distinguish between suspending and resuming signal
+        handler invocations.
+
+        But this additional m_suspended flag is not necessary since we can use m_suspendCount instead.
+        This patch drops m_suspended and use m_suspendCount. Since semaphore operations emit full memory
+        barrier, m_suspendCount is loaded and stored as we expect.
+
+        * wtf/Threading.h:
+        * wtf/ThreadingPthreads.cpp:
+        (WTF::Thread::signalHandlerSuspendResume):
+        (WTF::Thread::suspend):
+        (WTF::Thread::resume):
+
 2017-12-04  Simon Fraser  <simon.fraser@apple.com>
 
         Minor DisplayRefreshMonitor-related cleanup
index c74394b..af6ca02 100644 (file)
@@ -288,7 +288,6 @@ protected:
 #elif USE(PTHREADS)
     PlatformRegisters* m_platformRegisters { nullptr };
     unsigned m_suspendCount { 0 };
-    std::atomic<bool> m_suspended { false };
 #endif
 
     AtomicStringTable* m_currentAtomicStringTable { nullptr };
index cfc62aa..b2ca77d 100644 (file)
@@ -144,10 +144,10 @@ static UNUSED_FUNCTION bool isOnAlternativeSignalStack()
 
 void Thread::signalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
 {
-    // Touching thread local atomic types from signal handlers is allowed.
+    // Touching a global variable atomic types from signal handlers is allowed.
     Thread* thread = targetThread.load();
 
-    if (thread->m_suspended.load(std::memory_order_acquire)) {
+    if (thread->m_suspendCount) {
         // This is signal handler invocation that is intended to be used to resume sigsuspend.
         // So this handler invocation itself should not process.
         //
@@ -350,14 +350,12 @@ auto Thread::suspend() -> Expected<void, PlatformSuspendError>
     if (!m_suspendCount) {
         // Ideally, we would like to use pthread_sigqueue. It allows us to pass the argument to the signal handler.
         // But it can be used in a few platforms, like Linux.
-        // Instead, we use Thread* stored in the thread local storage to pass it to the signal handler.
+        // Instead, we use Thread* stored in a global variable to pass it to the signal handler.
         targetThread.store(this);
         int result = pthread_kill(m_handle, SigThreadSuspendResume);
         if (result)
             return makeUnexpected(result);
         globalSemaphoreForSuspendResume->wait();
-        // Release barrier ensures that this operation is always executed after all the above processing is done.
-        m_suspended.store(true, std::memory_order_release);
     }
     ++m_suspendCount;
     return { };
@@ -377,14 +375,12 @@ void Thread::resume()
         // There are several ways to distinguish the handler invocation for suspend and resume.
         // 1. Use different signal numbers. And check the signal number in the handler.
         // 2. Use some arguments to distinguish suspend and resume in the handler. If pthread_sigqueue can be used, we can take this.
-        // 3. Use thread local storage with atomic variables in the signal handler.
-        // In this implementaiton, we take (3). suspended flag is used to distinguish it.
+        // 3. Use thread's flag.
+        // In this implementaiton, we take (3). m_suspendCount is used to distinguish it.
         targetThread.store(this);
         if (pthread_kill(m_handle, SigThreadSuspendResume) == ESRCH)
             return;
         globalSemaphoreForSuspendResume->wait();
-        // Release barrier ensures that this operation is always executed after all the above processing is done.
-        m_suspended.store(false, std::memory_order_release);
     }
     --m_suspendCount;
 #endif