[macOS] Multiple third party apps crash due to the thread safety check in TimerBase...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Aug 2018 02:01:24 +0000 (02:01 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Aug 2018 02:01:24 +0000 (02:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188480

Reviewed by Simon Fraser.

Source/WebCore:

Suppress the release assert in WebKit1 on macOS (isInWebProcess is always true in non-Cocoa platforms).

In the future, we should consider throwing Objective-C exceptions when third party apps call WebKit1
or WebKit2 APIs in non-main threads.

* platform/Timer.cpp:
(WebCore::shouldSuppressThreadSafetyCheck): Extracted out of ~TimerBase and setNextFireTime.
(WebCore::TimerBase::~TimerBase):
(WebCore::TimerBase::setNextFireTime):

Source/WTF:

Added the SDK version for macOS Mojave.

* wtf/spi/darwin/dyldSPI.h:

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

Source/WTF/ChangeLog
Source/WTF/wtf/spi/darwin/dyldSPI.h
Source/WebCore/ChangeLog
Source/WebCore/platform/Timer.cpp

index 0730aa7..508d804 100644 (file)
@@ -1,3 +1,14 @@
+2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
+        https://bugs.webkit.org/show_bug.cgi?id=188480
+
+        Reviewed by Simon Fraser.
+
+        Added the SDK version for macOS Mojave.
+
+        * wtf/spi/darwin/dyldSPI.h:
+
 2018-08-10  Antti Koivisto  <antti@apple.com>
 
         Use OptionSet for various RenderLayer flags
index 4506098..805d65a 100644 (file)
 #define DYLD_MACOSX_VERSION_10_13 0x000A0D00
 #endif
 
+#ifndef DYLD_MACOSX_VERSION_10_14
+#define DYLD_MACOSX_VERSION_10_14 0x000A0E00
+#endif
+
 #else
 
 #define DYLD_IOS_VERSION_3_0 0x00030000
@@ -61,6 +65,7 @@
 #define DYLD_MACOSX_VERSION_10_11 0x000A0B00
 #define DYLD_MACOSX_VERSION_10_12 0x000A0C00
 #define DYLD_MACOSX_VERSION_10_13 0x000A0D00
+#define DYLD_MACOSX_VERSION_10_14 0x000A0E00
 
 #endif
 
index fb3b1d3..60339b7 100644 (file)
@@ -1,3 +1,20 @@
+2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
+        https://bugs.webkit.org/show_bug.cgi?id=188480
+
+        Reviewed by Simon Fraser.
+
+        Suppress the release assert in WebKit1 on macOS (isInWebProcess is always true in non-Cocoa platforms).
+
+        In the future, we should consider throwing Objective-C exceptions when third party apps call WebKit1
+        or WebKit2 APIs in non-main threads.
+
+        * platform/Timer.cpp:
+        (WebCore::shouldSuppressThreadSafetyCheck): Extracted out of ~TimerBase and setNextFireTime.
+        (WebCore::TimerBase::~TimerBase):
+        (WebCore::TimerBase::setNextFireTime):
+
 2018-08-10  Daniel Bates  <dabates@apple.com>
 
         Cleanup: Remove unnecessary code to resume animations from CachedFrameBase::restore()
index 722d453..239e1c9 100644 (file)
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "Timer.h"
 
+#include "RuntimeApplicationChecks.h"
 #include "SharedTimer.h"
 #include "ThreadGlobalData.h"
 #include "ThreadTimers.h"
 #include <wtf/MainThread.h>
 #include <wtf/Vector.h>
 
+#if USE(WEB_THREAD) || PLATFORM(MAC)
+#include <wtf/spi/darwin/dyldSPI.h>
+#endif
+
 namespace WebCore {
 
 class TimerHeapReference;
@@ -184,6 +189,17 @@ inline bool TimerHeapLessThanFunction::operator()(const TimerBase* a, const Time
 
 // ----------------
 
+static bool shouldSuppressThreadSafetyCheck()
+{
+#if USE(WEB_THREAD)
+    return WebThreadIsEnabled();
+#elif PLATFORM(MAC)
+    return !isInWebProcess() && applicationSDKVersion() < DYLD_MACOSX_VERSION_10_14;
+#else
+    return false;
+#endif
+}
+
 TimerBase::TimerBase()
     : m_heapIndex(-1)
     , m_wasDeleted(false)
@@ -192,12 +208,8 @@ TimerBase::TimerBase()
 
 TimerBase::~TimerBase()
 {
-#if USE(WEB_THREAD)
     ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
-    RELEASE_ASSERT(WebThreadIsEnabled() || canAccessThreadLocalDataForThread(m_thread.get()));
-#else
-    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
-#endif
+    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     stop();
     ASSERT(!inHeap());
     m_wasDeleted = true;
@@ -364,12 +376,8 @@ void TimerBase::updateHeapIfNeeded(MonotonicTime oldTime)
 
 void TimerBase::setNextFireTime(MonotonicTime newTime)
 {
-#if USE(WEB_THREAD)
     ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
-    RELEASE_ASSERT(WebThreadIsEnabled() || canAccessThreadLocalDataForThread(m_thread.get()));
-#else
-    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
-#endif
+    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_wasDeleted);
 
     if (m_unalignedNextFireTime != newTime)