- Revision
- 199726
- Author
- msab...@apple.com
- Date
- 2016-04-19 07:11:19 -0700 (Tue, 19 Apr 2016)
Log Message
iTunes crashing _javascript_Core.dll
https://bugs.webkit.org/show_bug.cgi?id=156647
Reviewed by Saam Barati.
Source/_javascript_Core:
Given that there there are only 128 FLS indices compared to over a 1000 for TLS, I
eliminated the thread specific m_threadSpecificForThread and instead we look for the
current thread in m_registeredThreads list when we need it. In most cases there
will only be one thread.
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::MachineThreads):
(JSC::MachineThreads::~MachineThreads):
(JSC::MachineThreads::addCurrentThread):
(JSC::MachineThreads::machineThreadForCurrentThread):
(JSC::MachineThreads::removeThread):
* heap/MachineStackMarker.h:
Source/WTF:
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 Windows 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 there 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.
LayoutTests:
Disabled fast/workers/dedicated-worker-lifecycle.html as it creates
more workers that we have ThreadSpecific keys. We need at least one
key per JSC VM we create. I didn't want to weaken the test for other
platforms.
* platform/win/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (199725 => 199726)
--- trunk/LayoutTests/ChangeLog 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/LayoutTests/ChangeLog 2016-04-19 14:11:19 UTC (rev 199726)
@@ -1,3 +1,17 @@
+2016-04-19 Michael Saboff <msab...@apple.com>
+
+ iTunes crashing _javascript_Core.dll
+ https://bugs.webkit.org/show_bug.cgi?id=156647
+
+ Reviewed by Saam Barati.
+
+ Disabled fast/workers/dedicated-worker-lifecycle.html as it creates
+ more workers that we have ThreadSpecific keys. We need at least one
+ key per JSC VM we create. I didn't want to weaken the test for other
+ platforms.
+
+ * platform/win/TestExpectations:
+
2016-04-19 Yusuke Suzuki <utatane....@gmail.com>
[INTL] Use @thisNumberValue instead of `instanceof @Number`
Modified: trunk/LayoutTests/platform/win/TestExpectations (199725 => 199726)
--- trunk/LayoutTests/platform/win/TestExpectations 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/LayoutTests/platform/win/TestExpectations 2016-04-19 14:11:19 UTC (rev 199726)
@@ -437,6 +437,10 @@
storage/storagequota-request-quota.html [ Skip ]
fast/workers/worker-storagequota-query-usage.html [ Skip ]
+# The number of workers in this test exceeds the number of
+# ThreadSpecificValues we have on Windows.
+fast/workers/dedicated-worker-lifecycle.html [ Skip ]
+
# Tests that require ENABLE(DOWNLOAD_ATTRIBUTE).
fast/dom/HTMLAnchorElement/anchor-nodownload.html [ Skip ]
fast/dom/HTMLAnchorElement/anchor-download.html [ Skip ]
Modified: trunk/Source/_javascript_Core/ChangeLog (199725 => 199726)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-19 14:11:19 UTC (rev 199726)
@@ -1,3 +1,23 @@
+2016-04-19 Michael Saboff <msab...@apple.com>
+
+ iTunes crashing _javascript_Core.dll
+ https://bugs.webkit.org/show_bug.cgi?id=156647
+
+ Reviewed by Saam Barati.
+
+ Given that there there are only 128 FLS indices compared to over a 1000 for TLS, I
+ eliminated the thread specific m_threadSpecificForThread and instead we look for the
+ current thread in m_registeredThreads list when we need it. In most cases there
+ will only be one thread.
+
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::MachineThreads):
+ (JSC::MachineThreads::~MachineThreads):
+ (JSC::MachineThreads::addCurrentThread):
+ (JSC::MachineThreads::machineThreadForCurrentThread):
+ (JSC::MachineThreads::removeThread):
+ * heap/MachineStackMarker.h:
+
2016-04-19 Yusuke Suzuki <utatane....@gmail.com>
[INTL] Use @thisNumberValue instead of `instanceof @Number`
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (199725 => 199726)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2016-04-19 14:11:19 UTC (rev 199726)
@@ -192,14 +192,12 @@
MachineThreads::MachineThreads(Heap* heap)
: m_registeredThreads(0)
, m_threadSpecificForMachineThreads(0)
- , m_threadSpecificForThread(0)
#if !ASSERT_DISABLED
, m_heap(heap)
#endif
{
UNUSED_PARAM(heap);
threadSpecificKeyCreate(&m_threadSpecificForMachineThreads, removeThread);
- threadSpecificKeyCreate(&m_threadSpecificForThread, nullptr);
activeMachineThreadsManager().add(this);
}
@@ -207,7 +205,6 @@
{
activeMachineThreadsManager().remove(this);
threadSpecificKeyDelete(m_threadSpecificForMachineThreads);
- threadSpecificKeyDelete(m_threadSpecificForThread);
LockHolder registeredThreadsLock(m_registeredThreadsMutex);
for (Thread* t = m_registeredThreads; t;) {
@@ -234,18 +231,6 @@
#endif
}
-#ifndef NDEBUG
-static bool isThreadInList(Thread* listHead, Thread* target)
-{
- for (Thread* thread = listHead; thread; thread = thread->next) {
- if (thread == target)
- return true;
- }
-
- return false;
-}
-#endif
-
void MachineThreads::addCurrentThread()
{
ASSERT(!m_heap->vm()->hasExclusiveThread() || m_heap->vm()->exclusiveThread() == std::this_thread::get_id());
@@ -254,15 +239,12 @@
#ifndef NDEBUG
LockHolder lock(m_registeredThreadsMutex);
ASSERT(threadSpecificGet(m_threadSpecificForMachineThreads) == this);
- ASSERT(threadSpecificGet(m_threadSpecificForThread));
- ASSERT(isThreadInList(m_registeredThreads, static_cast<Thread*>(threadSpecificGet(m_threadSpecificForThread))));
#endif
return;
}
Thread* thread = Thread::createForCurrentThread();
threadSpecificSet(m_threadSpecificForMachineThreads, this);
- threadSpecificSet(m_threadSpecificForThread, thread);
LockHolder lock(m_registeredThreadsMutex);
@@ -272,14 +254,15 @@
Thread* MachineThreads::machineThreadForCurrentThread()
{
- Thread* result = static_cast<Thread*>(threadSpecificGet(m_threadSpecificForThread));
- RELEASE_ASSERT(result);
-#ifndef NDEBUG
LockHolder lock(m_registeredThreadsMutex);
- ASSERT(isThreadInList(m_registeredThreads, result));
-#endif
+ PlatformThread platformThread = getCurrentPlatformThread();
+ for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
+ if (*thread == platformThread)
+ return thread;
+ }
- return result;
+ RELEASE_ASSERT_NOT_REACHED();
+ return nullptr;
}
void MachineThreads::removeThread(void* p)
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (199725 => 199726)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2016-04-19 14:11:19 UTC (rev 199726)
@@ -159,7 +159,6 @@
Lock m_registeredThreadsMutex;
Thread* m_registeredThreads;
WTF::ThreadSpecificKey m_threadSpecificForMachineThreads;
- WTF::ThreadSpecificKey m_threadSpecificForThread;
#if !ASSERT_DISABLED
Heap* m_heap;
#endif
Modified: trunk/Source/WTF/ChangeLog (199725 => 199726)
--- trunk/Source/WTF/ChangeLog 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/WTF/ChangeLog 2016-04-19 14:11:19 UTC (rev 199726)
@@ -1,3 +1,63 @@
+2016-04-19 Michael Saboff <msab...@apple.com>
+
+ iTunes crashing _javascript_Core.dll
+ https://bugs.webkit.org/show_bug.cgi?id=156647
+
+ Reviewed by Saam Barati.
+
+ 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 Windows 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 there 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-19 Yusuke Suzuki <utatane....@gmail.com>
[GTK] Use Generic WorkQueue instead of WorkQueueGLib
Modified: trunk/Source/WTF/wtf/ThreadSpecific.h (199725 => 199726)
--- trunk/Source/WTF/wtf/ThreadSpecific.h 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/WTF/wtf/ThreadSpecific.h 2016-04-19 14:11:19 UTC (rev 199726)
@@ -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 = 128;
-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 (199725 => 199726)
--- trunk/Source/WTF/wtf/ThreadSpecificWin.cpp 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/WTF/wtf/ThreadSpecificWin.cpp 2016-04-19 14:11:19 UTC (rev 199726)
@@ -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 (199725 => 199726)
--- trunk/Source/WTF/wtf/ThreadingWin.cpp 2016-04-19 13:34:02 UTC (rev 199725)
+++ trunk/Source/WTF/wtf/ThreadingWin.cpp 2016-04-19 14:11:19 UTC (rev 199726)
@@ -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;
}
