- Revision
- 249703
- Author
- [email protected]
- Date
- 2019-09-09 21:24:23 -0700 (Mon, 09 Sep 2019)
Log Message
[iOS] We sometimes attempt to use a terminated prewarmed WebContent process
https://bugs.webkit.org/show_bug.cgi?id=201614
<rdar://problem/54714507>
Reviewed by Geoffrey Garen.
On iOS, it is possible for our processes to get terminated (e.g. jetsammed) while the UIProcess
is suspended. Upon resuming, it takes a little while for the UIProcess to get the notification
that the mac connection to its child process has been severed and the UIProcess may try to use
it for a load. This is especially problematic for prewarmed process because the client will end
up showing a crash banner and reloading when we could have used a new process rather the prewarmed
one if we had known it was dead.
This patch makes 2 improvements:
1. It makes AuxiliaryProcessProxy::state() return Terminated if we still have a connection but
the PID is not the PID of a running process. I also added a check in tryTakePrewarmedProcess()
to not use the prewarmed process if it state() is Terminated.
2. When the UIProcess is about to get suspended, have the process pools terminate their non-critical
processes (i.e. prewarmed + the ones used for PageCache). This makes WebKit friendlier with
other apps on the system when suspended with regards to memory. Also, it makes it less likely
useful WebContent processes will get jetsammed.
* UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::state const):
(WebKit::AuxiliaryProcessProxy::isRunningProcessPID):
* UIProcess/AuxiliaryProcessProxy.h:
* UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::applicationIsAboutToSuspend):
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::tryTakePrewarmedProcess):
* UIProcess/WebProcessPool.h:
* UIProcess/ios/ProcessAssertionIOS.mm:
(-[WKProcessAssertionBackgroundTaskManager init]):
(-[WKProcessAssertionBackgroundTaskManager _releaseBackgroundTask]):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (249702 => 249703)
--- trunk/Source/WebKit/ChangeLog 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/ChangeLog 2019-09-10 04:24:23 UTC (rev 249703)
@@ -1,5 +1,42 @@
2019-09-09 Chris Dumez <[email protected]>
+ [iOS] We sometimes attempt to use a terminated prewarmed WebContent process
+ https://bugs.webkit.org/show_bug.cgi?id=201614
+ <rdar://problem/54714507>
+
+ Reviewed by Geoffrey Garen.
+
+ On iOS, it is possible for our processes to get terminated (e.g. jetsammed) while the UIProcess
+ is suspended. Upon resuming, it takes a little while for the UIProcess to get the notification
+ that the mac connection to its child process has been severed and the UIProcess may try to use
+ it for a load. This is especially problematic for prewarmed process because the client will end
+ up showing a crash banner and reloading when we could have used a new process rather the prewarmed
+ one if we had known it was dead.
+
+ This patch makes 2 improvements:
+ 1. It makes AuxiliaryProcessProxy::state() return Terminated if we still have a connection but
+ the PID is not the PID of a running process. I also added a check in tryTakePrewarmedProcess()
+ to not use the prewarmed process if it state() is Terminated.
+ 2. When the UIProcess is about to get suspended, have the process pools terminate their non-critical
+ processes (i.e. prewarmed + the ones used for PageCache). This makes WebKit friendlier with
+ other apps on the system when suspended with regards to memory. Also, it makes it less likely
+ useful WebContent processes will get jetsammed.
+
+ * UIProcess/AuxiliaryProcessProxy.cpp:
+ (WebKit::AuxiliaryProcessProxy::state const):
+ (WebKit::AuxiliaryProcessProxy::isRunningProcessPID):
+ * UIProcess/AuxiliaryProcessProxy.h:
+ * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
+ (WebKit::WebProcessPool::applicationIsAboutToSuspend):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::tryTakePrewarmedProcess):
+ * UIProcess/WebProcessPool.h:
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKProcessAssertionBackgroundTaskManager init]):
+ (-[WKProcessAssertionBackgroundTaskManager _releaseBackgroundTask]):
+
+2019-09-09 Chris Dumez <[email protected]>
+
REGRESSION: http/tests/resourceLoadStatistics/do-not-capture-statistics-for-simple-top-navigations.html is frequently timing out on iOS EWS bots
https://bugs.webkit.org/show_bug.cgi?id=201550
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-10 04:24:23 UTC (rev 249703)
@@ -28,6 +28,7 @@
#include "AuxiliaryProcessMessages.h"
#include "LoadParameters.h"
+#include "Logging.h"
#include "WebPageMessages.h"
#include <wtf/RunLoop.h>
@@ -108,12 +109,37 @@
if (m_processLauncher && m_processLauncher->isLaunching())
return AuxiliaryProcessProxy::State::Launching;
- if (!m_connection)
+ // There is sometimes a delay until we get the notification from mach about the connection getting closed.
+ // To help detect terminated process earlier, we also check that the PID is for a valid running process.
+ if (!m_connection || !isRunningProcessPID(processIdentifier()))
return AuxiliaryProcessProxy::State::Terminated;
return AuxiliaryProcessProxy::State::Running;
}
+bool AuxiliaryProcessProxy::isRunningProcessPID(ProcessID pid)
+{
+ if (!pid)
+ return false;
+
+#if PLATFORM(COCOA)
+ // Use kill() with a signal of 0 to check if there is actually still a process with the given PID.
+ if (!kill(pid, 0))
+ return true;
+
+ if (errno == ESRCH) {
+ // No process can be found corresponding to that specified by pid.
+ return false;
+ }
+
+ RELEASE_LOG_ERROR(Process, "kill() returned unexpected error %d", errno);
+ return true;
+#else
+ UNUSED_PARAM(pid);
+ return true;
+#endif
+}
+
bool AuxiliaryProcessProxy::sendMessage(std::unique_ptr<IPC::Encoder> encoder, OptionSet<IPC::SendOption> sendOptions)
{
switch (state()) {
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-09-10 04:24:23 UTC (rev 249703)
@@ -123,6 +123,7 @@
private:
virtual void connectionWillOpen(IPC::Connection&);
virtual void processWillShutDown(IPC::Connection&) = 0;
+ static bool isRunningProcessPID(ProcessID);
Vector<std::pair<std::unique_ptr<IPC::Encoder>, OptionSet<IPC::SendOption>>> m_pendingMessages;
RefPtr<ProcessLauncher> m_processLauncher;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm 2019-09-10 04:24:23 UTC (rev 249703)
@@ -29,6 +29,7 @@
#import "AccessibilitySupportSPI.h"
#import "CookieStorageUtilsCF.h"
#import "LegacyCustomProtocolManagerClient.h"
+#import "Logging.h"
#import "NetworkProcessCreationParameters.h"
#import "NetworkProcessMessages.h"
#import "NetworkProcessProxy.h"
@@ -580,4 +581,15 @@
return qos;
}
+#if PLATFORM(IOS_FAMILY)
+void WebProcessPool::applicationIsAboutToSuspend()
+{
+ RELEASE_LOG(ProcessSuspension, "Application is about to suspend so we simulate memory pressure to terminate non-critical processes");
+ // Simulate memory pressure handling so free as much memory as possible before suspending.
+ // In particular, this will terminate prewarmed and PageCache processes.
+ for (auto* processPool : allProcessPools())
+ processPool->handleMemoryPressureWarning(Critical::Yes);
+}
+#endif
+
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-09-10 04:24:23 UTC (rev 249703)
@@ -825,6 +825,14 @@
{
if (!m_prewarmedProcess)
return nullptr;
+
+ // There is sometimes a delay until we get notified that a prewarmed process has been terminated (e.g. after resuming
+ // from suspension) so make sure the process is still running here before deciding to use it.
+ if (m_prewarmedProcess->state() == AuxiliaryProcessProxy::State::Terminated) {
+ RELEASE_LOG_ERROR(Process, "Not using prewarmed process %d because it has been terminated", m_prewarmedProcess->processIdentifier());
+ m_prewarmedProcess = nullptr;
+ return nullptr;
+ }
#if PLATFORM(GTK) || PLATFORM(WPE)
// In platforms using Bubblewrap for sandboxing, prewarmed process is launched using the WebProcessPool primary WebsiteDataStore,
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-09-10 04:24:23 UTC (rev 249703)
@@ -222,6 +222,10 @@
void populateVisitedLinks();
+#if PLATFORM(IOS_FAMILY)
+ static void applicationIsAboutToSuspend();
+#endif
+
void handleMemoryPressureWarning(Critical);
#if ENABLE(NETSCAPE_PLUGIN_API)
Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (249702 => 249703)
--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-09-10 04:06:42 UTC (rev 249702)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-09-10 04:24:23 UTC (rev 249703)
@@ -30,6 +30,7 @@
#import "AssertionServicesSPI.h"
#import "Logging.h"
+#import "WebProcessPool.h"
#import <UIKit/UIApplication.h>
#import <wtf/HashMap.h>
#import <wtf/RunLoop.h>
@@ -81,6 +82,9 @@
[[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidEnterBackgroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
_applicationIsBackgrounded = YES;
+
+ if (_backgroundTask == UIBackgroundTaskInvalid)
+ WebKit::WebProcessPool::applicationIsAboutToSuspend();
}];
return self;
@@ -183,6 +187,8 @@
return;
RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - endBackgroundTask", self);
+ if (_applicationIsBackgrounded)
+ WebKit::WebProcessPool::applicationIsAboutToSuspend();
[[UIApplication sharedApplication] endBackgroundTask:_backgroundTask];
_backgroundTask = UIBackgroundTaskInvalid;
}
