Attachment 362007 Details for Bug 194014 – Patch

[patch] Patch

bug-194014-20190214114557.patch (text/plain), 4.25 KB, created by Dominik Inführ on 2019-02-14 02:45:58 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Dominik Inführ

Created: 2019-02-14 02:45:58 PST

Size: 4.25 KB

patch

obsolete

>Subversion Revision: 241515
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index faaa1a8fac2e2be357872c396bc99b352f7795a4..0ccb529693756e72847c39cfe627b63cbd700ea4 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,38 @@
>+2019-02-14  Dominik Infuehr  <dinfuehr@igalia.com>
>+
>+        Fix deadlock on Linux/x64 between SamplingProfiler and VMTraps
>+        https://bugs.webkit.org/show_bug.cgi?id=194014
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Do not block SIGUSR1 when installing signal handlers, since this signal
>+        is used to suspend/resume machine threads on Linux.
>+
>+        ftl-ai-filter-phantoms-should-clear-clear-value.js deadlocked with
>+        enabled watchdog and sampling.
>+
>+        Deadlock happened in the following situation:
>+
>+        Thread 1 (Sampling): SamplingProfiler.cpp:takeSample takes all needed locks
>+        and then tries to suspend the main thread.
>+
>+        Thread 2 (Watchdog/VMTraps): Before the Sampling-Thread suspends the main thread
>+        a signal is caught and the signal handler is invoked (VMTraps.cpp:SignalSender).
>+        SignalSender tries to lock codeBlockSet, but this is already locked by the
>+        SamplingProfiler.
>+
>+        The SamplingProfiler can only give up the lock when it suspends
>+        the thread. However since the VMTraps signal handler is active, all other signals blocked,
>+        therefore the SamplingProfiler also waits until its signal handler is invoked.
>+
>+        This patch fixes this by not blocking SIGUSR1 in installSignalHandler, since
>+        it is used to suspend/resume threads on Linux.
>+
>+        * wtf/Threading.h:
>+        * wtf/posix/ThreadingPOSIX.cpp:
>+        * wtf/threads/Signals.cpp:
>+        (WTF::installSignalHandler):
>+
> 2019-02-13  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         We should only make rope strings when concatenating strings long enough.
>diff --git a/Source/WTF/wtf/Threading.h b/Source/WTF/wtf/Threading.h
>index 3a8a4a55d339a0fe49fcca6cfe939becccd11f84..2d9242ccab4bbd5fd098f900574b7e5220f72b09 100644
>--- a/Source/WTF/wtf/Threading.h
>+++ b/Source/WTF/wtf/Threading.h
>@@ -64,6 +64,13 @@ class PrintStream;
> // This function can be called from any threads.
> WTF_EXPORT_PRIVATE void initializeThreading();
> 
>+#if USE(PTHREADS)
>+
>+// We use SIGUSR1 to suspend and resume machine threads in JavaScriptCore.
>+constexpr const int SigThreadSuspendResume = SIGUSR1;
>+
>+#endif
>+
> // FIXME: The following functions remain because they are used from WebKit Windows support library,
> // WebKitQuartzCoreAdditions.dll. When updating the support library, we should use new API instead
> // and the following workaound should be removed. And new code should not use the following APIs.
>diff --git a/Source/WTF/wtf/posix/ThreadingPOSIX.cpp b/Source/WTF/wtf/posix/ThreadingPOSIX.cpp
>index e48dd9b57edd4cf55f25331645823f3109da4221..64cd1f718d55f6ea5ba9d6c346d073885b32f567 100644
>--- a/Source/WTF/wtf/posix/ThreadingPOSIX.cpp
>+++ b/Source/WTF/wtf/posix/ThreadingPOSIX.cpp
>@@ -105,8 +105,6 @@ private:
> };
> static LazyNeverDestroyed<Semaphore> globalSemaphoreForSuspendResume;
> 
>-// We use SIGUSR1 to suspend and resume machine threads in JavaScriptCore.
>-static constexpr const int SigThreadSuspendResume = SIGUSR1;
> static std::atomic<Thread*> targetThread { nullptr };
> 
> void Thread::signalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
>diff --git a/Source/WTF/wtf/threads/Signals.cpp b/Source/WTF/wtf/threads/Signals.cpp
>index 5e7eb6acc3b870f97f078415610943a870839791..fcd0f1d68faece8340c79b4e6c5db071b0a11234 100644
>--- a/Source/WTF/wtf/threads/Signals.cpp
>+++ b/Source/WTF/wtf/threads/Signals.cpp
>@@ -258,6 +258,9 @@ void installSignalHandler(Signal signal, SignalHandler&& handler)
>             action.sa_sigaction = jscSignalHandler;
>             auto result = sigfillset(&action.sa_mask);
>             RELEASE_ASSERT(!result);
>+            // Do not block this signal since it is used on non-Darwin systems to suspend and resume threads.
>+            result = sigdelset(&action.sa_mask, SigThreadSuspendResume);
>+            RELEASE_ASSERT(!result);
>             action.sa_flags = SA_SIGINFO;
>             auto systemSignals = toSystemSignal(signal);
>             result = sigaction(std::get<0>(systemSignals), &action, &oldActions[offsetForSystemSignal(std::get<0>(systemSignals))]);

Actions: View | Formatted Diff | Diff

Attachments on bug 194014: 360565 | 360566 | 360568 | 362007