- Revision
- 245334
- Author
- [email protected]
- Date
- 2019-05-15 11:29:40 -0700 (Wed, 15 May 2019)
Log Message
[WK2][iOS] UIProcess may get killed because it is taking too long to release its background task after expiration
https://bugs.webkit.org/show_bug.cgi?id=197893
<rdar://problem/50234105>
Reviewed by Alex Christensen.
The UIProcess may get killed because it is taking too long to release its background task after its expiration handler
was called. The reason is that the background task's expiration handler was sequentially sending a ProcessWillSuspendImminently
synchronous IPC to each of its child processes and only then ends the background task. By the time we receive the response from
all child processes, it may be too late and we get killed.
To address the issue, we now:
1. Send the ProcessWillSuspendImminently asynchronously so that all processes can do their processing in parallel
2. After 2 seconds, the UIProcess releases the background task (We get killed after ~5 seconds)
Also, to make sure that the UIProcess supends promptly, we now make sure we never start a new background task *after*
the app has been backgrounded. The intention of our background task is too finish critical work (like releasing locked
files) after the app gets backgrounded, not to start new work and delay process suspension.
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::processWillSuspendImminently):
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/NetworkProcess.messages.in:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::sendProcessWillSuspendImminently):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::sendProcessWillSuspendImminently):
* UIProcess/ios/ProcessAssertionIOS.mm:
(-[WKProcessAssertionBackgroundTaskManager init]):
(-[WKProcessAssertionBackgroundTaskManager _scheduleReleaseTask]):
(-[WKProcessAssertionBackgroundTaskManager _cancelPendingReleaseTask]):
(-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::didReceiveSyncMessage):
(WebKit::WebProcess::processWillSuspendImminently):
* WebProcess/WebProcess.h:
* WebProcess/WebProcess.messages.in:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (245333 => 245334)
--- trunk/Source/WebKit/ChangeLog 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/ChangeLog 2019-05-15 18:29:40 UTC (rev 245334)
@@ -1,3 +1,43 @@
+2019-05-15 Chris Dumez <[email protected]>
+
+ [WK2][iOS] UIProcess may get killed because it is taking too long to release its background task after expiration
+ https://bugs.webkit.org/show_bug.cgi?id=197893
+ <rdar://problem/50234105>
+
+ Reviewed by Alex Christensen.
+
+ The UIProcess may get killed because it is taking too long to release its background task after its expiration handler
+ was called. The reason is that the background task's expiration handler was sequentially sending a ProcessWillSuspendImminently
+ synchronous IPC to each of its child processes and only then ends the background task. By the time we receive the response from
+ all child processes, it may be too late and we get killed.
+
+ To address the issue, we now:
+ 1. Send the ProcessWillSuspendImminently asynchronously so that all processes can do their processing in parallel
+ 2. After 2 seconds, the UIProcess releases the background task (We get killed after ~5 seconds)
+
+ Also, to make sure that the UIProcess supends promptly, we now make sure we never start a new background task *after*
+ the app has been backgrounded. The intention of our background task is too finish critical work (like releasing locked
+ files) after the app gets backgrounded, not to start new work and delay process suspension.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::processWillSuspendImminently):
+ * NetworkProcess/NetworkProcess.h:
+ * NetworkProcess/NetworkProcess.messages.in:
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKProcessAssertionBackgroundTaskManager init]):
+ (-[WKProcessAssertionBackgroundTaskManager _scheduleReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _cancelPendingReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::didReceiveSyncMessage):
+ (WebKit::WebProcess::processWillSuspendImminently):
+ * WebProcess/WebProcess.h:
+ * WebProcess/WebProcess.messages.in:
+
2019-05-15 Jiewen Tan <[email protected]>
[WebAuthN] Make WebAuthN default on
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (245333 => 245334)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-05-15 18:29:40 UTC (rev 245334)
@@ -1949,15 +1949,15 @@
#endif
}
-void NetworkProcess::processWillSuspendImminently(CompletionHandler<void(bool)>&& completionHandler)
+void NetworkProcess::processWillSuspendImminently()
{
- RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently()", this);
+ RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently() BEGIN", this);
#if PLATFORM(IOS_FAMILY) && ENABLE(INDEXED_DATABASE)
for (auto& server : m_idbServers.values())
server->tryStop(IDBServer::ShouldForceStop::Yes);
#endif
actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
- completionHandler(true);
+ RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently() END", this);
}
void NetworkProcess::prepareToSuspend()
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.h (245333 => 245334)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-05-15 18:29:40 UTC (rev 245334)
@@ -181,7 +181,7 @@
bool canHandleHTTPSServerTrustEvaluation() const { return m_canHandleHTTPSServerTrustEvaluation; }
- void processWillSuspendImminently(CompletionHandler<void(bool)>&&);
+ void processWillSuspendImminently();
void prepareToSuspend();
void cancelPrepareToSuspend();
void processDidResume();
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in (245333 => 245334)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in 2019-05-15 18:29:40 UTC (rev 245334)
@@ -77,7 +77,7 @@
ProcessDidTransitionToBackground()
ProcessDidTransitionToForeground()
- ProcessWillSuspendImminently() -> (bool handled) Synchronous
+ ProcessWillSuspendImminently()
PrepareToSuspend()
CancelPrepareToSuspend()
ProcessDidResume()
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (245333 => 245334)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2019-05-15 18:29:40 UTC (rev 245334)
@@ -957,11 +957,8 @@
void NetworkProcessProxy::sendProcessWillSuspendImminently()
{
- if (!canSendMessage())
- return;
-
- bool handled = false;
- sendSync(Messages::NetworkProcess::ProcessWillSuspendImminently(), Messages::NetworkProcess::ProcessWillSuspendImminently::Reply(handled), 0, 1_s);
+ if (canSendMessage())
+ send(Messages::NetworkProcess::ProcessWillSuspendImminently(), 0);
}
void NetworkProcessProxy::sendPrepareToSuspend()
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (245333 => 245334)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-05-15 18:29:40 UTC (rev 245334)
@@ -1146,11 +1146,8 @@
void WebProcessProxy::sendProcessWillSuspendImminently()
{
- if (!canSendMessage())
- return;
-
- bool handled = false;
- sendSync(Messages::WebProcess::ProcessWillSuspendImminently(), Messages::WebProcess::ProcessWillSuspendImminently::Reply(handled), 0, 1_s);
+ if (canSendMessage())
+ send(Messages::WebProcess::ProcessWillSuspendImminently(), 0);
}
void WebProcessProxy::sendPrepareToSuspend()
Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (245333 => 245334)
--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-05-15 18:29:40 UTC (rev 245334)
@@ -37,6 +37,11 @@
using WebKit::ProcessAndUIAssertion;
+// This gives some time to our child processes to process the ProcessWillSuspendImminently IPC but makes sure we release
+// the background task before the UIKit timeout (We get killed if we do not release the background task within 5 seconds
+// on the expiration handler getting called).
+static const Seconds releaseBackgroundTaskAfterExpirationDelay { 2_s };
+
@interface WKProcessAssertionBackgroundTaskManager : NSObject
+ (WKProcessAssertionBackgroundTaskManager *)shared;
@@ -50,7 +55,8 @@
{
UIBackgroundTaskIdentifier _backgroundTask;
HashSet<ProcessAndUIAssertion*> _assertionsNeedingBackgroundTask;
- BOOL _assertionHasExpiredInTheBackground;
+ BOOL _applicationIsBackgrounded;
+ dispatch_block_t _pendingTaskReleaseTask;
}
+ (WKProcessAssertionBackgroundTaskManager *)shared
@@ -68,10 +74,15 @@
_backgroundTask = UIBackgroundTaskInvalid;
[[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationWillEnterForegroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
- _assertionHasExpiredInTheBackground = NO;
+ _applicationIsBackgrounded = NO;
+ [self _cancelPendingReleaseTask];
[self _updateBackgroundTask];
}];
+ [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidEnterBackgroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
+ _applicationIsBackgrounded = YES;
+ }];
+
return self;
}
@@ -101,12 +112,40 @@
assertion->uiAssertionWillExpireImminently();
}
+
+- (void)_scheduleReleaseTask
+{
+ ASSERT(!_pendingTaskReleaseTask);
+ if (_pendingTaskReleaseTask)
+ return;
+
+ RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _scheduleReleaseTask because the expiration handler has been called", self);
+ _pendingTaskReleaseTask = dispatch_block_create((dispatch_block_flags_t)0, ^{
+ _pendingTaskReleaseTask = nil;
+ [self _releaseBackgroundTask];
+ });
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, releaseBackgroundTaskAfterExpirationDelay.value() * NSEC_PER_SEC), dispatch_get_main_queue(), _pendingTaskReleaseTask);
+#if !__has_feature(objc_arc)
+ // dispatch_async() does a Block_copy() / Block_release() on behalf of the caller.
+ Block_release(_pendingTaskReleaseTask);
+#endif
+}
+
+- (void)_cancelPendingReleaseTask
+{
+ if (!_pendingTaskReleaseTask)
+ return;
+
+ RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _cancelPendingReleaseTask because the application is foreground again", self);
+ dispatch_block_cancel(_pendingTaskReleaseTask);
+ _pendingTaskReleaseTask = nil;
+}
+
- (void)_updateBackgroundTask
{
if (!_assertionsNeedingBackgroundTask.isEmpty() && _backgroundTask == UIBackgroundTaskInvalid) {
- if (_assertionHasExpiredInTheBackground) {
- RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Ignored request to start a background task because we're still in the background and the previous task expired", self);
- // Our invalidation handler would not get called if we tried to re-take a new background assertion at this point, and the UIProcess would get killed (rdar://problem/50001505).
+ if (_applicationIsBackgrounded) {
+ RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Ignored request to start a new background task because the application is already in the background", self);
return;
}
RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - beginBackgroundTaskWithName", self);
@@ -121,9 +160,7 @@
});
}
- // Remember that the assertion has expired in the background so we do not try to re-take it until the application becomes foreground again.
- _assertionHasExpiredInTheBackground = YES;
- [self _releaseBackgroundTask];
+ [self _scheduleReleaseTask];
}];
} else if (_assertionsNeedingBackgroundTask.isEmpty())
[self _releaseBackgroundTask];
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (245333 => 245334)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2019-05-15 18:29:40 UTC (rev 245334)
@@ -732,8 +732,6 @@
{
if (messageReceiverMap().dispatchSyncMessage(connection, decoder, replyEncoder))
return;
-
- didReceiveSyncWebProcessMessage(connection, decoder, replyEncoder);
}
void WebProcess::didReceiveMessage(IPC::Connection& connection, IPC::Decoder& decoder)
@@ -1490,19 +1488,11 @@
});
}
-void WebProcess::processWillSuspendImminently(CompletionHandler<void(bool)>&& completionHandler)
+void WebProcess::processWillSuspendImminently()
{
- if (parentProcessConnection()->inSendSync()) {
- // Avoid reentrency bugs such as rdar://problem/21605505 by just bailing
- // if we get an incoming ProcessWillSuspendImminently message when waiting for a
- // reply to a sync message.
- // FIXME: ProcessWillSuspendImminently should not be a sync message.
- return completionHandler(false);
- }
-
- RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently()", this);
+ RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently() BEGIN", this);
actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
- completionHandler(true);
+ RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently() END", this);
}
void WebProcess::prepareToSuspend()
Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (245333 => 245334)
--- trunk/Source/WebKit/WebProcess/WebProcess.h 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h 2019-05-15 18:29:40 UTC (rev 245334)
@@ -216,7 +216,7 @@
void setHiddenPageDOMTimerThrottlingIncreaseLimit(int milliseconds);
- void processWillSuspendImminently(CompletionHandler<void(bool)>&&);
+ void processWillSuspendImminently();
void prepareToSuspend();
void cancelPrepareToSuspend();
void processDidResume();
@@ -420,7 +420,6 @@
// Implemented in generated WebProcessMessageReceiver.cpp
void didReceiveWebProcessMessage(IPC::Connection&, IPC::Decoder&);
- void didReceiveSyncWebProcessMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
#if PLATFORM(MAC)
void setScreenProperties(const WebCore::ScreenProperties&);
Modified: trunk/Source/WebKit/WebProcess/WebProcess.messages.in (245333 => 245334)
--- trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2019-05-15 18:20:52 UTC (rev 245333)
+++ trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2019-05-15 18:29:40 UTC (rev 245334)
@@ -96,7 +96,7 @@
EnsureAutomationSessionProxy(String sessionIdentifier)
DestroyAutomationSessionProxy()
- ProcessWillSuspendImminently() -> (bool handled) Synchronous
+ ProcessWillSuspendImminently()
PrepareToSuspend()
CancelPrepareToSuspend()
ProcessDidResume()
