Title: [248586] trunk
- Revision
- 248586
- Author
- [email protected]
- Date
- 2019-08-12 20:01:11 -0700 (Mon, 12 Aug 2019)
Log Message
[WTF] Thread::removeFromThreadGroup leaks weak pointers.
https://bugs.webkit.org/show_bug.cgi?id=199857
Patch by Takashi Komori <[email protected]> on 2019-08-12
Reviewed by Yusuke Suzuki.
Source/WTF:
Fix leaking of ThreadGroup's weak pointers.
Tests: WTF.ThreadGroupRemove API tests
* wtf/Threading.cpp:
(WTF::Thread::didExit):
(WTF::Thread::addToThreadGroup):
(WTF::Thread::removeFromThreadGroup):
(WTF::Thread::numberOfThreadGroups):
* wtf/Threading.h:
Tools:
* TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
(TestWebKitAPI::countThreadGroups):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (248585 => 248586)
--- trunk/Source/WTF/ChangeLog 2019-08-13 01:23:20 UTC (rev 248585)
+++ trunk/Source/WTF/ChangeLog 2019-08-13 03:01:11 UTC (rev 248586)
@@ -1,3 +1,21 @@
+2019-08-12 Takashi Komori <[email protected]>
+
+ [WTF] Thread::removeFromThreadGroup leaks weak pointers.
+ https://bugs.webkit.org/show_bug.cgi?id=199857
+
+ Reviewed by Yusuke Suzuki.
+
+ Fix leaking of ThreadGroup's weak pointers.
+
+ Tests: WTF.ThreadGroupRemove API tests
+
+ * wtf/Threading.cpp:
+ (WTF::Thread::didExit):
+ (WTF::Thread::addToThreadGroup):
+ (WTF::Thread::removeFromThreadGroup):
+ (WTF::Thread::numberOfThreadGroups):
+ * wtf/Threading.h:
+
2019-08-12 Sam Weinig <[email protected]>
Replace multiparameter overloads of append() in StringBuilder as a first step toward standardizinging on the flexibleAppend() implementation
Modified: trunk/Source/WTF/wtf/Threading.cpp (248585 => 248586)
--- trunk/Source/WTF/wtf/Threading.cpp 2019-08-13 01:23:20 UTC (rev 248585)
+++ trunk/Source/WTF/wtf/Threading.cpp 2019-08-13 03:01:11 UTC (rev 248586)
@@ -211,10 +211,10 @@
Vector<std::shared_ptr<ThreadGroup>> threadGroups;
{
auto locker = holdLock(m_mutex);
- for (auto& threadGroup : m_threadGroups) {
+ for (auto& threadGroupPointerPair : m_threadGroupMap) {
// If ThreadGroup is just being destroyed,
// we do not need to perform unregistering.
- if (auto retained = threadGroup.lock())
+ if (auto retained = threadGroupPointerPair.value.lock())
threadGroups.append(WTFMove(retained));
}
m_isShuttingDown = true;
@@ -240,7 +240,7 @@
if (m_isShuttingDown)
return ThreadGroupAddResult::NotAdded;
if (threadGroup.m_threads.add(*this).isNewEntry) {
- m_threadGroups.append(threadGroup.weakFromThis());
+ m_threadGroupMap.add(&threadGroup, threadGroup.weakFromThis());
return ThreadGroupAddResult::NewlyAdded;
}
return ThreadGroupAddResult::AlreadyAdded;
@@ -252,13 +252,15 @@
auto locker = holdLock(m_mutex);
if (m_isShuttingDown)
return;
- m_threadGroups.removeFirstMatching([&] (auto weakPtr) {
- if (auto sharedPtr = weakPtr.lock())
- return sharedPtr.get() == &threadGroup;
- return false;
- });
+ m_threadGroupMap.remove(&threadGroup);
}
+unsigned Thread::numberOfThreadGroups()
+{
+ auto locker = holdLock(m_mutex);
+ return m_threadGroupMap.size();
+}
+
bool Thread::exchangeIsCompilationThread(bool newValue)
{
auto& thread = Thread::current();
Modified: trunk/Source/WTF/wtf/Threading.h (248585 => 248586)
--- trunk/Source/WTF/wtf/Threading.h 2019-08-13 01:23:20 UTC (rev 248585)
+++ trunk/Source/WTF/wtf/Threading.h 2019-08-13 03:01:11 UTC (rev 248586)
@@ -36,6 +36,7 @@
#include <wtf/Expected.h>
#include <wtf/FastTLS.h>
#include <wtf/Function.h>
+#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
#include <wtf/PlatformRegisters.h>
#include <wtf/Ref.h>
@@ -95,6 +96,8 @@
WTF_EXPORT_PRIVATE static HashSet<Thread*>& allThreads(const LockHolder&);
WTF_EXPORT_PRIVATE static Lock& allThreadsMutex();
+ WTF_EXPORT_PRIVATE unsigned numberOfThreadGroups();
+
#if OS(WINDOWS)
// Returns ThreadIdentifier directly. It is useful if the user only cares about identity
// of threads. At that time, users should know that holding this ThreadIdentifier does not ensure
@@ -290,7 +293,7 @@
// Use WordLock since WordLock does not depend on ThreadSpecific and this "Thread".
WordLock m_mutex;
StackBounds m_stack { StackBounds::emptyBounds() };
- Vector<std::weak_ptr<ThreadGroup>> m_threadGroups;
+ HashMap<ThreadGroup*, std::weak_ptr<ThreadGroup>> m_threadGroupMap;
PlatformThreadHandle m_handle;
#if OS(WINDOWS)
ThreadIdentifier m_id { 0 };
Modified: trunk/Tools/ChangeLog (248585 => 248586)
--- trunk/Tools/ChangeLog 2019-08-13 01:23:20 UTC (rev 248585)
+++ trunk/Tools/ChangeLog 2019-08-13 03:01:11 UTC (rev 248586)
@@ -1,3 +1,14 @@
+2019-08-12 Takashi Komori <[email protected]>
+
+ [WTF] Thread::removeFromThreadGroup leaks weak pointers.
+ https://bugs.webkit.org/show_bug.cgi?id=199857
+
+ Reviewed by Yusuke Suzuki.
+
+ * TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
+ (TestWebKitAPI::countThreadGroups):
+ (TestWebKitAPI::TEST):
+
2019-08-12 Alexey Shvayka <[email protected]>
AX: Homebrew is not allowed to run any script under sudo.
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp (248585 => 248586)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp 2019-08-13 01:23:20 UTC (rev 248585)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp 2019-08-13 03:01:11 UTC (rev 248586)
@@ -142,4 +142,47 @@
}
}
+static unsigned countThreadGroups(Vector<Ref<Thread>>& threads)
+{
+ unsigned count = 0;
+ for (auto& thread : threads)
+ count += thread->numberOfThreadGroups();
+
+ return count;
+}
+
+TEST(WTF, ThreadGroupRemove)
+{
+ const unsigned NumberOfThreads = 64;
+
+ Lock lock;
+ Condition threadRunningCondition;
+ bool threadRunning = true;
+
+ Vector<Ref<Thread>> threads;
+
+ auto threadGroup = ThreadGroup::create();
+ for (unsigned i = 0; i < NumberOfThreads; i++) {
+ auto thread = Thread::create("ThreadGroupWorker", [&]() {
+ auto locker = holdLock(lock);
+ threadRunningCondition.wait(lock, [&]() {
+ return !threadRunning;
+ });
+ });
+ threadGroup->add(thread.get());
+ threads.append(WTFMove(thread));
+ }
+
+ EXPECT_EQ(NumberOfThreads, countThreadGroups(threads));
+
+ threadGroup = nullptr;
+
+ EXPECT_EQ(0U, countThreadGroups(threads));
+
+ threadRunning = false;
+ threadRunningCondition.notifyAll();
+ for (unsigned i = 0; i < NumberOfThreads; i++)
+ threads[i]->waitForCompletion();
+}
+
} // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
