- Revision
- 273749
- Author
- [email protected]
- Date
- 2021-03-02 11:51:34 -0800 (Tue, 02 Mar 2021)
Log 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
Modified Paths
Diff
Modified: branches/safari-611.1.21.1-branch/Source/WebKit/ChangeLog (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Source/WebKit/ChangeLog 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Source/WebKit/ChangeLog 2021-03-02 19:51:34 UTC (rev 273749)
@@ -1,3 +1,66 @@
+2021-03-02 Alan Coon <[email protected]>
+
+ 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
+
+ 2021-03-01 Chris Dumez <[email protected]>
+
+ Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
+ https://bugs.webkit.org/show_bug.cgi?id=222574
+ <rdar://68340471>
+
+ Reviewed by Geoffrey Garen.
+
+ 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):
+
2021-03-01 Alan Coon <[email protected]>
Cherry-pick r273564. rdar://problem/74886917
Modified: branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2021-03-02 19:51:34 UTC (rev 273749)
@@ -383,6 +383,16 @@
#endif
}
+- (BOOL)_requestWebProcessTermination:(pid_t)pid
+{
+ for (auto& process : _processPool->processes()) {
+ if (process->processIdentifier() == pid)
+ process->requestTermination(WebKit::ProcessTerminationReason::RequestedByClient);
+ return YES;
+ }
+ return NO;
+}
+
- (void)_makeNextWebProcessLaunchFailForTesting
{
_processPool->setShouldMakeNextWebProcessLaunchFailForTesting(true);
Modified: branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h 2021-03-02 19:51:34 UTC (rev 273749)
@@ -110,6 +110,7 @@
- (void)_clearWebProcessCache WK_API_AVAILABLE(macos(10.15.4), ios(13.4));
- (void)_setUseSeparateServiceWorkerProcess:(BOOL)forceServiceWorkerProcess WK_API_AVAILABLE(macos(10.15.4), ios(13.4));
- (pid_t)_gpuProcessIdentifier WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (BOOL)_requestWebProcessTermination:(pid_t)pid WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
// Test only. Returns web processes running web pages (does not include web processes running service workers)
- (size_t)_webPageContentProcessCount WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
Modified: branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2021-03-02 19:51:34 UTC (rev 273749)
@@ -448,6 +448,11 @@
{
RELEASE_ASSERT(isMainThreadOrCheckDisabled());
+ if (m_isInProcessCache) {
+ processPool().webProcessCache().removeProcess(*this, WebProcessCache::ShouldShutDownProcess::No);
+ ASSERT(!m_isInProcessCache);
+ }
+
shutDownProcess();
if (m_webConnection) {
@@ -865,11 +870,6 @@
for (auto& callback : isResponsiveCallbacks)
callback(false);
- if (m_isInProcessCache) {
- processPool().webProcessCache().removeProcess(*this, WebProcessCache::ShouldShutDownProcess::No);
- ASSERT(!m_isInProcessCache);
- }
-
if (isStandaloneServiceWorkerProcess())
processPool().serviceWorkerProcessCrashed(*this);
Modified: branches/safari-611.1.21.1-branch/Tools/ChangeLog (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Tools/ChangeLog 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Tools/ChangeLog 2021-03-02 19:51:34 UTC (rev 273749)
@@ -1,3 +1,53 @@
+2021-03-02 Alan Coon <[email protected]>
+
+ 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
+
+ 2021-03-01 Chris Dumez <[email protected]>
+
+ Crash under WebProcessPool::pageBeginUsingWebsiteDataStore()
+ https://bugs.webkit.org/show_bug.cgi?id=222574
+ <rdar://68340471>
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2021-03-01 Kocsen Chung <[email protected]>
Cherry-pick r273590. rdar://problem/74881385
Modified: branches/safari-611.1.21.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (273748 => 273749)
--- branches/safari-611.1.21.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2021-03-02 19:41:50 UTC (rev 273748)
+++ branches/safari-611.1.21.1-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2021-03-02 19:51:34 UTC (rev 273749)
@@ -3689,6 +3689,104 @@
EXPECT_EQ(1U, [processPool _webProcessCountIgnoringPrewarmed]);
}
+TEST(ProcessSwap, ProcessCrashedWhileInTheCache)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
+ [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
+ done = true;
+ }];
+
+ int webkitPID = 0;
+
+ @autoreleasepool {
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ [webView setNavigationDelegate:navigationDelegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+ webkitPID = [webView _webProcessIdentifier];
+ }
+
+ while ([processPool _processCacheSize] != 1)
+ TestWebKitAPI::Util::sleep(0.1);
+
+ kill(webkitPID, 9);
+
+ while ([processPool _processCacheSize])
+ TestWebKitAPI::Util::sleep(0.1);
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ [webView setNavigationDelegate:navigationDelegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_NE(webkitPID, [webView _webProcessIdentifier]);
+}
+
+TEST(ProcessSwap, ProcessTerminatedWhileInTheCache)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
+ [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
+ done = true;
+ }];
+
+ int webkitPID = 0;
+
+ @autoreleasepool {
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ [webView setNavigationDelegate:navigationDelegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+ webkitPID = [webView _webProcessIdentifier];
+ }
+
+ while ([processPool _processCacheSize] != 1)
+ TestWebKitAPI::Util::sleep(0.1);
+
+ EXPECT_TRUE([processPool _requestWebProcessTermination:webkitPID]);
+ TestWebKitAPI::Util::spinRunLoop(100);
+
+ EXPECT_EQ(0U, [processPool _processCacheSize]);
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ [webView setNavigationDelegate:navigationDelegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_NE(webkitPID, [webView _webProcessIdentifier]);
+}
+
TEST(ProcessSwap, UseWebProcessCacheForLoadInNewView)
{
auto processPoolConfiguration = psonProcessPoolConfiguration();
