- Revision
- 240042
- Author
- you...@apple.com
- Date
- 2019-01-16 11:07:22 -0800 (Wed, 16 Jan 2019)
Log Message
Prevent WorkerRunLoop::runInMode from spinning in nested cases
https://bugs.webkit.org/show_bug.cgi?id=193359
<rdar://problem/46345353>
Reviewed by Joseph Pecoraro.
Speculative fix for some cases where service worker is spinning and consuming a lot of CPU.
The hypothesis is that:
- Service Worker is checking for its script freshness through WorkerScriptLoader.
This triggers the worker run loop to be nested.
- The run loop timer is active and needs to fire immediately.
The hypothesis is that this happens in some cases like restarting a device after sleep mode.
WorkerRunLoop::runInMode will then compute a 0 timeout value for getting a message.
This will trigger a timeout while waiting for the message queue.
Since the run loop is nested, the run loop timer will not be able to fire,
and it will keep ask to fire immediately.
runInMode will return timeout as a result and WorkerRunLoop::run will call it immediately.
The fix is to prevent the shared timer to fire only when the run loop is being debugged through the web inspector.
We compute this by checking the run loop mode as debuggerMode().
Did some refactoring by introducing helper routines for running the loop and posting task in debugger mode.
* inspector/WorkerScriptDebugServer.cpp:
(WebCore::WorkerScriptDebugServer::runEventLoopWhilePaused):
* workers/WorkerInspectorProxy.cpp:
(WebCore::WorkerInspectorProxy::resumeWorkerIfPaused):
(WebCore::WorkerInspectorProxy::connectToWorkerInspectorController):
(WebCore::WorkerInspectorProxy::disconnectFromWorkerInspectorController):
(WebCore::WorkerInspectorProxy::sendMessageToWorkerInspectorController):
* workers/WorkerRunLoop.cpp:
(WebCore::ModePredicate::ModePredicate):
(WebCore::WorkerRunLoop::WorkerRunLoop):
(WebCore::debuggerMode):
(WebCore::RunLoopSetup::RunLoopSetup):
(WebCore::RunLoopSetup::~RunLoopSetup):
(WebCore::WorkerRunLoop::run):
(WebCore::WorkerRunLoop::runInDebuggerMode):
(WebCore::WorkerRunLoop::runInMode):
(WebCore::WorkerRunLoop::Task::performTask):
* workers/WorkerRunLoop.h:
(WebCore::WorkerRunLoop::isBeingDebugged const):
* workers/WorkerThread.cpp:
(WebCore::WorkerThread::startRunningDebuggerTasks):
* workers/service/context/ServiceWorkerInspectorProxy.cpp:
(WebCore::ServiceWorkerInspectorProxy::connectToWorker):
(WebCore::ServiceWorkerInspectorProxy::disconnectFromWorker):
(WebCore::ServiceWorkerInspectorProxy::sendMessageToWorker):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (240041 => 240042)
--- trunk/Source/WebCore/ChangeLog 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/ChangeLog 2019-01-16 19:07:22 UTC (rev 240042)
@@ -1,3 +1,54 @@
+2019-01-16 Youenn Fablet <you...@apple.com>
+
+ Prevent WorkerRunLoop::runInMode from spinning in nested cases
+ https://bugs.webkit.org/show_bug.cgi?id=193359
+ <rdar://problem/46345353>
+
+ Reviewed by Joseph Pecoraro.
+
+ Speculative fix for some cases where service worker is spinning and consuming a lot of CPU.
+ The hypothesis is that:
+ - Service Worker is checking for its script freshness through WorkerScriptLoader.
+ This triggers the worker run loop to be nested.
+ - The run loop timer is active and needs to fire immediately.
+ The hypothesis is that this happens in some cases like restarting a device after sleep mode.
+
+ WorkerRunLoop::runInMode will then compute a 0 timeout value for getting a message.
+ This will trigger a timeout while waiting for the message queue.
+ Since the run loop is nested, the run loop timer will not be able to fire,
+ and it will keep ask to fire immediately.
+ runInMode will return timeout as a result and WorkerRunLoop::run will call it immediately.
+
+ The fix is to prevent the shared timer to fire only when the run loop is being debugged through the web inspector.
+ We compute this by checking the run loop mode as debuggerMode().
+ Did some refactoring by introducing helper routines for running the loop and posting task in debugger mode.
+
+ * inspector/WorkerScriptDebugServer.cpp:
+ (WebCore::WorkerScriptDebugServer::runEventLoopWhilePaused):
+ * workers/WorkerInspectorProxy.cpp:
+ (WebCore::WorkerInspectorProxy::resumeWorkerIfPaused):
+ (WebCore::WorkerInspectorProxy::connectToWorkerInspectorController):
+ (WebCore::WorkerInspectorProxy::disconnectFromWorkerInspectorController):
+ (WebCore::WorkerInspectorProxy::sendMessageToWorkerInspectorController):
+ * workers/WorkerRunLoop.cpp:
+ (WebCore::ModePredicate::ModePredicate):
+ (WebCore::WorkerRunLoop::WorkerRunLoop):
+ (WebCore::debuggerMode):
+ (WebCore::RunLoopSetup::RunLoopSetup):
+ (WebCore::RunLoopSetup::~RunLoopSetup):
+ (WebCore::WorkerRunLoop::run):
+ (WebCore::WorkerRunLoop::runInDebuggerMode):
+ (WebCore::WorkerRunLoop::runInMode):
+ (WebCore::WorkerRunLoop::Task::performTask):
+ * workers/WorkerRunLoop.h:
+ (WebCore::WorkerRunLoop::isBeingDebugged const):
+ * workers/WorkerThread.cpp:
+ (WebCore::WorkerThread::startRunningDebuggerTasks):
+ * workers/service/context/ServiceWorkerInspectorProxy.cpp:
+ (WebCore::ServiceWorkerInspectorProxy::connectToWorker):
+ (WebCore::ServiceWorkerInspectorProxy::disconnectFromWorker):
+ (WebCore::ServiceWorkerInspectorProxy::sendMessageToWorker):
+
2019-01-16 Sihui Liu <sihui_...@apple.com>
IndexedDB: leak WebIDBConnectionToClient for retain cycle
Modified: trunk/Source/WebCore/inspector/WorkerScriptDebugServer.cpp (240041 => 240042)
--- trunk/Source/WebCore/inspector/WorkerScriptDebugServer.cpp 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/inspector/WorkerScriptDebugServer.cpp 2019-01-16 19:07:22 UTC (rev 240042)
@@ -74,7 +74,7 @@
MessageQueueWaitResult result;
do {
- result = m_workerGlobalScope.thread().runLoop().runInMode(&m_workerGlobalScope, WorkerRunLoop::debuggerMode());
+ result = m_workerGlobalScope.thread().runLoop().runInDebuggerMode(m_workerGlobalScope);
} while (result != MessageQueueTerminated && !m_doneProcessingDebuggerEvents);
}
Modified: trunk/Source/WebCore/workers/WorkerInspectorProxy.cpp (240041 => 240042)
--- trunk/Source/WebCore/workers/WorkerInspectorProxy.cpp 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/workers/WorkerInspectorProxy.cpp 2019-01-16 19:07:22 UTC (rev 240042)
@@ -90,9 +90,9 @@
void WorkerInspectorProxy::resumeWorkerIfPaused()
{
- m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
+ m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
- }, WorkerRunLoop::debuggerMode());
+ });
}
void WorkerInspectorProxy::connectToWorkerInspectorController(PageChannel* channel)
@@ -103,9 +103,9 @@
m_pageChannel = channel;
- m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
+ m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().connectFrontend();
- }, WorkerRunLoop::debuggerMode());
+ });
}
void WorkerInspectorProxy::disconnectFromWorkerInspectorController()
@@ -116,13 +116,13 @@
m_pageChannel = nullptr;
- m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
+ m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().disconnectFrontend(DisconnectReason::InspectorDestroyed);
// In case the worker is paused running debugger tasks, ensure we break out of
// the pause since this will be the last debugger task we send to the worker.
downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
- }, WorkerRunLoop::debuggerMode());
+ });
}
void WorkerInspectorProxy::sendMessageToWorkerInspectorController(const String& message)
@@ -131,9 +131,9 @@
if (!m_workerThread)
return;
- m_workerThread->runLoop().postTaskForMode([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
+ m_workerThread->runLoop().postDebuggerTask([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().dispatchMessageFromFrontend(message);
- }, WorkerRunLoop::debuggerMode());
+ });
}
void WorkerInspectorProxy::sendMessageFromWorkerToFrontend(const String& message)
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.cpp (240041 => 240042)
--- trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2019-01-16 19:07:22 UTC (rev 240042)
@@ -64,9 +64,9 @@
class ModePredicate {
public:
- ModePredicate(const String& mode)
- : m_mode(mode)
- , m_defaultMode(mode == WorkerRunLoop::defaultMode())
+ explicit ModePredicate(String&& mode)
+ : m_mode(WTFMove(mode))
+ , m_defaultMode(m_mode == WorkerRunLoop::defaultMode())
{
}
@@ -87,8 +87,6 @@
WorkerRunLoop::WorkerRunLoop()
: m_sharedTimer(std::make_unique<WorkerSharedTimer>())
- , m_nestedCount(0)
- , m_uniqueId(0)
{
}
@@ -102,7 +100,7 @@
return String();
}
-String WorkerRunLoop::debuggerMode()
+static String debuggerMode()
{
return "debugger"_s;
}
@@ -110,12 +108,16 @@
class RunLoopSetup {
WTF_MAKE_NONCOPYABLE(RunLoopSetup);
public:
- RunLoopSetup(WorkerRunLoop& runLoop)
+ enum class IsForDebugging { No, Yes };
+ RunLoopSetup(WorkerRunLoop& runLoop, IsForDebugging isForDebugging)
: m_runLoop(runLoop)
+ , m_isForDebugging(isForDebugging)
{
if (!m_runLoop.m_nestedCount)
threadGlobalData().threadTimers().setSharedTimer(m_runLoop.m_sharedTimer.get());
m_runLoop.m_nestedCount++;
+ if (m_isForDebugging == IsForDebugging::Yes)
+ m_runLoop.m_debugCount++;
}
~RunLoopSetup()
@@ -123,14 +125,17 @@
m_runLoop.m_nestedCount--;
if (!m_runLoop.m_nestedCount)
threadGlobalData().threadTimers().setSharedTimer(nullptr);
+ if (m_isForDebugging == IsForDebugging::Yes)
+ m_runLoop.m_debugCount--;
}
private:
WorkerRunLoop& m_runLoop;
+ IsForDebugging m_isForDebugging { IsForDebugging::No };
};
void WorkerRunLoop::run(WorkerGlobalScope* context)
{
- RunLoopSetup setup(*this);
+ RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::No);
ModePredicate modePredicate(defaultMode());
MessageQueueWaitResult result;
do {
@@ -139,10 +144,17 @@
runCleanupTasks(context);
}
+MessageQueueWaitResult WorkerRunLoop::runInDebuggerMode(WorkerGlobalScope& context)
+{
+ RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::Yes);
+ return runInMode(&context, ModePredicate { debuggerMode() }, WaitForMessage);
+}
+
MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerGlobalScope* context, const String& mode, WaitMode waitMode)
{
- RunLoopSetup setup(*this);
- ModePredicate modePredicate(mode);
+ ASSERT(mode != debuggerMode());
+ RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::No);
+ ModePredicate modePredicate(String { mode });
MessageQueueWaitResult result = runInMode(context, modePredicate, waitMode);
return result;
}
@@ -155,7 +167,7 @@
JSC::JSRunLoopTimer::TimerNotificationCallback timerAddedTask = WTF::createSharedTask<JSC::JSRunLoopTimer::TimerNotificationType>([this] {
// We don't actually do anything here, we just want to loop around runInMode
// to both recalculate our deadline and to potentially run the run loop.
- this->postTask([](ScriptExecutionContext&) { });
+ this->postTask([](ScriptExecutionContext&) { });
});
#if USE(GLIB)
@@ -202,7 +214,7 @@
break;
case MessageQueueTimeout:
- if (!context->isClosing() && !isNested())
+ if (!context->isClosing() && !isBeingDebugged())
m_sharedTimer->fire();
break;
}
@@ -251,6 +263,11 @@
m_messageQueue.append(std::make_unique<Task>(WTFMove(task), mode));
}
+void WorkerRunLoop::postDebuggerTask(ScriptExecutionContext::Task&& task)
+{
+ postTaskForMode(WTFMove(task), debuggerMode());
+}
+
void WorkerRunLoop::Task::performTask(WorkerGlobalScope* context)
{
if ((!context->isClosing() && context->script() && !context->script()->isTerminatingExecution()) || m_task.isCleanupTask())
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.h (240041 => 240042)
--- trunk/Source/WebCore/workers/WorkerRunLoop.h 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.h 2019-01-16 19:07:22 UTC (rev 240042)
@@ -53,6 +53,7 @@
// Waits for a single task and returns.
MessageQueueWaitResult runInMode(WorkerGlobalScope*, const String& mode, WaitMode = WaitForMessage);
+ MessageQueueWaitResult runInDebuggerMode(WorkerGlobalScope&);
void terminate();
bool terminated() const { return m_messageQueue.killed(); }
@@ -60,12 +61,11 @@
void postTask(ScriptExecutionContext::Task&&);
void postTaskAndTerminate(ScriptExecutionContext::Task&&);
void postTaskForMode(ScriptExecutionContext::Task&&, const String& mode);
+ void postDebuggerTask(ScriptExecutionContext::Task&&);
unsigned long createUniqueId() { return ++m_uniqueId; }
static String defaultMode();
- static String debuggerMode();
-
class Task {
WTF_MAKE_NONCOPYABLE(Task); WTF_MAKE_FAST_ALLOCATED;
public:
@@ -89,12 +89,13 @@
// This should only be called when the context is closed or loop has been terminated.
void runCleanupTasks(WorkerGlobalScope*);
- bool isNested() const { return m_nestedCount > 1; }
+ bool isBeingDebugged() const { return m_debugCount >= 1; }
MessageQueue<Task> m_messageQueue;
std::unique_ptr<WorkerSharedTimer> m_sharedTimer;
- int m_nestedCount;
- unsigned long m_uniqueId;
+ int m_nestedCount { 0 };
+ int m_debugCount { 0 };
+ unsigned long m_uniqueId { 0 };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/workers/WorkerThread.cpp (240041 => 240042)
--- trunk/Source/WebCore/workers/WorkerThread.cpp 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/workers/WorkerThread.cpp 2019-01-16 19:07:22 UTC (rev 240042)
@@ -249,7 +249,7 @@
MessageQueueWaitResult result;
do {
- result = m_runLoop.runInMode(m_workerGlobalScope.get(), WorkerRunLoop::debuggerMode());
+ result = m_runLoop.runInDebuggerMode(*m_workerGlobalScope);
} while (result != MessageQueueTerminated && m_pausedForDebugger);
}
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp (240041 => 240042)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp 2019-01-16 18:44:25 UTC (rev 240041)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp 2019-01-16 19:07:22 UTC (rev 240042)
@@ -61,9 +61,9 @@
{
m_channel = &channel;
- m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
+ m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().connectFrontend();
- }, WorkerRunLoop::debuggerMode());
+ });
}
void ServiceWorkerInspectorProxy::disconnectFromWorker(FrontendChannel& channel)
@@ -71,20 +71,20 @@
ASSERT_UNUSED(channel, &channel == m_channel);
m_channel = nullptr;
- m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
+ m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().disconnectFrontend(DisconnectReason::InspectorDestroyed);
// In case the worker is paused running debugger tasks, ensure we break out of
// the pause since this will be the last debugger task we send to the worker.
downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
- }, WorkerRunLoop::debuggerMode());
+ });
}
void ServiceWorkerInspectorProxy::sendMessageToWorker(const String& message)
{
- m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
+ m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
downcast<WorkerGlobalScope>(context).inspectorController().dispatchMessageFromFrontend(message);
- }, WorkerRunLoop::debuggerMode());
+ });
}
void ServiceWorkerInspectorProxy::sendMessageFromWorkerToFrontend(const String& message)
