Title: [221425] trunk/Source/WTF
- Revision
- 221425
- Author
- [email protected]
- Date
- 2017-08-31 11:15:48 -0700 (Thu, 31 Aug 2017)
Log Message
[Win] Crash under WorkQueue::performWorkOnRegisteredWorkThread in layout tests.
https://bugs.webkit.org/show_bug.cgi?id=176163
Reviewed by Alex Christensen.
My previous attempt at fixing this crash in <http://trac.webkit.org/changeset/221323>
was incorrect, since it is still crashing on the bot(s). The current theory of why this
is failing is that the WorkQueue object deletes itself in the middle of the
performWorkOnRegisteredWorkThread method when calling deref(). There is no need to
increase the reference count of the work queue for each function we want to call on the
work thread. It is sufficient to increase it for every work thread we start, and then
dereference it when the thread ends. We should also not attempt to access members after
the deref() call, which can potentially be unsafe.
* wtf/win/WorkQueueWin.cpp:
(WTF::WorkQueue::workThreadCallback):
(WTF::WorkQueue::performWorkOnRegisteredWorkThread):
(WTF::WorkQueue::dispatch):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (221424 => 221425)
--- trunk/Source/WTF/ChangeLog 2017-08-31 18:09:01 UTC (rev 221424)
+++ trunk/Source/WTF/ChangeLog 2017-08-31 18:15:48 UTC (rev 221425)
@@ -1,3 +1,24 @@
+2017-08-31 Per Arne Vollan <[email protected]>
+
+ [Win] Crash under WorkQueue::performWorkOnRegisteredWorkThread in layout tests.
+ https://bugs.webkit.org/show_bug.cgi?id=176163
+
+ Reviewed by Alex Christensen.
+
+ My previous attempt at fixing this crash in <http://trac.webkit.org/changeset/221323>
+ was incorrect, since it is still crashing on the bot(s). The current theory of why this
+ is failing is that the WorkQueue object deletes itself in the middle of the
+ performWorkOnRegisteredWorkThread method when calling deref(). There is no need to
+ increase the reference count of the work queue for each function we want to call on the
+ work thread. It is sufficient to increase it for every work thread we start, and then
+ dereference it when the thread ends. We should also not attempt to access members after
+ the deref() call, which can potentially be unsafe.
+
+ * wtf/win/WorkQueueWin.cpp:
+ (WTF::WorkQueue::workThreadCallback):
+ (WTF::WorkQueue::performWorkOnRegisteredWorkThread):
+ (WTF::WorkQueue::dispatch):
+
2017-08-22 Filip Pizlo <[email protected]>
Strings need to be in some kind of gigacage
Modified: trunk/Source/WTF/wtf/win/WorkQueueWin.cpp (221424 => 221425)
--- trunk/Source/WTF/wtf/win/WorkQueueWin.cpp 2017-08-31 18:09:01 UTC (rev 221424)
+++ trunk/Source/WTF/wtf/win/WorkQueueWin.cpp 2017-08-31 18:15:48 UTC (rev 221425)
@@ -37,10 +37,11 @@
WorkQueue* queue = static_cast<WorkQueue*>(context);
- if (!queue->tryRegisterAsWorkThread())
- return 0;
+ if (queue->tryRegisterAsWorkThread())
+ queue->performWorkOnRegisteredWorkThread();
- queue->performWorkOnRegisteredWorkThread();
+ queue->deref();
+
return 0;
}
@@ -56,10 +57,8 @@
// Allow more work to be scheduled while we're not using the queue directly.
m_functionQueueLock.unlock();
- for (auto& function : functionQueue) {
+ for (auto& function : functionQueue)
function();
- deref();
- }
m_functionQueueLock.lock();
}
@@ -100,14 +99,7 @@
void WorkQueue::dispatch(Function<void()>&& function)
{
- // FIXME: During layout tests, this method is sometimes called with a nullptr function parameter.
- // This is tracked in <http://webkit.org/b/176072>.
- ASSERT(function);
- if (!function)
- return;
-
MutexLocker locker(m_functionQueueLock);
- ref();
m_functionQueue.append(WTFMove(function));
// Spawn a work thread to perform the work we just added. As an optimization, we avoid
@@ -116,8 +108,10 @@
// hasn't registered itself yet, m_isWorkThreadRegistered will be false and we'll end up
// spawning a second work thread here. But work thread registration process will ensure that
// only one thread actually ends up performing work.)
- if (!m_isWorkThreadRegistered)
+ if (!m_isWorkThreadRegistered) {
+ ref();
::QueueUserWorkItem(workThreadCallback, this, WT_EXECUTEDEFAULT);
+ }
}
struct TimerContext : public ThreadSafeRefCounted<TimerContext> {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
