Diff
Modified: trunk/Source/WTF/ChangeLog (252686 => 252687)
--- trunk/Source/WTF/ChangeLog 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/ChangeLog 2019-11-20 16:04:52 UTC (rev 252687)
@@ -1,3 +1,42 @@
+2019-11-20 Fujii Hironori <hironori.fu...@sony.com>
+
+ [Win] Implement WTF::ThreadSpecific in WTF::Thread
+ https://bugs.webkit.org/show_bug.cgi?id=204341
+
+ Reviewed by Brent Fulgham and Yusuke Suzuki.
+
+ Thread::destructTLS had a tricky code to defer destroying
+ WTF::Thread in TLS in order to ensure WTF::Thread is destructed
+ after other ThreadSpecific are destructed, which is a part of
+ cause of nasty hanging issue in the process termination (Bug 204192).
+
+ This change implements WTF::ThreadSpecific in WTF::Thread by
+ adding a new class Thread::SpecificStorage to manage TLS. Simplify
+ Thread::destructTLS. Remove threadMapMutex in ThreadingWin.cpp
+
+ * wtf/PlatformWin.cmake:
+ * wtf/ThreadSpecific.h:
+ (WTF::canBeGCThread>::ThreadSpecific):
+ (WTF::canBeGCThread>::get):
+ (WTF::canBeGCThread>::setInTLS):
+ (WTF::canBeGCThread>::destroy):
+ (WTF::canBeGCThread>::~ThreadSpecific): Deleted.
+ * wtf/Threading.h:
+ (WTF::Thread::specificStorage):
+ (WTF::Thread::current):
+ * wtf/win/ThreadSpecificWin.cpp: Removed.
+ * wtf/win/ThreadingWin.cpp:
+ (WTF::Thread::initializeTLSKey):
+ (WTF::Thread::initializeTLS):
+ (WTF::Thread::destructTLS):
+ (WTF::Thread::SpecificStorage::allocateKey):
+ (WTF::Thread::SpecificStorage::get):
+ (WTF::Thread::SpecificStorage::set):
+ (WTF::Thread::SpecificStorage::destroySlots):
+ (): Deleted.
+ (WTF::Thread::currentDying): Deleted.
+ (WTF::Thread::get): Deleted.
+
2019-11-19 Ross Kirsling <ross.kirsl...@sony.com>
Implement String.prototype.replaceAll
Modified: trunk/Source/WTF/wtf/PlatformWin.cmake (252686 => 252687)
--- trunk/Source/WTF/wtf/PlatformWin.cmake 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/wtf/PlatformWin.cmake 2019-11-20 16:04:52 UTC (rev 252687)
@@ -21,7 +21,6 @@
win/OSAllocatorWin.cpp
win/PathWalker.cpp
win/RunLoopWin.cpp
- win/ThreadSpecificWin.cpp
win/ThreadingWin.cpp
win/WorkQueueWin.cpp
)
Modified: trunk/Source/WTF/wtf/ThreadSpecific.h (252686 => 252687)
--- trunk/Source/WTF/wtf/ThreadSpecific.h 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/wtf/ThreadSpecific.h 2019-11-20 16:04:52 UTC (rev 252687)
@@ -100,12 +100,12 @@
T* get();
T* set();
void setInTLS(Data*);
- void static THREAD_SPECIFIC_CALL destroy(void* ptr);
+ void static destroy(void* ptr);
#if USE(PTHREADS)
pthread_key_t m_key { };
#elif OS(WINDOWS)
- int m_index;
+ int m_key;
#endif
};
@@ -136,47 +136,28 @@
#elif OS(WINDOWS)
-// 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.
-static constexpr int maxFlsKeySize = 128;
-
-WTF_EXPORT_PRIVATE long& flsKeyCount();
-WTF_EXPORT_PRIVATE DWORD* flsKeys();
-
template<typename T, CanBeGCThread canBeGCThread>
inline ThreadSpecific<T, canBeGCThread>::ThreadSpecific()
- : m_index(-1)
+ : m_key(-1)
{
- DWORD flsKey = FlsAlloc(destroy);
- if (flsKey == FLS_OUT_OF_INDEXES)
+ bool ok = Thread::SpecificStorage::allocateKey(m_key, destroy);
+ if (!ok)
CRASH();
-
- m_index = InterlockedIncrement(&flsKeyCount()) - 1;
- if (m_index >= maxFlsKeySize)
- CRASH();
- flsKeys()[m_index] = flsKey;
}
template<typename T, CanBeGCThread canBeGCThread>
-inline ThreadSpecific<T, canBeGCThread>::~ThreadSpecific()
-{
- FlsFree(flsKeys()[m_index]);
-}
-
-template<typename T, CanBeGCThread canBeGCThread>
inline T* ThreadSpecific<T, canBeGCThread>::get()
{
- Data* data = ""
- if (data)
- return data->storagePointer();
- return nullptr;
+ auto data = ""
+ if (!data)
+ return nullptr;
+ return data->storagePointer();
}
template<typename T, CanBeGCThread canBeGCThread>
inline void ThreadSpecific<T, canBeGCThread>::setInTLS(Data* data)
{
- FlsSetValue(flsKeys()[m_index], data);
+ return Thread::current().specificStorage().set(m_key, data);
}
#else
@@ -184,7 +165,7 @@
#endif
template<typename T, CanBeGCThread canBeGCThread>
-inline void THREAD_SPECIFIC_CALL ThreadSpecific<T, canBeGCThread>::destroy(void* ptr)
+inline void ThreadSpecific<T, canBeGCThread>::destroy(void* ptr)
{
Data* data = ""
Modified: trunk/Source/WTF/wtf/Threading.h (252686 => 252687)
--- trunk/Source/WTF/wtf/Threading.h 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/wtf/Threading.h 2019-11-20 16:04:52 UTC (rev 252687)
@@ -52,6 +52,10 @@
#include <signal.h>
#endif
+#if OS(WINDOWS)
+#include <array>
+#endif
+
namespace WTF {
class AbstractLocker;
@@ -106,6 +110,23 @@
WTF_EXPORT_PRIVATE static ThreadIdentifier currentID();
ThreadIdentifier id() const { return m_id; }
+
+ class SpecificStorage {
+ public:
+ using DestroyFunction = void (*)(void*);
+ WTF_EXPORT_PRIVATE static bool allocateKey(int& key, DestroyFunction);
+ WTF_EXPORT_PRIVATE void* get(int key);
+ WTF_EXPORT_PRIVATE void set(int key, void* value);
+ void destroySlots();
+
+ private:
+ static constexpr size_t s_maxKeys = 32;
+ static Atomic<int> s_numberOfKeys;
+ static std::array<Atomic<DestroyFunction>, s_maxKeys> s_destroyFunctions;
+ std::array<void*, s_maxKeys> m_slots { };
+ };
+
+ SpecificStorage& specificStorage() { return m_specificStorage; };
#endif
WTF_EXPORT_PRIVATE void changePriority(int);
@@ -269,16 +290,11 @@
// Returns nullptr if thread-specific storage was not initialized.
static Thread* currentMayBeNull();
-#if OS(WINDOWS)
- WTF_EXPORT_PRIVATE static Thread* currentDying();
- static RefPtr<Thread> get(ThreadIdentifier);
-#endif
-
// This thread-specific destructor is called 2 times when thread terminates:
// - first, when all the other thread-specific destructors are called, it simply remembers it was 'destroyed once'
// and (1) re-sets itself into the thread-specific slot or (2) constructs thread local value to call it again later.
- // - second, after all thread-specific destructors were invoked, it gets called again - this time, we remove the
- // Thread from the threadMap, completing the cleanup.
+ // - second, after all thread-specific destructors were invoked, it gets called again - this time, we deref the
+ // Thread in the TLS, completing the cleanup.
static void THREAD_SPECIFIC_CALL destructTLS(void* data);
JoinableState m_joinableState { Joinable };
@@ -303,6 +319,10 @@
unsigned m_suspendCount { 0 };
#endif
+#if OS(WINDOWS)
+ SpecificStorage m_specificStorage;
+#endif
+
AtomStringTable* m_currentAtomStringTable { nullptr };
AtomStringTable m_defaultAtomStringTable;
@@ -348,10 +368,6 @@
#endif
if (auto* thread = currentMayBeNull())
return *thread;
-#if OS(WINDOWS)
- if (auto* thread = currentDying())
- return *thread;
-#endif
return initializeCurrentTLS();
}
Deleted: trunk/Source/WTF/wtf/win/ThreadSpecificWin.cpp (252686 => 252687)
--- trunk/Source/WTF/wtf/win/ThreadSpecificWin.cpp 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/wtf/win/ThreadSpecificWin.cpp 2019-11-20 16:04:52 UTC (rev 252687)
@@ -1,43 +0,0 @@
-/*
- * Copyright (C) 2009 Jian Li <jia...@chromium.org>
- * Copyright (C) 2012 Patrick Gansterer <par...@paroga.com>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB. If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- *
- */
-
-#include "config.h"
-#include <wtf/ThreadSpecific.h>
-
-#if !USE(PTHREADS)
-
-namespace WTF {
-
-long& flsKeyCount()
-{
- static long count;
- return count;
-}
-
-DWORD* flsKeys()
-{
- static DWORD keys[maxFlsKeySize];
- return keys;
-}
-
-} // namespace WTF
-
-#endif // !USE(PTHREADS)
Modified: trunk/Source/WTF/wtf/win/ThreadingWin.cpp (252686 => 252687)
--- trunk/Source/WTF/wtf/win/ThreadingWin.cpp 2019-11-20 15:18:55 UTC (rev 252686)
+++ trunk/Source/WTF/wtf/win/ThreadingWin.cpp 2019-11-20 16:04:52 UTC (rev 252687)
@@ -265,37 +265,11 @@
#define InvalidThread reinterpret_cast<Thread*>(static_cast<uintptr_t>(0xbbadbeef))
-static WordLock threadMapMutex;
-
-static HashMap<ThreadIdentifier, Thread*>& threadMap()
-{
- static NeverDestroyed<HashMap<ThreadIdentifier, Thread*>> map;
- return map.get();
-}
-
void Thread::initializeTLSKey()
{
- threadMap();
threadSpecificKeyCreate(&s_key, destructTLS);
}
-Thread* Thread::currentDying()
-{
- ASSERT(s_key != InvalidThreadSpecificKey);
- // After FLS is destroyed, this map offers the value until the second thread exit callback is called.
- auto locker = holdLock(threadMapMutex);
- return threadMap().get(currentID());
-}
-
-RefPtr<Thread> Thread::get(ThreadIdentifier id)
-{
- auto locker = holdLock(threadMapMutex);
- Thread* thread = threadMap().get(id);
- if (thread)
- return thread;
- return nullptr;
-}
-
Thread& Thread::initializeTLS(Ref<Thread>&& thread)
{
ASSERT(s_key != InvalidThreadSpecificKey);
@@ -304,10 +278,6 @@
// We leak the ref to keep the Thread alive while it is held in TLS. destructTLS will deref it later at thread destruction time.
auto& threadInTLS = thread.leakRef();
threadSpecificSet(s_key, &threadInTLS);
- {
- auto locker = holdLock(threadMapMutex);
- threadMap().add(id, &threadInTLS);
- }
return threadInTLS;
}
@@ -319,41 +289,51 @@
Thread* thread = static_cast<Thread*>(data);
ASSERT(thread);
- // Delay the deallocation of Thread more.
- // It defers Thread deallocation after the other ThreadSpecific values are deallocated.
- static thread_local class ThreadExitCallback {
- public:
- ThreadExitCallback(Thread* thread)
- : m_thread(thread)
- {
- }
+ thread->specificStorage().destroySlots();
+ thread->didExit();
+ thread->deref();
- ~ThreadExitCallback()
- {
- Thread::destructTLS(m_thread);
- }
+ // Fill the FLS with the non-nullptr value. While FLS destructor won't be called for that,
+ // non-nullptr value tells us that we already destructed Thread. This allows us to
+ // detect incorrect use of Thread::current() after this point because it will crash.
+ threadSpecificSet(s_key, InvalidThread);
+}
- private:
- Thread* m_thread;
- } callback(thread);
+Atomic<int> Thread::SpecificStorage::s_numberOfKeys;
+std::array<Atomic<Thread::SpecificStorage::DestroyFunction>, Thread::SpecificStorage::s_maxKeys> Thread::SpecificStorage::s_destroyFunctions;
- if (thread->m_isDestroyedOnce) {
- {
- auto locker = holdLock(threadMapMutex);
- ASSERT(threadMap().contains(thread->id()));
- threadMap().remove(thread->id());
+bool Thread::SpecificStorage::allocateKey(int& key, DestroyFunction destroy)
+{
+ int k = s_numberOfKeys.exchangeAdd(1);
+ if (k >= s_maxKeys) {
+ s_numberOfKeys.exchangeSub(1);
+ return false;
+ }
+ key = k;
+ s_destroyFunctions[key].store(destroy);
+ return true;
+}
+
+void* Thread::SpecificStorage::get(int key)
+{
+ return m_slots[key];
+}
+
+void Thread::SpecificStorage::set(int key, void* value)
+{
+ m_slots[key] = value;
+}
+
+void Thread::SpecificStorage::destroySlots()
+{
+ auto numberOfKeys = s_numberOfKeys.load();
+ for (size_t i = 0; i < numberOfKeys; i++) {
+ auto destroy = s_destroyFunctions[i].load();
+ if (destroy && m_slots[i]) {
+ destroy(m_slots[i]);
+ m_slots[i] = nullptr;
}
- thread->didExit();
- thread->deref();
-
- // Fill the FLS with the non-nullptr value. While FLS destructor won't be called for that,
- // non-nullptr value tells us that we already destructed Thread. This allows us to
- // detect incorrect use of Thread::current() after this point because it will crash.
- threadSpecificSet(s_key, InvalidThread);
- return;
}
- threadSpecificSet(s_key, InvalidThread);
- thread->m_isDestroyedOnce = true;
}
Mutex::~Mutex()
