Title: [199621] trunk/Source/WTF
Revision
199621
Author
[email protected]
Date
2016-04-15 21:52:18 -0700 (Fri, 15 Apr 2016)

Log Message

iTunes crashing _javascript_Core.dll
https://bugs.webkit.org/show_bug.cgi?id=156647

Reviewed by Geoffrey Garen.

If a thread was created without using the WTF thread apis and that thread uses
a _javascript_ VM and that thread exits with the VM still around, JSC won't know
that the thread has exited.  Currently, we use ThreadSpecificThreadExit() to
clean up any thread specific keys.  Cleaning up these keys is how JSC is
notified of a thread exit.  We only call ThreadSpecificThreadExit() from
wtfThreadEntryPoint() when the thread entry point function returns.
This mechanism was put in place for Windos because we layer the WTF::ThreadSpecific
functionality on top of TLS (Thread Local Storage), but TLS doesn't have
a thread exiting callback the way that pthread_create_key.

The fix is to change from using TLS to using FLS (Fiber Local Storage).  Although
Windows allows multiple fibers per thread, WebKit is not designed to work with a
multiple fibers per thread.  When ther is only one fiber per thread, FLS works just
like TLS, but it has the destroy callback.

I restructured the Windows version of WTF::ThreadSpecific to be almost the same
as the pthread version.
        
* wtf/ThreadSpecific.h:
(WTF::threadSpecificKeyCreate):
(WTF::threadSpecificKeyDelete):
(WTF::threadSpecificSet):
(WTF::threadSpecificGet):
(WTF::ThreadSpecific<T>::ThreadSpecific):
(WTF::ThreadSpecific<T>::~ThreadSpecific):
(WTF::ThreadSpecific<T>::get):
(WTF::ThreadSpecific<T>::set):
(WTF::ThreadSpecific<T>::destroy):
Restructured to use FLS.  Renamed TLS* to FLS*.

* wtf/ThreadSpecificWin.cpp:
(WTF::flsKeyCount):
(WTF::flsKeys):
Renamed from tlsKey*() to flsKey*().

(WTF::destructorsList): Deleted.
(WTF::destructorsMutex): Deleted.
(WTF::PlatformThreadSpecificKey::PlatformThreadSpecificKey): Deleted.
(WTF::PlatformThreadSpecificKey::~PlatformThreadSpecificKey): Deleted.
(WTF::PlatformThreadSpecificKey::setValue): Deleted.
(WTF::PlatformThreadSpecificKey::value): Deleted.
(WTF::PlatformThreadSpecificKey::callDestructor): Deleted.
(WTF::tlsKeyCount): Deleted.
(WTF::tlsKeys): Deleted.
(WTF::threadSpecificKeyCreate): Deleted.
(WTF::threadSpecificKeyDelete): Deleted.
(WTF::threadSpecificSet): Deleted.
(WTF::threadSpecificGet): Deleted.
(WTF::ThreadSpecificThreadExit): Deleted.

* wtf/ThreadingWin.cpp:
(WTF::wtfThreadEntryPoint): Eliminated call to ThreadSpecificThreadExit.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (199620 => 199621)


--- trunk/Source/WTF/ChangeLog	2016-04-16 04:29:32 UTC (rev 199620)
+++ trunk/Source/WTF/ChangeLog	2016-04-16 04:52:18 UTC (rev 199621)
@@ -1,3 +1,63 @@
+2016-04-15  Michael Saboff  <[email protected]>
+
+        iTunes crashing _javascript_Core.dll
+        https://bugs.webkit.org/show_bug.cgi?id=156647
+
+        Reviewed by Geoffrey Garen.
+
+        If a thread was created without using the WTF thread apis and that thread uses
+        a _javascript_ VM and that thread exits with the VM still around, JSC won't know
+        that the thread has exited.  Currently, we use ThreadSpecificThreadExit() to
+        clean up any thread specific keys.  Cleaning up these keys is how JSC is
+        notified of a thread exit.  We only call ThreadSpecificThreadExit() from
+        wtfThreadEntryPoint() when the thread entry point function returns.
+        This mechanism was put in place for Windos because we layer the WTF::ThreadSpecific
+        functionality on top of TLS (Thread Local Storage), but TLS doesn't have
+        a thread exiting callback the way that pthread_create_key.
+
+        The fix is to change from using TLS to using FLS (Fiber Local Storage).  Although
+        Windows allows multiple fibers per thread, WebKit is not designed to work with a
+        multiple fibers per thread.  When ther is only one fiber per thread, FLS works just
+        like TLS, but it has the destroy callback.
+
+        I restructured the Windows version of WTF::ThreadSpecific to be almost the same
+        as the pthread version.
+        
+        * wtf/ThreadSpecific.h:
+        (WTF::threadSpecificKeyCreate):
+        (WTF::threadSpecificKeyDelete):
+        (WTF::threadSpecificSet):
+        (WTF::threadSpecificGet):
+        (WTF::ThreadSpecific<T>::ThreadSpecific):
+        (WTF::ThreadSpecific<T>::~ThreadSpecific):
+        (WTF::ThreadSpecific<T>::get):
+        (WTF::ThreadSpecific<T>::set):
+        (WTF::ThreadSpecific<T>::destroy):
+        Restructured to use FLS.  Renamed TLS* to FLS*.
+
+        * wtf/ThreadSpecificWin.cpp:
+        (WTF::flsKeyCount):
+        (WTF::flsKeys):
+        Renamed from tlsKey*() to flsKey*().
+
+        (WTF::destructorsList): Deleted.
+        (WTF::destructorsMutex): Deleted.
+        (WTF::PlatformThreadSpecificKey::PlatformThreadSpecificKey): Deleted.
+        (WTF::PlatformThreadSpecificKey::~PlatformThreadSpecificKey): Deleted.
+        (WTF::PlatformThreadSpecificKey::setValue): Deleted.
+        (WTF::PlatformThreadSpecificKey::value): Deleted.
+        (WTF::PlatformThreadSpecificKey::callDestructor): Deleted.
+        (WTF::tlsKeyCount): Deleted.
+        (WTF::tlsKeys): Deleted.
+        (WTF::threadSpecificKeyCreate): Deleted.
+        (WTF::threadSpecificKeyDelete): Deleted.
+        (WTF::threadSpecificSet): Deleted.
+        (WTF::threadSpecificGet): Deleted.
+        (WTF::ThreadSpecificThreadExit): Deleted.
+
+        * wtf/ThreadingWin.cpp:
+        (WTF::wtfThreadEntryPoint): Eliminated call to ThreadSpecificThreadExit.
+
 2016-04-12  Filip Pizlo  <[email protected]>
 
         PolymorphicAccess should buffer AccessCases before regenerating

Modified: trunk/Source/WTF/wtf/ThreadSpecific.h (199620 => 199621)


--- trunk/Source/WTF/wtf/ThreadSpecific.h	2016-04-16 04:29:32 UTC (rev 199620)
+++ trunk/Source/WTF/wtf/ThreadSpecific.h	2016-04-16 04:52:18 UTC (rev 199621)
@@ -94,9 +94,6 @@
 
         T* value;
         ThreadSpecific<T>* owner;
-#if OS(WINDOWS)
-        void (*destructor)(void*);
-#endif
     };
 
 #if USE(PTHREADS)
@@ -158,47 +155,64 @@
 
 #elif OS(WINDOWS)
 
-// The maximum number of TLS keys that can be created. For simplification, we assume that:
+// The maximum number of FLS keys that can be created. For simplification, we assume that:
 // 1) Once the instance of ThreadSpecific<> is created, it will not be destructed until the program dies.
 // 2) We do not need to hold many instances of ThreadSpecific<> data. This fixed number should be far enough.
-const int kMaxTlsKeySize = 256;
+const int kMaxFlsKeySize = 100;
 
-WTF_EXPORT_PRIVATE long& tlsKeyCount();
-WTF_EXPORT_PRIVATE DWORD* tlsKeys();
+WTF_EXPORT_PRIVATE long& flsKeyCount();
+WTF_EXPORT_PRIVATE DWORD* flsKeys();
 
-class PlatformThreadSpecificKey;
-typedef PlatformThreadSpecificKey* ThreadSpecificKey;
+typedef DWORD ThreadSpecificKey;
 
-WTF_EXPORT_PRIVATE void threadSpecificKeyCreate(ThreadSpecificKey*, void (*)(void *));
-WTF_EXPORT_PRIVATE void threadSpecificKeyDelete(ThreadSpecificKey);
-WTF_EXPORT_PRIVATE void threadSpecificSet(ThreadSpecificKey, void*);
-WTF_EXPORT_PRIVATE void* threadSpecificGet(ThreadSpecificKey);
+inline void threadSpecificKeyCreate(ThreadSpecificKey* key, void (*destructor)(void *))
+{
+    DWORD flsKey = FlsAlloc(reinterpret_cast<PFLS_CALLBACK_FUNCTION>(destructor));
+    if (flsKey == FLS_OUT_OF_INDEXES)
+        CRASH();
 
+    *key = flsKey;
+}
+
+inline void threadSpecificKeyDelete(ThreadSpecificKey key)
+{
+    FlsFree(key);
+}
+
+inline void threadSpecificSet(ThreadSpecificKey key, void* data)
+{
+    FlsSetValue(key, data);
+}
+
+inline void* threadSpecificGet(ThreadSpecificKey key)
+{
+    return FlsGetValue(key);
+}
+
 template<typename T>
 inline ThreadSpecific<T>::ThreadSpecific()
     : m_index(-1)
 {
-    DWORD tlsKey = TlsAlloc();
-    if (tlsKey == TLS_OUT_OF_INDEXES)
+    DWORD flsKey = FlsAlloc(reinterpret_cast<PFLS_CALLBACK_FUNCTION>(destroy));
+    if (flsKey == FLS_OUT_OF_INDEXES)
         CRASH();
 
-    m_index = InterlockedIncrement(&tlsKeyCount()) - 1;
-    if (m_index >= kMaxTlsKeySize)
+    m_index = InterlockedIncrement(&flsKeyCount()) - 1;
+    if (m_index >= kMaxFlsKeySize)
         CRASH();
-    tlsKeys()[m_index] = tlsKey;
+    flsKeys()[m_index] = flsKey;
 }
 
 template<typename T>
 inline ThreadSpecific<T>::~ThreadSpecific()
 {
-    // Does not invoke destructor functions. They will be called from ThreadSpecificThreadExit when the thread is detached.
-    TlsFree(tlsKeys()[m_index]);
+    FlsFree(flsKeys()[m_index]);
 }
 
 template<typename T>
 inline T* ThreadSpecific<T>::get()
 {
-    Data* data = ""
+    Data* data = ""
     return data ? data->value : 0;
 }
 
@@ -207,8 +221,7 @@
 {
     ASSERT(!get());
     Data* data = "" Data(ptr, this);
-    data->destructor = &ThreadSpecific<T>::destroy;
-    TlsSetValue(tlsKeys()[m_index], data);
+    FlsSetValue(flsKeys()[m_index], data);
 }
 
 #else
@@ -232,7 +245,7 @@
 #if USE(PTHREADS)
     pthread_setspecific(data->owner->m_key, 0);
 #elif OS(WINDOWS)
-    TlsSetValue(tlsKeys()[data->owner->m_index], 0);
+    FlsSetValue(flsKeys()[data->owner->m_index], 0);
 #else
 #error ThreadSpecific is not implemented for this platform.
 #endif

Modified: trunk/Source/WTF/wtf/ThreadSpecificWin.cpp (199620 => 199621)


--- trunk/Source/WTF/wtf/ThreadSpecificWin.cpp	2016-04-16 04:29:32 UTC (rev 199620)
+++ trunk/Source/WTF/wtf/ThreadSpecificWin.cpp	2016-04-16 04:52:18 UTC (rev 199621)
@@ -24,117 +24,22 @@
 
 #if OS(WINDOWS)
 
-#include "StdLibExtras.h"
-#include "ThreadingPrimitives.h"
-#include <wtf/DoublyLinkedList.h>
-
 #if !USE(PTHREADS)
 
 namespace WTF {
 
-static DoublyLinkedList<PlatformThreadSpecificKey>& destructorsList()
+long& flsKeyCount()
 {
-    static DoublyLinkedList<PlatformThreadSpecificKey> staticList;
-    return staticList;
-}
-
-static Mutex& destructorsMutex()
-{
-    static Mutex staticMutex;
-    return staticMutex;
-}
-
-class PlatformThreadSpecificKey : public DoublyLinkedListNode<PlatformThreadSpecificKey> {
-public:
-    friend class DoublyLinkedListNode<PlatformThreadSpecificKey>;
-
-    PlatformThreadSpecificKey(void (*destructor)(void *))
-        : m_destructor(destructor)
-    {
-        m_tlsKey = TlsAlloc();
-        if (m_tlsKey == TLS_OUT_OF_INDEXES)
-            CRASH();
-    }
-
-    ~PlatformThreadSpecificKey()
-    {
-        TlsFree(m_tlsKey);
-    }
-
-    void setValue(void* data) { TlsSetValue(m_tlsKey, data); }
-    void* value() { return TlsGetValue(m_tlsKey); }
-
-    void callDestructor()
-    {
-       if (void* data = ""
-            m_destructor(data);
-    }
-
-private:
-    void (*m_destructor)(void *);
-    DWORD m_tlsKey;
-    PlatformThreadSpecificKey* m_prev;
-    PlatformThreadSpecificKey* m_next;
-};
-
-long& tlsKeyCount()
-{
     static long count;
     return count;
 }
 
-DWORD* tlsKeys()
+DWORD* flsKeys()
 {
-    static DWORD keys[kMaxTlsKeySize];
+    static DWORD keys[kMaxFlsKeySize];
     return keys;
 }
 
-void threadSpecificKeyCreate(ThreadSpecificKey* key, void (*destructor)(void *))
-{
-    // Use the original malloc() instead of fastMalloc() to use this function in FastMalloc code.
-    *key = static_cast<PlatformThreadSpecificKey*>(::malloc(sizeof(PlatformThreadSpecificKey)));
-    new (*key) PlatformThreadSpecificKey(destructor);
-
-    MutexLocker locker(destructorsMutex());
-    destructorsList().push(*key);
-}
-
-void threadSpecificKeyDelete(ThreadSpecificKey key)
-{
-    MutexLocker locker(destructorsMutex());
-    destructorsList().remove(key);
-    key->~PlatformThreadSpecificKey();
-    ::free(key);
-}
-
-void threadSpecificSet(ThreadSpecificKey key, void* data)
-{
-    key->setValue(data);
-}
-
-void* threadSpecificGet(ThreadSpecificKey key)
-{
-    return key->value();
-}
-
-void ThreadSpecificThreadExit()
-{
-    for (long i = 0; i < tlsKeyCount(); i++) {
-        // The layout of ThreadSpecific<T>::Data does not depend on T. So we are safe to do the static cast to ThreadSpecific<int> in order to access its data member.
-        ThreadSpecific<int>::Data* data = ""
-        if (data)
-            data->destructor(data);
-    }
-
-    MutexLocker locker(destructorsMutex());
-    PlatformThreadSpecificKey* key = destructorsList().head();
-    while (key) {
-        PlatformThreadSpecificKey* nextKey = key->next();
-        key->callDestructor();
-        key = nextKey;
-    }
-}
-
 } // namespace WTF
 
 #endif // !USE(PTHREADS)

Modified: trunk/Source/WTF/wtf/ThreadingWin.cpp (199620 => 199621)


--- trunk/Source/WTF/wtf/ThreadingWin.cpp	2016-04-16 04:29:32 UTC (rev 199620)
+++ trunk/Source/WTF/wtf/ThreadingWin.cpp	2016-04-16 04:52:18 UTC (rev 199621)
@@ -102,10 +102,6 @@
 #include <wtf/RandomNumberSeed.h>
 #include <wtf/WTFThreadData.h>
 
-#if !USE(PTHREADS) && OS(WINDOWS)
-#include "ThreadSpecific.h"
-#endif
-
 #if HAVE(ERRNO_H)
 #include <errno.h>
 #endif
@@ -199,11 +195,6 @@
     std::unique_ptr<ThreadFunctionInvocation> invocation(static_cast<ThreadFunctionInvocation*>(param));
     invocation->function(invocation->data);
 
-#if !USE(PTHREADS) && OS(WINDOWS)
-    // Do the TLS cleanup.
-    ThreadSpecificThreadExit();
-#endif
-
     return 0;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to