Diff
Modified: branches/safari-605-branch/LayoutTests/ChangeLog (227358 => 227359)
--- branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-22 22:17:09 UTC (rev 227359)
@@ -1,5 +1,24 @@
2018-01-22 Jason Marcell <[email protected]>
+ Cherry-pick r227174. rdar://problem/36723030
+
+ 2018-01-18 Chris Dumez <[email protected]>
+
+ We should be able to terminate service workers that are unresponsive
+ https://bugs.webkit.org/show_bug.cgi?id=181563
+ <rdar://problem/35280031>
+
+ Reviewed by Alex Christensen.
+
+ Add layout test coverage.
+
+ * http/tests/workers/service/postmessage-after-terminating-hung-worker-expected.txt: Added.
+ * http/tests/workers/service/postmessage-after-terminating-hung-worker.html: Added.
+ * http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js: Added.
+ * http/tests/workers/service/resources/postmessage-echo-worker-mayhang.js: Added.
+
+2018-01-22 Jason Marcell <[email protected]>
+
Cherry-pick r227280. rdar://problem/36722450
2018-01-19 Ryosuke Niwa <[email protected]>
Added: branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker-expected.txt (0 => 227359)
--- branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker-expected.txt (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker-expected.txt 2018-01-22 22:17:09 UTC (rev 227359)
@@ -0,0 +1,8 @@
+PASS: Service worker received message 'HANG'
+PASS: ServiceWorker received message: PASS: Service worker received message 'HANG'
+Service Worker should now be hung
+Terminating service worker...
+Terminated service worker.
+PASS: Service worker received message 'MessageAfterTerminatingHungServiceWorker'
+PASS: Service worker responded to message after hanging and being terminated
+
Added: branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker.html (0 => 227359)
--- branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker.html (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/workers/service/postmessage-after-terminating-hung-worker.html 2018-01-22 22:17:09 UTC (rev 227359)
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+
+<script src=""
+</body>
+</html>
Added: branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js (0 => 227359)
--- branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js 2018-01-22 22:17:09 UTC (rev 227359)
@@ -0,0 +1,29 @@
+let state = "WaitingForHang";
+
+navigator.serviceWorker.addEventListener("message", function(event) {
+ log(event.data);
+ if (state === "WaitingForHang") {
+ log("PASS: ServiceWorker received message: " + event.data);
+ log("Service Worker should now be hung");
+ log("Terminating service worker...")
+ internals.terminateServiceWorker(worker);
+ log("Terminated service worker.");
+ state = "WaitingForMessageAfterTerminatingHungServiceWorker"
+ handle = setInterval(function() {
+ worker.postMessage("MessageAfterTerminatingHungServiceWorker");
+ }, 10);
+ return;
+ }
+ if (state === "WaitingForMessageAfterTerminatingHungServiceWorker") {
+ log("PASS: Service worker responded to message after hanging and being terminated");
+ state = "Done";
+ clearInterval(handle);
+ finishSWTest();
+ return;
+ }
+});
+
+navigator.serviceWorker.register("resources/postmessage-echo-worker-mayhang.js", { }).then(function(registration) {
+ worker = registration.installing;
+ worker.postMessage("HANG");
+});
Added: branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-echo-worker-mayhang.js (0 => 227359)
--- branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-echo-worker-mayhang.js (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/workers/service/resources/postmessage-echo-worker-mayhang.js 2018-01-22 22:17:09 UTC (rev 227359)
@@ -0,0 +1,8 @@
+self.addEventListener("message", (event) => {
+ event.source.postMessage("PASS: Service worker received message '" + event.data + "'");
+
+ if (event.data ="" "HANG") {
+ while(1) { };
+ }
+});
+
Modified: branches/safari-605-branch/Source/WTF/ChangeLog (227358 => 227359)
--- branches/safari-605-branch/Source/WTF/ChangeLog 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WTF/ChangeLog 2018-01-22 22:17:09 UTC (rev 227359)
@@ -1,3 +1,19 @@
+2018-01-22 Jason Marcell <[email protected]>
+
+ Cherry-pick r227174. rdar://problem/36723030
+
+ 2018-01-18 Chris Dumez <[email protected]>
+
+ We should be able to terminate service workers that are unresponsive
+ https://bugs.webkit.org/show_bug.cgi?id=181563
+ <rdar://problem/35280031>
+
+ Reviewed by Alex Christensen.
+
+ * wtf/ObjectIdentifier.h:
+ (WTF::ObjectIdentifier::loggingString const):
+ Allow using loggingString() from RELEASE_LOG().
+
2018-01-18 Jason Marcell <[email protected]>
Cherry-pick r226956. rdar://problem/36598688
Modified: branches/safari-605-branch/Source/WTF/wtf/ObjectIdentifier.h (227358 => 227359)
--- branches/safari-605-branch/Source/WTF/wtf/ObjectIdentifier.h 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WTF/wtf/ObjectIdentifier.h 2018-01-22 22:17:09 UTC (rev 227359)
@@ -67,12 +67,10 @@
uint64_t toUInt64() const { return m_identifier; }
-#ifndef NDEBUG
String loggingString() const
{
return String::number(m_identifier);
}
-#endif
private:
template<typename U> friend ObjectIdentifier<U> generateObjectIdentifier();
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (227358 => 227359)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 22:17:09 UTC (rev 227359)
@@ -1,5 +1,33 @@
2018-01-22 Jason Marcell <[email protected]>
+ Cherry-pick r227174. rdar://problem/36723030
+
+ 2018-01-18 Chris Dumez <[email protected]>
+
+ We should be able to terminate service workers that are unresponsive
+ https://bugs.webkit.org/show_bug.cgi?id=181563
+ <rdar://problem/35280031>
+
+ Reviewed by Alex Christensen.
+
+ Test: http/tests/workers/service/postmessage-after-terminating-hung-worker.html
+
+ * workers/service/context/SWContextManager.cpp:
+ (WebCore::SWContextManager::terminateWorker):
+ Before calling WorkerThread::stop(), set a timer with the given timeout parameter.
+ If the worker thread has not stopped when the timer fires, forcefully exit the
+ service worker process. The StorageProcess will take care of relaunching the
+ service worker process if it exits abruptly.
+
+ (WebCore::SWContextManager::serviceWorkerFailedToTerminate):
+ Log error message if we failed to terminate a service worker and call exit().
+
+ (WebCore::SWContextManager::ServiceWorkerTerminationRequest::ServiceWorkerTerminationRequest):
+
+ * workers/service/context/SWContextManager.h:
+
+2018-01-22 Jason Marcell <[email protected]>
+
Cherry-pick r227280. rdar://problem/36722450
2018-01-19 Ryosuke Niwa <[email protected]>
Modified: branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.cpp (227358 => 227359)
--- branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.cpp 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.cpp 2018-01-22 22:17:09 UTC (rev 227359)
@@ -27,8 +27,11 @@
#if ENABLE(SERVICE_WORKER)
#include "SWContextManager.h"
+
+#include "Logging.h"
#include "ServiceWorkerClientIdentifier.h"
#include "ServiceWorkerGlobalScope.h"
+#include <unistd.h>
namespace WebCore {
@@ -102,21 +105,28 @@
serviceWorker->thread().fireActivateEvent();
}
-void SWContextManager::terminateWorker(ServiceWorkerIdentifier identifier, Function<void()>&& completionHandler)
+void SWContextManager::terminateWorker(ServiceWorkerIdentifier identifier, Seconds timeout, Function<void()>&& completionHandler)
{
auto serviceWorker = m_workerMap.take(identifier);
- if (!serviceWorker)
+ if (!serviceWorker) {
+ if (completionHandler)
+ completionHandler();
return;
+ }
serviceWorker->setAsTerminatingOrTerminated();
- serviceWorker->thread().stop([identifier, serviceWorker = WTFMove(serviceWorker), completionHandler = WTFMove(completionHandler)]() mutable {
+ m_pendingServiceWorkerTerminationRequests.add(identifier, std::make_unique<ServiceWorkerTerminationRequest>(*this, identifier, timeout));
+
+ serviceWorker->thread().stop([this, identifier, serviceWorker = WTFMove(serviceWorker), completionHandler = WTFMove(completionHandler)]() mutable {
+ m_pendingServiceWorkerTerminationRequests.remove(identifier);
+
if (auto* connection = SWContextManager::singleton().connection())
connection->workerTerminated(identifier);
if (completionHandler)
completionHandler();
-
+
// Spin the runloop before releasing the worker thread proxy, as there would otherwise be
// a race towards its destruction.
callOnMainThread([serviceWorker = WTFMove(serviceWorker)] { });
@@ -141,6 +151,19 @@
return true;
}
+NO_RETURN_DUE_TO_CRASH void SWContextManager::serviceWorkerFailedToTerminate(ServiceWorkerIdentifier serviceWorkerIdentifier)
+{
+ UNUSED_PARAM(serviceWorkerIdentifier);
+ RELEASE_LOG_ERROR(ServiceWorker, "Failed to terminate service worker with identifier %s, killing the service worker process", serviceWorkerIdentifier.loggingString().utf8().data());
+ _exit(EXIT_FAILURE);
+}
+
+SWContextManager::ServiceWorkerTerminationRequest::ServiceWorkerTerminationRequest(SWContextManager& manager, ServiceWorkerIdentifier serviceWorkerIdentifier, Seconds timeout)
+ : m_timeoutTimer([this, &manager, serviceWorkerIdentifier] { manager.serviceWorkerFailedToTerminate(serviceWorkerIdentifier); })
+{
+ m_timeoutTimer.startOneShot(timeout);
+}
+
} // namespace WebCore
#endif
Modified: branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.h (227358 => 227359)
--- branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.h 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WebCore/workers/service/context/SWContextManager.h 2018-01-22 22:17:09 UTC (rev 227359)
@@ -70,7 +70,7 @@
WEBCORE_EXPORT void postMessageToServiceWorker(ServiceWorkerIdentifier destination, Ref<SerializedScriptValue>&& message, ServiceWorkerOrClientData&& sourceData);
WEBCORE_EXPORT void fireInstallEvent(ServiceWorkerIdentifier);
WEBCORE_EXPORT void fireActivateEvent(ServiceWorkerIdentifier);
- WEBCORE_EXPORT void terminateWorker(ServiceWorkerIdentifier, Function<void()>&&);
+ WEBCORE_EXPORT void terminateWorker(ServiceWorkerIdentifier, Seconds timeout, Function<void()>&&);
void forEachServiceWorkerThread(const WTF::Function<void(ServiceWorkerThreadProxy&)>&);
@@ -85,10 +85,20 @@
SWContextManager() = default;
void startedServiceWorker(std::optional<ServiceWorkerJobDataIdentifier>, ServiceWorkerIdentifier, const String& exceptionMessage);
+ void serviceWorkerFailedToTerminate(ServiceWorkerIdentifier);
HashMap<ServiceWorkerIdentifier, RefPtr<ServiceWorkerThreadProxy>> m_workerMap;
std::unique_ptr<Connection> m_connection;
ServiceWorkerCreationCallback* m_serviceWorkerCreationCallback { nullptr };
+
+ class ServiceWorkerTerminationRequest {
+ public:
+ ServiceWorkerTerminationRequest(SWContextManager&, ServiceWorkerIdentifier, Seconds timeout);
+
+ private:
+ Timer m_timeoutTimer;
+ };
+ HashMap<ServiceWorkerIdentifier, std::unique_ptr<ServiceWorkerTerminationRequest>> m_pendingServiceWorkerTerminationRequests;
};
} // namespace WebCore
Modified: branches/safari-605-branch/Source/WebKit/ChangeLog (227358 => 227359)
--- branches/safari-605-branch/Source/WebKit/ChangeLog 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WebKit/ChangeLog 2018-01-22 22:17:09 UTC (rev 227359)
@@ -1,5 +1,27 @@
2018-01-22 Jason Marcell <[email protected]>
+ Cherry-pick r227174. rdar://problem/36723030
+
+ 2018-01-18 Chris Dumez <[email protected]>
+
+ We should be able to terminate service workers that are unresponsive
+ https://bugs.webkit.org/show_bug.cgi?id=181563
+ <rdar://problem/35280031>
+
+ Reviewed by Alex Christensen.
+
+ * WebProcess/Storage/WebSWContextManagerConnection.cpp:
+ (WebKit::WebSWContextManagerConnection::terminateWorker):
+ Use a 10 second timeout for forcefully exiting the service worker process when
+ the service worker in question fails to terminate.
+
+ (WebKit::WebSWContextManagerConnection::syncTerminateWorker):
+ Use a 100ms timeout for forcefully exiting the service worker process when
+ the service worker in question fails to terminate. This method is only called
+ from the layout tests, which is why we use a very short timeout.
+
+2018-01-22 Jason Marcell <[email protected]>
+
Cherry-pick r227274. rdar://problem/36722660
2018-01-20 Andy Estes <[email protected]>
Modified: branches/safari-605-branch/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp (227358 => 227359)
--- branches/safari-605-branch/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp 2018-01-22 22:15:22 UTC (rev 227358)
+++ branches/safari-605-branch/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp 2018-01-22 22:17:09 UTC (rev 227359)
@@ -62,6 +62,9 @@
namespace WebKit {
+static const Seconds asyncWorkerTerminationTimeout { 10_s };
+static const Seconds syncWorkerTerminationTimeout { 100_ms }; // Only used by layout tests.
+
class ServiceWorkerFrameLoaderClient final : public EmptyFrameLoaderClient {
public:
ServiceWorkerFrameLoaderClient(WebSWContextManagerConnection& connection, PAL::SessionID sessionID, uint64_t pageID, uint64_t frameID, const String& userAgent)
@@ -196,12 +199,12 @@
void WebSWContextManagerConnection::terminateWorker(ServiceWorkerIdentifier identifier)
{
- SWContextManager::singleton().terminateWorker(identifier, nullptr);
+ SWContextManager::singleton().terminateWorker(identifier, asyncWorkerTerminationTimeout, nullptr);
}
void WebSWContextManagerConnection::syncTerminateWorker(ServiceWorkerIdentifier identifier, Ref<Messages::WebSWContextManagerConnection::SyncTerminateWorker::DelayedReply>&& reply)
{
- SWContextManager::singleton().terminateWorker(identifier, [reply = WTFMove(reply)] {
+ SWContextManager::singleton().terminateWorker(identifier, syncWorkerTerminationTimeout, [reply = WTFMove(reply)] {
reply->send();
});
}
