Title: [261520] branches/safari-609-branch/Source
- Revision
- 261520
- Author
- [email protected]
- Date
- 2020-05-11 17:21:56 -0700 (Mon, 11 May 2020)
Log Message
Cherry-pick r260682. rdar://problem/62978266
[WTF] allThreads registration is racy with allThreads unregistration
https://bugs.webkit.org/show_bug.cgi?id=210995
<rdar://problem/61609690>
Reviewed by Keith Miller.
Source/WebCore:
* page/cocoa/ResourceUsageThreadCocoa.mm:
(WebCore::ResourceUsageThread::platformCollectCPUData):
Source/WTF:
There is a race between registering a thread to allThreads and unregistering a thread from allThreads.
1. Caller: A new thread is created, but not registering it to allThreads yet.
2. Thread: The thread is running.
3. Thread: The thread finishes its execution before the thread is registered into allThreads.
4. Thread: The thread unregisters itself from allThreads.
5. Caller: Registers the new thread to allThreads after it already finished its execution.
6. The thread is never removed from allThreads.
This patch adds m_didUnregisterFromAllThreads flag to Thread, and add the thread to allThreads only when this flag is false.
Covered by LayoutTests/inspector/cpu-profiler/threads.html.
* wtf/Threading.cpp:
(WTF::Thread::create):
(WTF::Thread::didExit):
* wtf/Threading.h:
(WTF::Thread::Thread):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260682 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-609-branch/Source/WTF/ChangeLog (261519 => 261520)
--- branches/safari-609-branch/Source/WTF/ChangeLog 2020-05-12 00:21:53 UTC (rev 261519)
+++ branches/safari-609-branch/Source/WTF/ChangeLog 2020-05-12 00:21:56 UTC (rev 261520)
@@ -1,3 +1,68 @@
+2020-05-07 Russell Epstein <[email protected]>
+
+ Cherry-pick r260682. rdar://problem/62978266
+
+ [WTF] allThreads registration is racy with allThreads unregistration
+ https://bugs.webkit.org/show_bug.cgi?id=210995
+ <rdar://problem/61609690>
+
+ Reviewed by Keith Miller.
+
+ Source/WebCore:
+
+ * page/cocoa/ResourceUsageThreadCocoa.mm:
+ (WebCore::ResourceUsageThread::platformCollectCPUData):
+
+ Source/WTF:
+
+ There is a race between registering a thread to allThreads and unregistering a thread from allThreads.
+
+ 1. Caller: A new thread is created, but not registering it to allThreads yet.
+ 2. Thread: The thread is running.
+ 3. Thread: The thread finishes its execution before the thread is registered into allThreads.
+ 4. Thread: The thread unregisters itself from allThreads.
+ 5. Caller: Registers the new thread to allThreads after it already finished its execution.
+ 6. The thread is never removed from allThreads.
+
+ This patch adds m_didUnregisterFromAllThreads flag to Thread, and add the thread to allThreads only when this flag is false.
+
+ Covered by LayoutTests/inspector/cpu-profiler/threads.html.
+
+ * wtf/Threading.cpp:
+ (WTF::Thread::create):
+ (WTF::Thread::didExit):
+ * wtf/Threading.h:
+ (WTF::Thread::Thread):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260682 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-04-24 Yusuke Suzuki <[email protected]>
+
+ [WTF] allThreads registration is racy with allThreads unregistration
+ https://bugs.webkit.org/show_bug.cgi?id=210995
+ <rdar://problem/61609690>
+
+ Reviewed by Keith Miller.
+
+ There is a race between registering a thread to allThreads and unregistering a thread from allThreads.
+
+ 1. Caller: A new thread is created, but not registering it to allThreads yet.
+ 2. Thread: The thread is running.
+ 3. Thread: The thread finishes its execution before the thread is registered into allThreads.
+ 4. Thread: The thread unregisters itself from allThreads.
+ 5. Caller: Registers the new thread to allThreads after it already finished its execution.
+ 6. The thread is never removed from allThreads.
+
+ This patch adds m_didUnregisterFromAllThreads flag to Thread, and add the thread to allThreads only when this flag is false.
+
+ Covered by LayoutTests/inspector/cpu-profiler/threads.html.
+
+ * wtf/Threading.cpp:
+ (WTF::Thread::create):
+ (WTF::Thread::didExit):
+ * wtf/Threading.h:
+ (WTF::Thread::Thread):
+
2020-04-16 Alan Coon <[email protected]>
Cherry-pick r259980. rdar://problem/61888315
Modified: branches/safari-609-branch/Source/WTF/wtf/Threading.cpp (261519 => 261520)
--- branches/safari-609-branch/Source/WTF/wtf/Threading.cpp 2020-05-12 00:21:53 UTC (rev 261519)
+++ branches/safari-609-branch/Source/WTF/wtf/Threading.cpp 2020-05-12 00:21:56 UTC (rev 261520)
@@ -176,9 +176,14 @@
#endif
}
+ // We must register threads here since threads registered in allThreads are expected to have complete thread data which can be initialized in launched thread side.
+ // However, it is also possible that the launched thread has finished its execution before it is registered in allThreads here! In this case, the thread has already
+ // called Thread::didExit to unregister itself from allThreads. Registering such a thread will register a stale thread pointer to allThreads, which will not be removed
+ // even after Thread is destroyed. Register a thread only when it has not unregistered itself from allThreads yet.
{
- LockHolder lock(allThreadsMutex());
- allThreads(lock).add(&thread.get());
+ auto locker = holdLock(allThreadsMutex());
+ if (!thread->m_didUnregisterFromAllThreads)
+ allThreads(locker).add(thread.ptr());
}
ASSERT(!thread->stack().isEmpty());
@@ -202,8 +207,9 @@
void Thread::didExit()
{
{
- LockHolder lock(allThreadsMutex());
- allThreads(lock).remove(this);
+ auto locker = holdLock(allThreadsMutex());
+ allThreads(locker).remove(this);
+ m_didUnregisterFromAllThreads = true;
}
if (shouldRemoveThreadFromThreadGroup()) {
Modified: branches/safari-609-branch/Source/WTF/wtf/Threading.h (261519 => 261520)
--- branches/safari-609-branch/Source/WTF/wtf/Threading.h 2020-05-12 00:21:53 UTC (rev 261519)
+++ branches/safari-609-branch/Source/WTF/wtf/Threading.h 2020-05-12 00:21:56 UTC (rev 261520)
@@ -307,6 +307,8 @@
bool m_isCompilationThread: 1;
unsigned m_gcThreadType : 2;
+ bool m_didUnregisterFromAllThreads { false };
+
// Lock & ParkingLot rely on ThreadSpecific. But Thread object can be destroyed even after ThreadSpecific things are destroyed.
// Use WordLock since WordLock does not depend on ThreadSpecific and this "Thread".
WordLock m_mutex;
Modified: branches/safari-609-branch/Source/WebCore/ChangeLog (261519 => 261520)
--- branches/safari-609-branch/Source/WebCore/ChangeLog 2020-05-12 00:21:53 UTC (rev 261519)
+++ branches/safari-609-branch/Source/WebCore/ChangeLog 2020-05-12 00:21:56 UTC (rev 261520)
@@ -1,5 +1,54 @@
2020-05-07 Russell Epstein <[email protected]>
+ Cherry-pick r260682. rdar://problem/62978266
+
+ [WTF] allThreads registration is racy with allThreads unregistration
+ https://bugs.webkit.org/show_bug.cgi?id=210995
+ <rdar://problem/61609690>
+
+ Reviewed by Keith Miller.
+
+ Source/WebCore:
+
+ * page/cocoa/ResourceUsageThreadCocoa.mm:
+ (WebCore::ResourceUsageThread::platformCollectCPUData):
+
+ Source/WTF:
+
+ There is a race between registering a thread to allThreads and unregistering a thread from allThreads.
+
+ 1. Caller: A new thread is created, but not registering it to allThreads yet.
+ 2. Thread: The thread is running.
+ 3. Thread: The thread finishes its execution before the thread is registered into allThreads.
+ 4. Thread: The thread unregisters itself from allThreads.
+ 5. Caller: Registers the new thread to allThreads after it already finished its execution.
+ 6. The thread is never removed from allThreads.
+
+ This patch adds m_didUnregisterFromAllThreads flag to Thread, and add the thread to allThreads only when this flag is false.
+
+ Covered by LayoutTests/inspector/cpu-profiler/threads.html.
+
+ * wtf/Threading.cpp:
+ (WTF::Thread::create):
+ (WTF::Thread::didExit):
+ * wtf/Threading.h:
+ (WTF::Thread::Thread):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@260682 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-04-24 Yusuke Suzuki <[email protected]>
+
+ [WTF] allThreads registration is racy with allThreads unregistration
+ https://bugs.webkit.org/show_bug.cgi?id=210995
+ <rdar://problem/61609690>
+
+ Reviewed by Keith Miller.
+
+ * page/cocoa/ResourceUsageThreadCocoa.mm:
+ (WebCore::ResourceUsageThread::platformCollectCPUData):
+
+2020-05-07 Russell Epstein <[email protected]>
+
Cherry-pick r260598. rdar://problem/62978929
Allow credentials for same-origin css mask images
Modified: branches/safari-609-branch/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm (261519 => 261520)
--- branches/safari-609-branch/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm 2020-05-12 00:21:53 UTC (rev 261519)
+++ branches/safari-609-branch/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm 2020-05-12 00:21:56 UTC (rev 261520)
@@ -156,8 +156,8 @@
HashSet<mach_port_t> knownWebKitThreads;
{
- LockHolder lock(Thread::allThreadsMutex());
- for (auto* thread : Thread::allThreads(lock)) {
+ auto locker = holdLock(Thread::allThreadsMutex());
+ for (auto* thread : Thread::allThreads(locker)) {
mach_port_t machThread = thread->machThread();
if (machThread != MACH_PORT_NULL)
knownWebKitThreads.add(machThread);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
