Title: [231319] trunk/Source/WebCore
Revision
231319
Author
cdu...@apple.com
Date
2018-05-03 12:17:46 -0700 (Thu, 03 May 2018)

Log Message

REGRESSION(iOS 11.3): Crashes in TimerBase::~TimerBase() in Tencent x5gamehelper
https://bugs.webkit.org/show_bug.cgi?id=185073
<rdar://problem/39821223>

Reviewed by Alexey Proskuryakov.

The following changes were made:
- Make sure SocketStream callbacks are always scheduled on the right runloop:
  WebThreadRunLoop() on WebKitLegacy iOS, loaderRunLoop() on Windows and
  main runloop otherwise.
- When the SocketStream callbacks are called, unconditionally call callOnMainThreadAndWait()
  before calling methods on the SocketStream client. Previously, this code path
  was specific to Windows but there is no reason to have platform-specific code here.
  callOnMainThreadAndWait() calls the function right away if we're already on the main
  thread, which will be the case on other platform than Windows.

* platform/network/cf/SocketStreamHandleImplCFNet.cpp:
(WebCore::callbacksRunLoop):
(WebCore::callbacksRunLoopMode):
(WebCore::SocketStreamHandleImpl::scheduleStreams):
(WebCore::SocketStreamHandleImpl::pacExecutionCallback):
(WebCore::SocketStreamHandleImpl::executePACFileURL):
(WebCore::SocketStreamHandleImpl::removePACRunLoopSource):
(WebCore::SocketStreamHandleImpl::readStreamCallback):
(WebCore::SocketStreamHandleImpl::writeStreamCallback):
(WebCore::SocketStreamHandleImpl::platformClose):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (231318 => 231319)


--- trunk/Source/WebCore/ChangeLog	2018-05-03 18:39:44 UTC (rev 231318)
+++ trunk/Source/WebCore/ChangeLog	2018-05-03 19:17:46 UTC (rev 231319)
@@ -1,3 +1,32 @@
+2018-05-03  Chris Dumez  <cdu...@apple.com>
+
+        REGRESSION(iOS 11.3): Crashes in TimerBase::~TimerBase() in Tencent x5gamehelper
+        https://bugs.webkit.org/show_bug.cgi?id=185073
+        <rdar://problem/39821223>
+
+        Reviewed by Alexey Proskuryakov.
+
+        The following changes were made:
+        - Make sure SocketStream callbacks are always scheduled on the right runloop:
+          WebThreadRunLoop() on WebKitLegacy iOS, loaderRunLoop() on Windows and
+          main runloop otherwise.
+        - When the SocketStream callbacks are called, unconditionally call callOnMainThreadAndWait()
+          before calling methods on the SocketStream client. Previously, this code path
+          was specific to Windows but there is no reason to have platform-specific code here.
+          callOnMainThreadAndWait() calls the function right away if we're already on the main
+          thread, which will be the case on other platform than Windows.
+
+        * platform/network/cf/SocketStreamHandleImplCFNet.cpp:
+        (WebCore::callbacksRunLoop):
+        (WebCore::callbacksRunLoopMode):
+        (WebCore::SocketStreamHandleImpl::scheduleStreams):
+        (WebCore::SocketStreamHandleImpl::pacExecutionCallback):
+        (WebCore::SocketStreamHandleImpl::executePACFileURL):
+        (WebCore::SocketStreamHandleImpl::removePACRunLoopSource):
+        (WebCore::SocketStreamHandleImpl::readStreamCallback):
+        (WebCore::SocketStreamHandleImpl::writeStreamCallback):
+        (WebCore::SocketStreamHandleImpl::platformClose):
+
 2018-05-03  Zalan Bujtas  <za...@apple.com>
 
         [LFC] Enable multiple layout roots for incremental layout.

Modified: trunk/Source/WebCore/platform/network/cf/SocketStreamHandleImplCFNet.cpp (231318 => 231319)


--- trunk/Source/WebCore/platform/network/cf/SocketStreamHandleImplCFNet.cpp	2018-05-03 18:39:44 UTC (rev 231318)
+++ trunk/Source/WebCore/platform/network/cf/SocketStreamHandleImplCFNet.cpp	2018-05-03 19:17:46 UTC (rev 231319)
@@ -53,6 +53,10 @@
 #include <WebKitSystemInterface/WebKitSystemInterface.h>
 #endif
 
+#if PLATFORM(IOS)
+#include "WebCoreThreadInternal.h"
+#endif
+
 #if PLATFORM(IOS) || PLATFORM(MAC)
 extern "C" const CFStringRef kCFStreamPropertySourceApplication;
 extern "C" const CFStringRef _kCFStreamSocketSetNoDelay;
@@ -71,6 +75,26 @@
 
 namespace WebCore {
 
+static inline CFRunLoopRef callbacksRunLoop()
+{
+#if PLATFORM(WIN)
+    return loaderRunLoop();
+#elif PLATFORM(IOS)
+    return WebThreadRunLoop();
+#else
+    return CFRunLoopGetMain();
+#endif
+}
+
+static inline auto callbacksRunLoopMode()
+{
+#if PLATFORM(WIN)
+    return kCFRunLoopDefaultMode;
+#else
+    return kCFRunLoopCommonModes;
+#endif
+}
+
 SocketStreamHandleImpl::SocketStreamHandleImpl(const URL& url, SocketStreamHandleClient& client, PAL::SessionID sessionID, const String& credentialPartition, SourceApplicationAuditToken&& auditData)
     : SocketStreamHandle(url, client)
     , m_connectingSubstate(New)
@@ -119,14 +143,8 @@
     CFReadStreamSetClient(m_readStream.get(), static_cast<CFOptionFlags>(-1), readStreamCallback, &clientContext);
     CFWriteStreamSetClient(m_writeStream.get(), static_cast<CFOptionFlags>(-1), writeStreamCallback, &clientContext);
 
-#if PLATFORM(WIN)
-    CFReadStreamScheduleWithRunLoop(m_readStream.get(), loaderRunLoop(), kCFRunLoopDefaultMode);
-    CFWriteStreamScheduleWithRunLoop(m_writeStream.get(), loaderRunLoop(), kCFRunLoopDefaultMode);
-#else
-    RELEASE_ASSERT(isMainThread());
-    CFReadStreamScheduleWithRunLoop(m_readStream.get(), CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
-    CFWriteStreamScheduleWithRunLoop(m_writeStream.get(), CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
-#endif
+    CFReadStreamScheduleWithRunLoop(m_readStream.get(), callbacksRunLoop(), callbacksRunLoopMode());
+    CFWriteStreamScheduleWithRunLoop(m_writeStream.get(), callbacksRunLoop(), callbacksRunLoopMode());
 
     CFReadStreamOpen(m_readStream.get());
     CFWriteStreamOpen(m_writeStream.get());
@@ -167,6 +185,7 @@
 {
     SocketStreamHandleImpl* handle = static_cast<SocketStreamHandleImpl*>(client);
 
+    RefPtr<SocketStreamHandle> protector(handle);
     callOnMainThreadAndWait([&] {
         ASSERT(handle->m_connectingSubstate == ExecutingPACFile);
         // This time, the array won't have PAC as a first entry.
@@ -183,11 +202,7 @@
     // CFNetwork returns an empty proxy array for WebSocket schemes, so use m_httpsURL.
     CFStreamClientContext clientContext = { 0, this, retainSocketStreamHandle, releaseSocketStreamHandle, copyPACExecutionDescription };
     m_pacRunLoopSource = adoptCF(CFNetworkExecuteProxyAutoConfigurationURL(pacFileURL, m_httpsURL.get(), pacExecutionCallback, &clientContext));
-#if PLATFORM(WIN)
-    CFRunLoopAddSource(loaderRunLoop(), m_pacRunLoopSource.get(), kCFRunLoopDefaultMode);
-#else
-    CFRunLoopAddSource(CFRunLoopGetCurrent(), m_pacRunLoopSource.get(), kCFRunLoopCommonModes);
-#endif
+    CFRunLoopAddSource(callbacksRunLoop(), m_pacRunLoopSource.get(), callbacksRunLoopMode());
     m_connectingSubstate = ExecutingPACFile;
 }
 
@@ -196,11 +211,7 @@
     ASSERT(m_pacRunLoopSource);
 
     CFRunLoopSourceInvalidate(m_pacRunLoopSource.get());
-#if PLATFORM(WIN)
-    CFRunLoopRemoveSource(loaderRunLoop(), m_pacRunLoopSource.get(), kCFRunLoopDefaultMode);
-#else
-    CFRunLoopRemoveSource(CFRunLoopGetCurrent(), m_pacRunLoopSource.get(), kCFRunLoopCommonModes);
-#endif
+    CFRunLoopRemoveSource(callbacksRunLoop(), m_pacRunLoopSource.get(), callbacksRunLoopMode());
     m_pacRunLoopSource = 0;
 }
 
@@ -463,16 +474,11 @@
     if (!handle->m_readStream)
         return;
 
-#if PLATFORM(WIN)
     RefPtr<SocketStreamHandle> protector(handle);
     callOnMainThreadAndWait([&] {
         if (handle->m_readStream)
             handle->readStreamCallback(type);
     });
-#else
-    RELEASE_ASSERT(isMainThread());
-    handle->readStreamCallback(type);
-#endif
 }
 
 void SocketStreamHandleImpl::writeStreamCallback(CFWriteStreamRef stream, CFStreamEventType type, void* clientCallBackInfo)
@@ -483,16 +489,11 @@
     if (!handle->m_writeStream)
         return;
 
-#if PLATFORM(WIN)
     RefPtr<SocketStreamHandle> protector(handle);
     callOnMainThreadAndWait([&] {
         if (handle->m_writeStream)
             handle->writeStreamCallback(type);
     });
-#else
-    ASSERT(isMainThread());
-    handle->writeStreamCallback(type);
-#endif
 }
 
 #if !PLATFORM(IOS)
@@ -727,13 +728,8 @@
         return;
     }
 
-#if PLATFORM(WIN)
-    CFReadStreamUnscheduleFromRunLoop(m_readStream.get(), loaderRunLoop(), kCFRunLoopDefaultMode);
-    CFWriteStreamUnscheduleFromRunLoop(m_writeStream.get(), loaderRunLoop(), kCFRunLoopDefaultMode);
-#else
-    CFReadStreamUnscheduleFromRunLoop(m_readStream.get(), CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
-    CFWriteStreamUnscheduleFromRunLoop(m_writeStream.get(), CFRunLoopGetCurrent(), kCFRunLoopCommonModes);
-#endif
+    CFReadStreamUnscheduleFromRunLoop(m_readStream.get(), callbacksRunLoop(), callbacksRunLoopMode());
+    CFWriteStreamUnscheduleFromRunLoop(m_writeStream.get(), callbacksRunLoop(), callbacksRunLoopMode());
 
     CFReadStreamClose(m_readStream.get());
     CFWriteStreamClose(m_writeStream.get());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to