Add finite timeout when synchronously terminating a service worker
https://bugs.webkit.org/show_bug.cgi?id=206325
<rdar://problem/58183380>
Patch by Alex Christensen <achristensen@webkit.org> on 2020-01-16
Reviewed by Youenn Fablet.
When this message reply is never received, it hangs everything.
If we haven't received verification that a service worker was terminated in 10 seconds, unhang everything
and tell the UI process to terminate the hanging service worker process. Continue handling messages during
these 10 seconds to hopefully turn a bad hang into no perceptible hang.
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
(WebKit::WebSWServerToContextConnection::syncTerminateWorker):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254706 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index d500eda..918a3e5 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,19 @@
+2020-01-16 Alex Christensen <achristensen@webkit.org>
+
+ Add finite timeout when synchronously terminating a service worker
+ https://bugs.webkit.org/show_bug.cgi?id=206325
+ <rdar://problem/58183380>
+
+ Reviewed by Youenn Fablet.
+
+ When this message reply is never received, it hangs everything.
+ If we haven't received verification that a service worker was terminated in 10 seconds, unhang everything
+ and tell the UI process to terminate the hanging service worker process. Continue handling messages during
+ these 10 seconds to hopefully turn a bad hang into no perceptible hang.
+
+ * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+ (WebKit::WebSWServerToContextConnection::syncTerminateWorker):
+
2020-01-16 Brady Eidson <beidson@apple.com>
Make the callAsyncJavaScriptFunction function actually be async (so await works).
diff --git a/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp b/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
index 0596480..0c203a1 100644
--- a/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
+++ b/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
@@ -32,6 +32,7 @@
#include "Logging.h"
#include "NetworkConnectionToWebProcess.h"
#include "NetworkProcess.h"
+#include "NetworkProcessProxyMessages.h"
#include "ServiceWorkerFetchTask.h"
#include "ServiceWorkerFetchTaskMessages.h"
#include "WebCoreArgumentCoders.h"
@@ -107,7 +108,8 @@
void WebSWServerToContextConnection::syncTerminateWorker(ServiceWorkerIdentifier serviceWorkerIdentifier)
{
- sendSync(Messages::WebSWContextManagerConnection::SyncTerminateWorker(serviceWorkerIdentifier), Messages::WebSWContextManagerConnection::SyncTerminateWorker::Reply());
+ if (!sendSync(Messages::WebSWContextManagerConnection::SyncTerminateWorker(serviceWorkerIdentifier), Messages::WebSWContextManagerConnection::SyncTerminateWorker::Reply(), 0, 10_s, IPC::SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply))
+ m_connection.networkProcess().parentProcessConnection()->send(Messages::NetworkProcessProxy::TerminateUnresponsiveServiceWorkerProcesses(registrableDomain(), m_connection.sessionID()), 0);
}
void WebSWServerToContextConnection::findClientByIdentifierCompleted(uint64_t requestIdentifier, const Optional<ServiceWorkerClientData>& data, bool hasSecurityError)
diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
index e40e544..9cb90f1 100644
--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
@@ -425,6 +425,12 @@
page->logDiagnosticMessage(message, description, shouldSample);
}
+void NetworkProcessProxy::terminateUnresponsiveServiceWorkerProcesses(WebCore::RegistrableDomain&& domain, PAL::SessionID sessionID)
+{
+ for (auto* processPool : WebProcessPool::allProcessPools())
+ processPool->terminateServiceWorkerProcess(domain, sessionID);
+}
+
void NetworkProcessProxy::logDiagnosticMessageWithResult(WebPageProxyIdentifier pageID, const String& message, const String& description, uint32_t result, WebCore::ShouldSample shouldSample)
{
WebPageProxy* page = WebProcessProxy::webPage(pageID);
diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
index 619676d..77e7f6a 100644
--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
@@ -174,6 +174,7 @@
void didSyncAllCookies();
void testProcessIncomingSyncMessagesWhenWaitingForSyncReply(WebPageProxyIdentifier, Messages::NetworkProcessProxy::TestProcessIncomingSyncMessagesWhenWaitingForSyncReplyDelayedReply&&);
+ void terminateUnresponsiveServiceWorkerProcesses(WebCore::RegistrableDomain&&, PAL::SessionID);
ProcessThrottler& throttler() { return m_throttler; }
void updateProcessAssertion();
diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
index da9709b..3d919be 100644
--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
@@ -30,6 +30,7 @@
DidSyncAllCookies()
TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(WebKit::WebPageProxyIdentifier pageID) -> (bool handled) Synchronous
+ TerminateUnresponsiveServiceWorkerProcesses(WebCore::RegistrableDomain domain, PAL::SessionID sessionId)
SetIsHoldingLockedFiles(bool isHoldingLockedFiles)
diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
index 1b49741..e1ce3a1 100644
--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
@@ -1783,6 +1783,17 @@
#endif
}
+void WebProcessPool::terminateServiceWorkerProcess(const WebCore::RegistrableDomain& domain, const PAL::SessionID& sessionID)
+{
+#if ENABLE(SERVICE_WORKER)
+ auto protectedThis = makeRef(*this);
+ if (auto process = m_serviceWorkerProcesses.get({ domain, sessionID })) {
+ process->disableServiceWorkers();
+ process->requestTermination(ProcessTerminationReason::ExceededCPULimit);
+ }
+#endif
+}
+
void WebProcessPool::syncNetworkProcessCookies()
{
ensureNetworkProcess().syncAllCookies();
diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
index 4b86204..1dbd181 100644
--- a/Source/WebKit/UIProcess/WebProcessPool.h
+++ b/Source/WebKit/UIProcess/WebProcessPool.h
@@ -326,6 +326,7 @@
void sendNetworkProcessWillSuspendImminentlyForTesting();
void sendNetworkProcessDidResume();
void terminateServiceWorkers();
+ void terminateServiceWorkerProcess(const WebCore::RegistrableDomain&, const PAL::SessionID&);
void syncNetworkProcessCookies();
void syncLocalStorage(CompletionHandler<void()>&& callback);
