Merge r220569 - 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.
WebKit · Aug 14, 2017 · d6210b2 · d6210b2
1 parent 58aa303
commit d6210b2
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 10 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+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>
 
         [JSC] Use @toNumber in builtins

diff --git a/Source/WebCore/platform/ThreadGlobalData.cpp b/Source/WebCore/platform/ThreadGlobalData.cpp
@@ -81,7 +81,7 @@ void ThreadGlobalData::destroy()
 }
 
 #if USE(WEB_THREAD)
-static ThreadSpecific<std::unique_ptr<ThreadGlobalData>>* staticData { nullptr };
+static ThreadSpecific<RefPtr<ThreadGlobalData>>* staticData { nullptr };
 static ThreadGlobalData* sharedMainThreadStaticData { nullptr };
 
 void ThreadGlobalData::setWebCoreThreadData()
@@ -90,32 +90,30 @@ void ThreadGlobalData::setWebCoreThreadData()
     ASSERT(&threadGlobalData() != sharedMainThreadStaticData);
 
     // 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);
 }
 
 ThreadGlobalData& threadGlobalData()
 {
     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;
     }
 
     auto& result = **staticData;
     if (!result)
-        result.reset(new ThreadGlobalData());
+        result = adoptRef(new ThreadGlobalData);
     return *result;
 }
 

diff --git a/Source/WebCore/platform/ThreadGlobalData.h b/Source/WebCore/platform/ThreadGlobalData.h
@@ -27,6 +27,7 @@
 #ifndef ThreadGlobalData_h
 #define ThreadGlobalData_h
 
+#include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/text/StringHash.h>
 
 namespace WebCore {
@@ -39,7 +40,11 @@ namespace WebCore {
     struct ICUConverterWrapper;
     struct TECConverterWrapper;
 
+#if USE(WEB_THREAD)
+    class ThreadGlobalData : public ThreadSafeRefCounted<ThreadGlobalData> {
+#else
     class ThreadGlobalData {
+#endif
         WTF_MAKE_NONCOPYABLE(ThreadGlobalData);
         WTF_MAKE_FAST_ALLOCATED;
     public: