⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 273749 in webkit


Ignore:
Timestamp:
Mar 2, 2021, 11:51:34 AM (6 years ago)
Author:
Ruben Turcios
Message:

Cherry-pick r273695. rdar://problem/74940739

Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
​https://bugs.webkit.org/show_bug.cgi?id=222574
<rdar://68340471>

Reviewed by Geoffrey Garen.

Source/WebKit:

The issue was that when WebProcessProxy::requestTermination() was called (e.g. process is killed by WebKit
for using too much memory), we would fail to remove the process from the WebProcessCache. Because the
terminated would stay in the cache (even though WebProcessProxy::shutDown() was called), we could potentially
try and use it later on for a navigation to the same domain. This would lead to crashes because
WebProcessProxy::shutDown() has already been called.

Note that we were previously correctly removing the process from the cache in case of a proper crash, inside
WebProcessProxy::processDidTerminateOrFailedToLaunch(). I have moved the logic to remove from the cache
from processDidTerminateOrFailedToLaunch() to shutDown() to avoid similar issues in the future.

  • UIProcess/API/Cocoa/WKProcessPool.mm: (-[WKProcessPool _requestWebProcessTermination:]):
  • UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
  • UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::shutDown): (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):

Tools:

Add API test coverage.

  • TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@273695 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-611.1.21.1-branch
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-611.1.21.1-branch/Source/WebKit/ChangeLog

    r273689 r273749  
     12021-03-02  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r273695. rdar://problem/74940739
     4
     5    Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
     6    https://bugs.webkit.org/show_bug.cgi?id=222574
     7    <rdar://68340471>
     8   
     9    Reviewed by Geoffrey Garen.
     10   
     11    Source/WebKit:
     12   
     13    The issue was that when WebProcessProxy::requestTermination() was called (e.g. process is killed by WebKit
     14    for using too much memory), we would fail to remove the process from the WebProcessCache. Because the
     15    terminated would stay in the cache (even though WebProcessProxy::shutDown() was called), we could potentially
     16    try and use it later on for a navigation to the same domain. This would lead to crashes because
     17    WebProcessProxy::shutDown() has already been called.
     18   
     19    Note that we were previously correctly removing the process from the cache in case of a proper crash, inside
     20    WebProcessProxy::processDidTerminateOrFailedToLaunch(). I have moved the logic to remove from the cache
     21    from processDidTerminateOrFailedToLaunch() to shutDown() to avoid similar issues in the future.
     22   
     23    * UIProcess/API/Cocoa/WKProcessPool.mm:
     24    (-[WKProcessPool _requestWebProcessTermination:]):
     25    * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
     26    * UIProcess/WebProcessProxy.cpp:
     27    (WebKit::WebProcessProxy::shutDown):
     28    (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
     29   
     30    Tools:
     31   
     32    Add API test coverage.
     33   
     34    * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     35   
     36   
     37    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273695 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     38
     39    2021-03-01  Chris Dumez  <cdumez@apple.com>
     40
     41            Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
     42            https://bugs.webkit.org/show_bug.cgi?id=222574
     43            <rdar://68340471>
     44
     45            Reviewed by Geoffrey Garen.
     46
     47            The issue was that when WebProcessProxy::requestTermination() was called (e.g. process is killed by WebKit
     48            for using too much memory), we would fail to remove the process from the WebProcessCache. Because the
     49            terminated would stay in the cache (even though WebProcessProxy::shutDown() was called), we could potentially
     50            try and use it later on for a navigation to the same domain. This would lead to crashes because
     51            WebProcessProxy::shutDown() has already been called.
     52
     53            Note that we were previously correctly removing the process from the cache in case of a proper crash, inside
     54            WebProcessProxy::processDidTerminateOrFailedToLaunch(). I have moved the logic to remove from the cache
     55            from processDidTerminateOrFailedToLaunch() to shutDown() to avoid similar issues in the future.
     56
     57            * UIProcess/API/Cocoa/WKProcessPool.mm:
     58            (-[WKProcessPool _requestWebProcessTermination:]):
     59            * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
     60            * UIProcess/WebProcessProxy.cpp:
     61            (WebKit::WebProcessProxy::shutDown):
     62            (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
     63
    1642021-03-01  Alan Coon  <alancoon@apple.com>
    265

  • branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

    r270997 r273749  
    384384}
    385385
     386- (BOOL)_requestWebProcessTermination:(pid_t)pid
     387{
     388    for (auto& process : _processPool->processes()) {
     389        if (process->processIdentifier() == pid)
     390            process->requestTermination(WebKit::ProcessTerminationReason::RequestedByClient);
     391        return YES;
     392    }
     393    return NO;
     394}
     395
    386396- (void)_makeNextWebProcessLaunchFailForTesting
    387397{

  • branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h

    r272291 r273749  
    111111- (void)_setUseSeparateServiceWorkerProcess:(BOOL)forceServiceWorkerProcess WK_API_AVAILABLE(macos(10.15.4), ios(13.4));
    112112- (pid_t)_gpuProcessIdentifier WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
     113- (BOOL)_requestWebProcessTermination:(pid_t)pid WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
    113114
    114115// Test only. Returns web processes running web pages (does not include web processes running service workers)

  • branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r273395 r273749  
    449449    RELEASE_ASSERT(isMainThreadOrCheckDisabled());
    450450
     451    if (m_isInProcessCache) {
     452        processPool().webProcessCache().removeProcess(*this, WebProcessCache::ShouldShutDownProcess::No);
     453        ASSERT(!m_isInProcessCache);
     454    }
     455
    451456    shutDownProcess();
    452457
    … …  
    865870    for (auto& callback : isResponsiveCallbacks)
    866871        callback(false);
    867 
    868     if (m_isInProcessCache) {
    869         processPool().webProcessCache().removeProcess(*this, WebProcessCache::ShouldShutDownProcess::No);
    870         ASSERT(!m_isInProcessCache);
    871     }
    872872
    873873    if (isStandaloneServiceWorkerProcess())

  • branches/safari-611.1.21.1-branch/Tools/ChangeLog

    r273668 r273749  
     12021-03-02  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r273695. rdar://problem/74940739
     4
     5    Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
     6    https://bugs.webkit.org/show_bug.cgi?id=222574
     7    <rdar://68340471>
     8   
     9    Reviewed by Geoffrey Garen.
     10   
     11    Source/WebKit:
     12   
     13    The issue was that when WebProcessProxy::requestTermination() was called (e.g. process is killed by WebKit
     14    for using too much memory), we would fail to remove the process from the WebProcessCache. Because the
     15    terminated would stay in the cache (even though WebProcessProxy::shutDown() was called), we could potentially
     16    try and use it later on for a navigation to the same domain. This would lead to crashes because
     17    WebProcessProxy::shutDown() has already been called.
     18   
     19    Note that we were previously correctly removing the process from the cache in case of a proper crash, inside
     20    WebProcessProxy::processDidTerminateOrFailedToLaunch(). I have moved the logic to remove from the cache
     21    from processDidTerminateOrFailedToLaunch() to shutDown() to avoid similar issues in the future.
     22   
     23    * UIProcess/API/Cocoa/WKProcessPool.mm:
     24    (-[WKProcessPool _requestWebProcessTermination:]):
     25    * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
     26    * UIProcess/WebProcessProxy.cpp:
     27    (WebKit::WebProcessProxy::shutDown):
     28    (WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
     29   
     30    Tools:
     31   
     32    Add API test coverage.
     33   
     34    * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     35   
     36   
     37    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273695 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     38
     39    2021-03-01  Chris Dumez  <cdumez@apple.com>
     40
     41            Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
     42            https://bugs.webkit.org/show_bug.cgi?id=222574
     43            <rdar://68340471>
     44
     45            Reviewed by Geoffrey Garen.
     46
     47            Add API test coverage.
     48
     49            * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     50
    1512021-03-01  Kocsen Chung  <kocsen_chung@apple.com>
    252

  • branches/safari-611.1.21.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

    r270638 r273749  
    36903690}
    36913691
     3692TEST(ProcessSwap, ProcessCrashedWhileInTheCache)
     3693{
     3694    auto processPoolConfiguration = psonProcessPoolConfiguration();
     3695    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     3696
     3697    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
     3698    [webViewConfiguration setProcessPool:processPool.get()];
     3699    auto handler = adoptNS([[PSONScheme alloc] init]);
     3700    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
     3701
     3702    auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
     3703    [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
     3704        done = true;
     3705    }];
     3706
     3707    int webkitPID = 0;
     3708
     3709    @autoreleasepool {
     3710        auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     3711        [webView setNavigationDelegate:navigationDelegate.get()];
     3712
     3713        NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     3714
     3715        [webView loadRequest:request];
     3716        TestWebKitAPI::Util::run(&done);
     3717        done = false;
     3718        webkitPID = [webView _webProcessIdentifier];
     3719    }
     3720
     3721    while ([processPool _processCacheSize] != 1)
     3722        TestWebKitAPI::Util::sleep(0.1);
     3723
     3724    kill(webkitPID, 9);
     3725
     3726    while ([processPool _processCacheSize])
     3727        TestWebKitAPI::Util::sleep(0.1);
     3728
     3729    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     3730    [webView setNavigationDelegate:navigationDelegate.get()];
     3731
     3732    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     3733
     3734    [webView loadRequest:request];
     3735    TestWebKitAPI::Util::run(&done);
     3736    done = false;
     3737
     3738    EXPECT_NE(webkitPID, [webView _webProcessIdentifier]);
     3739}
     3740
     3741TEST(ProcessSwap, ProcessTerminatedWhileInTheCache)
     3742{
     3743    auto processPoolConfiguration = psonProcessPoolConfiguration();
     3744    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     3745
     3746    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
     3747    [webViewConfiguration setProcessPool:processPool.get()];
     3748    auto handler = adoptNS([[PSONScheme alloc] init]);
     3749    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
     3750
     3751    auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
     3752    [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
     3753        done = true;
     3754    }];
     3755
     3756    int webkitPID = 0;
     3757
     3758    @autoreleasepool {
     3759        auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     3760        [webView setNavigationDelegate:navigationDelegate.get()];
     3761
     3762        NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     3763
     3764        [webView loadRequest:request];
     3765        TestWebKitAPI::Util::run(&done);
     3766        done = false;
     3767        webkitPID = [webView _webProcessIdentifier];
     3768    }
     3769
     3770    while ([processPool _processCacheSize] != 1)
     3771        TestWebKitAPI::Util::sleep(0.1);
     3772
     3773    EXPECT_TRUE([processPool _requestWebProcessTermination:webkitPID]);
     3774    TestWebKitAPI::Util::spinRunLoop(100);
     3775
     3776    EXPECT_EQ(0U, [processPool _processCacheSize]);
     3777
     3778    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     3779    [webView setNavigationDelegate:navigationDelegate.get()];
     3780
     3781    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     3782
     3783    [webView loadRequest:request];
     3784    TestWebKitAPI::Util::run(&done);
     3785    done = false;
     3786
     3787    EXPECT_NE(webkitPID, [webView _webProcessIdentifier]);
     3788}
     3789
    36923790TEST(ProcessSwap, UseWebProcessCacheForLoadInNewView)
    36933791{
Note: See TracChangeset for help on using the changeset viewer.