Changeset 241855 in webkit


Ignore:
Timestamp:
Feb 20, 2019 5:25:46 PM (5 years ago)
Author:
Chris Dumez
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:
Location:
trunk/Source/WebKit
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r241853 r241855  
     12019-02-20  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] Make sure hung processes are not kept alive by suspended pages or process caching
     4        https://bugs.webkit.org/show_bug.cgi?id=194881
     5        <rdar://problem/48249014>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        After we construct a SuspendedPageProxy and before we send the IPC to the WebProcess to
     10        ask it to suspend, start a 10 seconds timer. If the process does not answer the request
     11        to suspend before the timer fires, we destroy the SuspendedPageProxy so that we do not
     12        keep a hung process around.
     13
     14        For the WebProcessCache, we now call WebProcessProxy::isResponsive() on the process
     15        before adding it to the cache. Internally, this relies on an IPC handshake with the
     16        WebProcess. If the process is not responsive, we do not add it to the cache and we
     17        shut it down. If it is responsive then we proceed normally with adding it to the
     18        cache.
     19
     20        * UIProcess/SuspendedPageProxy.cpp:
     21        (WebKit::SuspendedPageProxy::SuspendedPageProxy):
     22        (WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
     23        (WebKit::SuspendedPageProxy::suspensionTimedOut):
     24        * UIProcess/SuspendedPageProxy.h:
     25        * UIProcess/WebProcessCache.cpp:
     26        (WebKit::WebProcessCache::addProcessIfPossible):
     27        (WebKit::WebProcessCache::addProcess):
     28        * UIProcess/WebProcessCache.h:
     29        * UIProcess/WebProcessProxy.cpp:
     30        (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
     31        (WebKit::WebProcessProxy::maybeShutDown):
     32        (WebKit::WebProcessProxy::isResponsive):
     33        * UIProcess/WebProcessProxy.h:
     34
    1352019-02-20  Chris Dumez  <cdumez@apple.com>
    236
  • trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp

    r241751 r241855  
    4141namespace WebKit {
    4242using namespace WebCore;
     43
     44static const Seconds suspensionTimeout { 10_s };
    4345
    4446#if !LOG_DISABLED
     
    8284    , m_mainFrameID(mainFrameID)
    8385    , m_registrableDomain(toRegistrableDomain(URL(URL(), item.url())))
     86    , m_suspensionTimeoutTimer(RunLoop::main(), this, &SuspendedPageProxy::suspensionTimedOut)
    8487#if PLATFORM(IOS_FAMILY)
    8588    , m_suspensionToken(m_process->throttler().backgroundActivityToken())
     
    8992    m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID(), *this);
    9093
     94    m_suspensionTimeoutTimer.startOneShot(suspensionTimeout);
    9195    m_process->send(Messages::WebPage::SetIsSuspended(true), m_page.pageID());
    9296}
     
    165169    m_suspensionState = newSuspensionState;
    166170
     171    m_suspensionTimeoutTimer.stop();
     172
    167173#if PLATFORM(IOS_FAMILY)
    168174    m_suspensionToken = nullptr;
     
    184190}
    185191
     192void SuspendedPageProxy::suspensionTimedOut()
     193{
     194    RELEASE_LOG_ERROR(ProcessSwapping, "%p - SuspendedPageProxy::suspensionTimedOut() destroying the suspended page because it failed to suspend in time", this);
     195    m_process->processPool().removeSuspendedPage(*this); // Will destroy |this|.
     196}
     197
    186198void SuspendedPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder& decoder)
    187199{
  • trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h

    r241853 r241855  
    6363    enum class SuspensionState : uint8_t { Suspending, FailedToSuspend, Suspended, Resumed };
    6464    void didProcessRequestToSuspend(SuspensionState);
     65    void suspensionTimedOut();
    6566
    6667    // IPC::MessageReceiver
     
    7677    SuspensionState m_suspensionState { SuspensionState::Suspending };
    7778    CompletionHandler<void(SuspendedPageProxy*)> m_readyToUnsuspendHandler;
     79    RunLoop::Timer<SuspendedPageProxy> m_suspensionTimeoutTimer;
    7880#if PLATFORM(IOS_FAMILY)
    7981    ProcessThrottler::BackgroundActivityToken m_suspensionToken;
  • trunk/Source/WebKit/UIProcess/WebProcessCache.cpp

    r241635 r241855  
    4545}
    4646
    47 bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcessProxy>&& process)
     47bool WebProcessCache::addProcessIfPossible(const String& registrableDomain, Ref<WebProcessProxy>&& process)
    4848{
    4949    ASSERT(!registrableDomain.isEmpty());
     
    5656
    5757    if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
     58        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because we are under memory pressure", this, process->processIdentifier());
     59        return false;
     60    }
     61
     62    if (m_processesPerRegistrableDomain.contains(registrableDomain)) {
     63        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because we already have a cached process for this domain", this, process->processIdentifier());
     64        return false;
     65    }
     66
     67    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Checking if process %i is responsive before caching it...", this, process->processIdentifier());
     68    process->setIsInProcessCache(true);
     69    process->isResponsive([process = process.copyRef(), registrableDomain](bool isResponsive) {
     70        process->setIsInProcessCache(false);
     71        if (!isResponsive) {
     72            RELEASE_LOG_ERROR(ProcessSwapping, "%p - WebProcessCache::addProcessIfPossible(): Not caching process %i because it is not responsive", &process->processPool().webProcessCache(), process->processIdentifier());
     73            process->shutDown();
     74            return;
     75        }
     76        if (!process->processPool().webProcessCache().addProcess(registrableDomain, process.copyRef()))
     77            process->shutDown();
     78    });
     79    return true;
     80}
     81
     82bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcessProxy>&& process)
     83{
     84    ASSERT(!process->pageCount());
     85    ASSERT(!process->provisionalPageCount());
     86    ASSERT(!process->processPool().hasSuspendedPageFor(process));
     87
     88    if (!capacity())
     89        return false;
     90
     91    if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
    5892        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Not caching process %i because we are under memory pressure", this, process->processIdentifier());
    5993        return false;
     
    69103        return std::make_unique<CachedProcess>(WTFMove(process));
    70104    });
    71     if (!addResult.isNewEntry)
    72         return false;
     105    if (!addResult.isNewEntry) {
     106        RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess(): Not caching process %i because we already have a cached process for this domain", this, process->processIdentifier());
     107        return false;
     108    }
    73109
    74110    RELEASE_LOG(ProcessSwapping, "%p - WebProcessCache::addProcess: Adding process %i to WebProcess cache, cache size: [%u / %u]", this, process->processIdentifier(), size(), capacity());
  • trunk/Source/WebKit/UIProcess/WebProcessCache.h

    r241635 r241855  
    4242    explicit WebProcessCache(WebProcessPool&);
    4343
    44     bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
     44    bool addProcessIfPossible(const String& registrableDomain, Ref<WebProcessProxy>&&);
    4545    RefPtr<WebProcessProxy> takeProcess(const String& registrableDomain, WebsiteDataStore&);
    4646
     
    5959    void evictProcess(WebProcessProxy&);
    6060    void platformInitialize();
     61    bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
    6162
    6263    unsigned m_capacity { 0 };
  • trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r241778 r241855  
    637637    auto provisionalPages = WTF::map(m_provisionalPages, [](auto* provisionalPage) { return makeWeakPtr(provisionalPage); });
    638638
     639    auto isResponsiveCallbacks = WTFMove(m_isResponsiveCallbacks);
     640    for (auto& callback : isResponsiveCallbacks)
     641        callback(false);
     642
    639643    if (m_isInProcessCache) {
    640644        auto removedProcess = processPool().webProcessCache().takeProcess(registrableDomain(), websiteDataStore());
     
    840844        return;
    841845
    842     if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcess(registrableDomain(), *this))
     846    if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(registrableDomain(), *this))
    843847        return;
    844848
     
    11841188}
    11851189
    1186 void WebProcessProxy::isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&& callback)
     1190void WebProcessProxy::isResponsive(CompletionHandler<void(bool isWebProcessResponsive)>&& callback)
    11871191{
    11881192    if (m_isResponsive == NoOrMaybe::No) {
    11891193        if (callback) {
    1190             RunLoop::main().dispatch([callback = WTFMove(callback)] {
     1194            RunLoop::main().dispatch([callback = WTFMove(callback)]() mutable {
    11911195                bool isWebProcessResponsive = false;
    11921196                callback(isWebProcessResponsive);
  • trunk/Source/WebKit/UIProcess/WebProcessProxy.h

    r241778 r241855  
    207207    ProcessThrottler& throttler() { return m_throttler; }
    208208
    209     void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
     209    void isResponsive(CompletionHandler<void(bool isWebProcessResponsive)>&&);
    210210    void isResponsiveWithLazyStop();
    211211    void didReceiveMainThreadPing();
     
    410410
    411411    enum class NoOrMaybe { No, Maybe } m_isResponsive;
    412     Vector<WTF::Function<void(bool webProcessIsResponsive)>> m_isResponsiveCallbacks;
     412    Vector<CompletionHandler<void(bool webProcessIsResponsive)>> m_isResponsiveCallbacks;
    413413
    414414    VisibleWebPageCounter m_visiblePageCounter;
Note: See TracChangeset for help on using the changeset viewer.