Attachment 373360 Details for Bug 199420 – Patch

[patch] Patch

bug-199420-20190702141151.patch (text/plain), 6.67 KB, created by Chris Dumez on 2019-07-02 14:11:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-07-02 14:11:51 PDT

Size: 6.67 KB

patch

obsolete

>Subversion Revision: 247051
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 2534920651b17fd8e578e835739255698b4d6bdc..0b159efdc859b1d1c8fe78fda82da32a643d95e3 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-07-02  Chris Dumez  <cdumez@apple.com>
>+
>+        ThreadSafeRefCounted<DestructionThread::Main> is not safe to use in the UIProcess
>+        https://bugs.webkit.org/show_bug.cgi?id=199420
>+        <rdar://problem/52289717>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/MainThread.cpp:
>+        (WTF::isMainRunLoop):
>+        (WTF::callOnMainRunLoop):
>+        * wtf/MainThread.h:
>+        Add some function to MainThread.h to be used by ThreadSafeRefCounted to interact with the
>+        main RunLoop. This is used to avoid a circular dependency between RunLoop (which is
>+        ThreadSafeRefCounted) and ThreadSafeReCounted.
>+
>+        * wtf/ThreadSafeRefCounted.h:
>+        (WTF::ThreadSafeRefCounted::deref const):
>+        Add a new DestructionThread::MainRunLoop enum value to be used by classes that need to
>+        be destroyed on the main RunLoop rather than the main thread (which may be different
>+        when WK1 is invoved)
>+
> 2019-07-02  Keith Miller  <keith_miller@apple.com>
> 
>         PACCage should first cage leaving PAC bits intact then authenticate
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7681dc6a2af771fb750c3792d68fa178216663b1..5f6f21fe6d7c6f94886d266e838c19f0c5bf4d72 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-07-02  Chris Dumez  <cdumez@apple.com>
>+
>+        ThreadSafeRefCounted<DestructionThread::Main> is not safe to use in the UIProcess
>+        https://bugs.webkit.org/show_bug.cgi?id=199420
>+        <rdar://problem/52289717>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update IPC::Connection and DeviceIdHashSaltStorage to use DestructionThread::MainRunLoop
>+        instead of DestructionThread::Main, since both classes are used in the UIProcess.
>+
>+        Using DestructionThread::Main is not safe in the UIProcess because its implementation relies
>+        on isMainThread() / callOnMainThread(). Those get confused about which thread is the main
>+        thread when an application uses both WK1 and WK2.
>+
>+        * Platform/IPC/Connection.h:
>+        * UIProcess/DeviceIdHashSaltStorage.h:
>+
> 2019-07-02  Chris Dumez  <cdumez@apple.com>
> 
>         Protect NetworkProcess::m_networkSessions against corruption
>diff --git a/Source/WTF/wtf/MainThread.cpp b/Source/WTF/wtf/MainThread.cpp
>index fce7b46a52bdbec4a60d54109b46a0663e60bd19..9edd8ef18d0b5bfc973bb011d0ded31b5ea237d9 100644
>--- a/Source/WTF/wtf/MainThread.cpp
>+++ b/Source/WTF/wtf/MainThread.cpp
>@@ -35,6 +35,7 @@
> #include <wtf/Lock.h>
> #include <wtf/MonotonicTime.h>
> #include <wtf/NeverDestroyed.h>
>+#include <wtf/RunLoop.h>
> #include <wtf/StdLibExtras.h>
> #include <wtf/ThreadSpecific.h>
> #include <wtf/Threading.h>
>@@ -126,6 +127,16 @@ void dispatchFunctionsFromMainThread()
>     }
> }
> 
>+bool isMainRunLoop()
>+{
>+    return RunLoop::isMain();
>+}
>+
>+void callOnMainRunLoop(Function<void()>&& function)
>+{
>+    RunLoop::main().dispatch(WTFMove(function));
>+}
>+
> void callOnMainThread(Function<void()>&& function)
> {
>     ASSERT(function);
>diff --git a/Source/WTF/wtf/MainThread.h b/Source/WTF/wtf/MainThread.h
>index 4e45c689a022503ff186286a05130d57b4bc674c..08341335d2a70d6e80565b84da035b90a664b949 100644
>--- a/Source/WTF/wtf/MainThread.h
>+++ b/Source/WTF/wtf/MainThread.h
>@@ -57,6 +57,9 @@ WTF_EXPORT_PRIVATE bool isMainThreadIfInitialized();
> 
> WTF_EXPORT_PRIVATE bool canAccessThreadLocalDataForThread(Thread&);
> 
>+WTF_EXPORT_PRIVATE bool isMainRunLoop();
>+WTF_EXPORT_PRIVATE void callOnMainRunLoop(Function<void()>&&);
>+
> #if USE(WEB_THREAD)
> WTF_EXPORT_PRIVATE bool isWebThread();
> WTF_EXPORT_PRIVATE bool isUIThread();
>diff --git a/Source/WTF/wtf/ThreadSafeRefCounted.h b/Source/WTF/wtf/ThreadSafeRefCounted.h
>index c4e809ae93ab4b289bdcea2e9eaa03edad3ad7e6..8a624b22aa8507d452258b45a1d6791268561f93 100644
>--- a/Source/WTF/wtf/ThreadSafeRefCounted.h
>+++ b/Source/WTF/wtf/ThreadSafeRefCounted.h
>@@ -64,7 +64,7 @@ private:
>     mutable std::atomic<unsigned> m_refCount { 1 };
> };
> 
>-enum class DestructionThread { Any, Main };
>+enum class DestructionThread { Any, Main, MainRunLoop };
> 
> template<class T, DestructionThread destructionThread = DestructionThread::Any> class ThreadSafeRefCounted : public ThreadSafeRefCountedBase {
> public:
>@@ -72,13 +72,27 @@ public:
>     {
>         if (!derefBase())
>             return;
>-        if (destructionThread == DestructionThread::Any || isMainThread()) {
>+
>+        auto deleteThis = [this] {
>             delete static_cast<const T*>(this);
>-            return;
>+        };
>+        switch (destructionThread) {
>+        case DestructionThread::Any:
>+            break;
>+        case DestructionThread::Main:
>+            if (!isMainThread()) {
>+                callOnMainThread(WTFMove(deleteThis));
>+                return;
>+            }
>+            break;
>+        case DestructionThread::MainRunLoop:
>+            if (!isMainRunLoop()) {
>+                callOnMainRunLoop(WTFMove(deleteThis));
>+                return;
>+            }
>+            break;
>         }
>-        callOnMainThread([this] {
>-            delete static_cast<const T*>(this);
>-        });
>+        deleteThis();
>     }
> 
> protected:
>diff --git a/Source/WebKit/Platform/IPC/Connection.h b/Source/WebKit/Platform/IPC/Connection.h
>index 3edf82adf3b5ae3b284c7dd5915b885e7523176f..4de0687ab532792aa34dd7d9ee3cd36d557af6f4 100644
>--- a/Source/WebKit/Platform/IPC/Connection.h
>+++ b/Source/WebKit/Platform/IPC/Connection.h
>@@ -90,7 +90,7 @@ template<typename AsyncReplyResult> struct AsyncReplyError {
> class MachMessage;
> class UnixMessage;
> 
>-class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThread::Main> {
>+class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThread::MainRunLoop> {
> public:
>     class Client : public MessageReceiver {
>     public:
>diff --git a/Source/WebKit/UIProcess/DeviceIdHashSaltStorage.h b/Source/WebKit/UIProcess/DeviceIdHashSaltStorage.h
>index 28caa7ca80309811ffa3b89ce370846cbed9184b..72cfa7b4079152487c1e48ab6799758f9e06988e 100644
>--- a/Source/WebKit/UIProcess/DeviceIdHashSaltStorage.h
>+++ b/Source/WebKit/UIProcess/DeviceIdHashSaltStorage.h
>@@ -36,7 +36,7 @@
> 
> namespace WebKit {
> 
>-class DeviceIdHashSaltStorage : public ThreadSafeRefCounted<DeviceIdHashSaltStorage, WTF::DestructionThread::Main> {
>+class DeviceIdHashSaltStorage : public ThreadSafeRefCounted<DeviceIdHashSaltStorage, WTF::DestructionThread::MainRunLoop> {
> public:
>     static Ref<DeviceIdHashSaltStorage> create(const String& deviceIdHashSaltStorageDirectory);
>     ~DeviceIdHashSaltStorage();

Actions: View | Formatted Diff | Diff

Attachments on bug 199420: 373360