Changeset 220569 in webkit

Timestamp:

Aug 10, 2017 5:41:53 PM (7 years ago)

Author:

Yusuke Suzuki

Message:

Make ThreadGlobalData RefCounted for web thread
​https://bugs.webkit.org/show_bug.cgi?id=175439

Reviewed by Mark Lam.

When the web thread is enabled, we share ThreadGlobalData between the web thread and the main thread.
The problem happens when the main thread is dying. It could start deallocating TLS and the web
thread may see the destructed ThreadGlobalData.

Even though, the current implementation is safe because the main thread do not perform TLS deallocation
in the Darwin environment. But this is not true in Windows. And we should not rely on this condition
that depends on the platforms.

In this patch, we make ThreadGlobalData ThreadSafeRefCounted. This type verbosely describes that
ThreadGlobalData could be shared between threads when the web thread enabled. And make the life time
management simple instead of relying on the platform dependent TLS implementation.

• platform/ThreadGlobalData.cpp:

(WebCore::ThreadGlobalData::setWebCoreThreadData):
(WebCore::threadGlobalData):

• platform/ThreadGlobalData.h:

(WebCore::ThreadGlobalData::cachedResourceRequestInitiators): Deleted.
(WebCore::ThreadGlobalData::eventNames): Deleted.
(WebCore::ThreadGlobalData::threadTimers): Deleted.
(WebCore::ThreadGlobalData::qualifiedNameCache): Deleted.
(WebCore::ThreadGlobalData::cachedConverterICU): Deleted.
(WebCore::ThreadGlobalData::cachedConverterTEC): Deleted.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/ThreadGlobalData.cpp (modified) (4 diffs)
• platform/ThreadGlobalData.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-08-10  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Make ThreadGlobalData RefCounted for web thread
+        https://bugs.webkit.org/show_bug.cgi?id=175439
+
+        Reviewed by Mark Lam.
+
+        When the web thread is enabled, we share ThreadGlobalData between the web thread and the main thread.
+        The problem happens when the main thread is dying. It could start deallocating TLS and the web
+        thread may see the destructed ThreadGlobalData.
+
+        Even though, the current implementation is safe because the main thread do not perform TLS deallocation
+        in the Darwin environment. But this is not true in Windows. And we should not rely on this condition
+        that depends on the platforms.
+
+        In this patch, we make ThreadGlobalData ThreadSafeRefCounted. This type verbosely describes that
+        ThreadGlobalData could be shared between threads when the web thread enabled. And make the life time
+        management simple instead of relying on the platform dependent TLS implementation.
+
+        * platform/ThreadGlobalData.cpp:
+        (WebCore::ThreadGlobalData::setWebCoreThreadData):
+        (WebCore::threadGlobalData):
+        * platform/ThreadGlobalData.h:
+        (WebCore::ThreadGlobalData::cachedResourceRequestInitiators): Deleted.
+        (WebCore::ThreadGlobalData::eventNames): Deleted.
+        (WebCore::ThreadGlobalData::threadTimers): Deleted.
+        (WebCore::ThreadGlobalData::qualifiedNameCache): Deleted.
+        (WebCore::ThreadGlobalData::cachedConverterICU): Deleted.
+        (WebCore::ThreadGlobalData::cachedConverterTEC): Deleted.
+
 2017-08-10  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/WebCore/platform/ThreadGlobalData.cpp

@@ -82 +82 @@
 
 #if USE(WEB_THREAD)
-static ThreadSpecific<std::unique_ptr<ThreadGlobalData>>* staticData { nullptr };
+static ThreadSpecific<RefPtr<ThreadGlobalData>>* staticData { nullptr };
 static ThreadGlobalData* sharedMainThreadStaticData { nullptr };
 
@@ -91 +91 @@
 
     // Set WebThread's ThreadGlobalData object to be the same as the main UI thread.
-    // The web thread never finishes, and we expect the main thread to also never finish.
-    // Hence, it is safe to store the same ThreadGlobalData pointer in a thread specific std::unique_ptr.
-    // FIXME: Make ThreadGlobalData RefCounted for web thread.
-    // https://bugs.webkit.org/show_bug.cgi?id=175439
-    (**staticData).reset(sharedMainThreadStaticData);
+    **staticData = adoptRef(sharedMainThreadStaticData);
 
     ASSERT(&threadGlobalData() == sharedMainThreadStaticData);
@@ -103 +99 @@
 {
     if (UNLIKELY(!staticData)) {
-        staticData = new ThreadSpecific<std::unique_ptr<ThreadGlobalData>>;
+        staticData = new ThreadSpecific<RefPtr<ThreadGlobalData>>;
         auto& result = **staticData;
         ASSERT(!result);
-        result.reset(new ThreadGlobalData());
+        result = adoptRef(new ThreadGlobalData);
         // WebThread and main UI thread need to share the same object. Save it in a static
         // here, the WebThread will pick it up in setWebCoreThreadData().
-        if (pthread_main_np())
+        if (pthread_main_np()) {
             sharedMainThreadStaticData = result.get();
+            result->ref();
+        }
         return *result;
     }
@@ -116 +114 @@
     auto& result = **staticData;
     if (!result)
-        result.reset(new ThreadGlobalData());
+        result = adoptRef(new ThreadGlobalData);
     return *result;
 }

trunk/Source/WebCore/platform/ThreadGlobalData.h

@@ -28 +28 @@
 #define ThreadGlobalData_h
 
+#include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/text/StringHash.h>
 
@@ -40 +41 @@
     struct TECConverterWrapper;
 
+#if USE(WEB_THREAD)
+    class ThreadGlobalData : public ThreadSafeRefCounted<ThreadGlobalData> {
+#else
     class ThreadGlobalData {
+#endif
         WTF_MAKE_NONCOPYABLE(ThreadGlobalData);
         WTF_MAKE_FAST_ALLOCATED;