- Revision
- 241855
- Author
- cdu...@apple.com
- Date
- 2019-02-20 17:25:46 -0800 (Wed, 20 Feb 2019)
Log Message
[PSON] Make sure hung processes are not kept alive by suspended pages or process caching
https://bugs.webkit.org/show_bug.cgi?id=194881
<rdar://problem/48249014>
Reviewed by Geoffrey Garen.
After we construct a SuspendedPageProxy and before we send the IPC to the WebProcess to
ask it to suspend, start a 10 seconds timer. If the process does not answer the request
to suspend before the timer fires, we destroy the SuspendedPageProxy so that we do not
keep a hung process around.
For the WebProcessCache, we now call WebProcessProxy::isResponsive() on the process
before adding it to the cache. Internally, this relies on an IPC handshake with the
WebProcess. If the process is not responsive, we do not add it to the cache and we
shut it down. If it is responsive then we proceed normally with adding it to the
cache.
* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
(WebKit::SuspendedPageProxy::suspensionTimedOut):
* UIProcess/SuspendedPageProxy.h:
* UIProcess/WebProcessCache.cpp:
(WebKit::WebProcessCache::addProcessIfPossible):
(WebKit::WebProcessCache::addProcess):
* UIProcess/WebProcessCache.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
(WebKit::WebProcessProxy::maybeShutDown):
(WebKit::WebProcessProxy::isResponsive):
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (241854 => 241855)
--- trunk/Source/WebKit/ChangeLog 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/ChangeLog 2019-02-21 01:25:46 UTC (rev 241855)
@@ -1,5 +1,39 @@
2019-02-20 Chris Dumez <cdu...@apple.com>
+ [PSON] Make sure hung processes are not kept alive by suspended pages or process caching
+ https://bugs.webkit.org/show_bug.cgi?id=194881
+ <rdar://problem/48249014>
+
+ Reviewed by Geoffrey Garen.
+
+ After we construct a SuspendedPageProxy and before we send the IPC to the WebProcess to
+ ask it to suspend, start a 10 seconds timer. If the process does not answer the request
+ to suspend before the timer fires, we destroy the SuspendedPageProxy so that we do not
+ keep a hung process around.
+
+ For the WebProcessCache, we now call WebProcessProxy::isResponsive() on the process
+ before adding it to the cache. Internally, this relies on an IPC handshake with the
+ WebProcess. If the process is not responsive, we do not add it to the cache and we
+ shut it down. If it is responsive then we proceed normally with adding it to the
+ cache.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+ (WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
+ (WebKit::SuspendedPageProxy::suspensionTimedOut):
+ * UIProcess/SuspendedPageProxy.h:
+ * UIProcess/WebProcessCache.cpp:
+ (WebKit::WebProcessCache::addProcessIfPossible):
+ (WebKit::WebProcessCache::addProcess):
+ * UIProcess/WebProcessCache.h:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
+ (WebKit::WebProcessProxy::maybeShutDown):
+ (WebKit::WebProcessProxy::isResponsive):
+ * UIProcess/WebProcessProxy.h:
+
+2019-02-20 Chris Dumez <cdu...@apple.com>
+
Unreviewed build fix after r241823.
* UIProcess/SuspendedPageProxy.h:
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-02-21 01:25:46 UTC (rev 241855)
@@ -41,6 +41,8 @@
namespace WebKit {
using namespace WebCore;
+static const Seconds suspensionTimeout { 10_s };
+
#if !LOG_DISABLED
static const HashSet<IPC::StringReference>& messageNamesToIgnoreWhileSuspended()
{
@@ -81,6 +83,7 @@
, m_process(WTFMove(process))
, m_mainFrameID(mainFrameID)
, m_registrableDomain(toRegistrableDomain(URL(URL(), item.url())))
+ , m_suspensionTimeoutTimer(RunLoop::main(), this, &SuspendedPageProxy::suspensionTimedOut)
#if PLATFORM(IOS_FAMILY)
, m_suspensionToken(m_process->throttler().backgroundActivityToken())
#endif
@@ -88,6 +91,7 @@
item.setSuspendedPage(this);
m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID(), *this);
+ m_suspensionTimeoutTimer.startOneShot(suspensionTimeout);
m_process->send(Messages::WebPage::SetIsSuspended(true), m_page.pageID());
}
@@ -164,6 +168,8 @@
m_suspensionState = newSuspensionState;
+ m_suspensionTimeoutTimer.stop();
+
#if PLATFORM(IOS_FAMILY)
m_suspensionToken = nullptr;
#endif
@@ -183,6 +189,12 @@
m_readyToUnsuspendHandler(this);
}
+void SuspendedPageProxy::suspensionTimedOut()
+{
+ RELEASE_LOG_ERROR(ProcessSwapping, "%p - SuspendedPageProxy::suspensionTimedOut() destroying the suspended page because it failed to suspend in time", this);
+ m_process->processPool().removeSuspendedPage(*this); // Will destroy |this|.
+}
+
void SuspendedPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder& decoder)
{
ASSERT(decoder.messageReceiverName() == Messages::WebPageProxy::messageReceiverName());
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-02-21 01:25:46 UTC (rev 241855)
@@ -62,6 +62,7 @@
private:
enum class SuspensionState : uint8_t { Suspending, FailedToSuspend, Suspended, Resumed };
void didProcessRequestToSuspend(SuspensionState);
+ void suspensionTimedOut();
// IPC::MessageReceiver
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
@@ -75,6 +76,7 @@
SuspensionState m_suspensionState { SuspensionState::Suspending };
CompletionHandler<void(SuspendedPageProxy*)> m_readyToUnsuspendHandler;
+ RunLoop::Timer<SuspendedPageProxy> m_suspensionTimeoutTimer;
#if PLATFORM(IOS_FAMILY)
ProcessThrottler::BackgroundActivityToken m_suspensionToken;
#endif
Modified: trunk/Source/WebKit/UIProcess/WebProcessCache.cpp (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/WebProcessCache.cpp 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/WebProcessCache.cpp 2019-02-21 01:25:46 UTC (rev 241855)
@@ -44,7 +44,7 @@
platformInitialize();
}
-bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcessProxy>&& process)
+bool WebProcessCache::addProcessIfPossible(const String& registrableDomain, Ref<WebProcessProxy>&& process)
{
ASSERT(!registrableDomain.isEmpty());
ASSERT(!process->pageCount());
@@ -55,6 +55,40 @@
return false;
if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
+ RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because we are under memory pressure", this, process->processIdentifier());
+ return false;
+ }
+
+ if (m_processesPerRegistrableDomain.contains(registrableDomain)) {
+ RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because we already have a cached process for this domain", this, process->processIdentifier());
+ return false;
+ }
+
+ RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Checking if process %i is responsive before caching it...", this, process->processIdentifier());
+ process->setIsInProcessCache(true);
+ process->isResponsive([process = process.copyRef(), registrableDomain](bool isResponsive) {
+ process->setIsInProcessCache(false);
+ if (!isResponsive) {
+ RELEASE_LOG_ERROR(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because it is not responsive", &process->processPool().webProcessCache(), process->processIdentifier());
+ process->shutDown();
+ return;
+ }
+ if (!process->processPool().webProcessCache().addProcess(registrableDomain, process.copyRef()))
+ process->shutDown();
+ });
+ return true;
+}
+
+bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcessProxy>&& process)
+{
+ ASSERT(!process->pageCount());
+ ASSERT(!process->provisionalPageCount());
+ ASSERT(!process->processPool().hasSuspendedPageFor(process));
+
+ if (!capacity())
+ return false;
+
+ if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Not caching process %i because we are under memory pressure", this, process->processIdentifier());
return false;
}
@@ -68,8 +102,10 @@
auto addResult = m_processesPerRegistrableDomain.ensure(registrableDomain, [process = process.copyRef()]() mutable {
return std::make_unique<CachedProcess>(WTFMove(process));
});
- if (!addResult.isNewEntry)
+ if (!addResult.isNewEntry) {
+ RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Not caching process %i because we already have a cached process for this domain", this, process->processIdentifier());
return false;
+ }
RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess: Adding process %i to WebProcess cache, cache size: [%u / %u]", this, process->processIdentifier(), size(), capacity());
return true;
Modified: trunk/Source/WebKit/UIProcess/WebProcessCache.h (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/WebProcessCache.h 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/WebProcessCache.h 2019-02-21 01:25:46 UTC (rev 241855)
@@ -41,7 +41,7 @@
public:
explicit WebProcessCache(WebProcessPool&);
- bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
+ bool addProcessIfPossible(const String& registrableDomain, Ref<WebProcessProxy>&&);
RefPtr<WebProcessProxy> takeProcess(const String& registrableDomain, WebsiteDataStore&);
void updateCapacity(WebProcessPool&);
@@ -58,6 +58,7 @@
void evictProcess(WebProcessProxy&);
void platformInitialize();
+ bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
unsigned m_capacity { 0 };
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-02-21 01:25:46 UTC (rev 241855)
@@ -636,6 +636,10 @@
auto pages = copyToVectorOf<RefPtr<WebPageProxy>>(m_pageMap.values());
auto provisionalPages = WTF::map(m_provisionalPages, [](auto* provisionalPage) { return makeWeakPtr(provisionalPage); });
+ auto isResponsiveCallbacks = WTFMove(m_isResponsiveCallbacks);
+ for (auto& callback : isResponsiveCallbacks)
+ callback(false);
+
if (m_isInProcessCache) {
auto removedProcess = processPool().webProcessCache().takeProcess(registrableDomain(), websiteDataStore());
ASSERT_UNUSED(removedProcess, removedProcess.get() == this);
@@ -839,7 +843,7 @@
if (state() == State::Terminated || !canTerminateAuxiliaryProcess())
return;
- if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcess(registrableDomain(), *this))
+ if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(registrableDomain(), *this))
return;
shutDown();
@@ -1183,11 +1187,11 @@
}
}
-void WebProcessProxy::isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&& callback)
+void WebProcessProxy::isResponsive(CompletionHandler<void(bool isWebProcessResponsive)>&& callback)
{
if (m_isResponsive == NoOrMaybe::No) {
if (callback) {
- RunLoop::main().dispatch([callback = WTFMove(callback)] {
+ RunLoop::main().dispatch([callback = WTFMove(callback)]() mutable {
bool isWebProcessResponsive = false;
callback(isWebProcessResponsive);
});
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (241854 => 241855)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-02-21 01:09:39 UTC (rev 241854)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-02-21 01:25:46 UTC (rev 241855)
@@ -206,7 +206,7 @@
ProcessThrottler& throttler() { return m_throttler; }
- void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
+ void isResponsive(CompletionHandler<void(bool isWebProcessResponsive)>&&);
void isResponsiveWithLazyStop();
void didReceiveMainThreadPing();
void didReceiveBackgroundResponsivenessPing();
@@ -409,7 +409,7 @@
bool m_isInProcessCache { false };
enum class NoOrMaybe { No, Maybe } m_isResponsive;
- Vector<WTF::Function<void(bool webProcessIsResponsive)>> m_isResponsiveCallbacks;
+ Vector<CompletionHandler<void(bool webProcessIsResponsive)>> m_isResponsiveCallbacks;
VisibleWebPageCounter m_visiblePageCounter;
