Changeset 251304 in webkit

Timestamp:

Oct 18, 2019 3:45:42 PM (5 years ago)

Author:

Chris Dumez

Message:

[iOS] "Unexpectedly Resumed" process assertion may cause us to get terminated
​https://bugs.webkit.org/show_bug.cgi?id=203046
<rdar://problem/56179592>

Reviewed by Geoffrey Garen.

This patch implements the following to avoid getting terminated:

1. Schedule the task to release the assertion on a background thread instead of the main thread so that we end up releasing the task even if the main thread is somehow hung.
2. Add an invalidation handler to the process assertion which releases the assertion upon expiration.

• UIProcess/Cocoa/WebProcessProxyCocoa.mm:

(WebKit::WebProcessProxy::processWasUnexpectedlyUnsuspended):

• WebProcess/WebProcess.cpp:

(WebKit::WebProcess::processDidResume):

• WebProcess/WebProcess.h:
• WebProcess/cocoa/WebProcessCocoa.mm:

(WebKit::WebProcess::processTaskStateDidChange):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/Cocoa/WebProcessProxyCocoa.mm (modified) (1 diff)
• WebProcess/WebProcess.cpp (modified) (1 diff)
• WebProcess/WebProcess.h (modified) (2 diffs)
• WebProcess/cocoa/WebProcessCocoa.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-10-18  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] "Unexpectedly Resumed" process assertion may cause us to get terminated
+        https://bugs.webkit.org/show_bug.cgi?id=203046
+        <rdar://problem/56179592>
+
+        Reviewed by Geoffrey Garen.
+
+        This patch implements the following to avoid getting terminated:
+        1. Schedule the task to release the assertion on a background thread instead of
+           the main thread so that we end up releasing the task even if the main thread
+           is somehow hung.
+        2. Add an invalidation handler to the process assertion which releases the assertion
+           upon expiration.
+
+        * UIProcess/Cocoa/WebProcessProxyCocoa.mm:
+        (WebKit::WebProcessProxy::processWasUnexpectedlyUnsuspended):
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::processDidResume):
+        * WebProcess/WebProcess.h:
+        * WebProcess/cocoa/WebProcessCocoa.mm:
+        (WebKit::WebProcess::processTaskStateDidChange):
+
 2019-10-18  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm

@@ -195 +195 @@
     if (m_throttler.shouldBeRunnable()) {
         // The process becoming unsuspended was not unexpected; it likely was notified of its running state
-        // before receiving a procsessDidResume() message from the UIProcess.
+        // before receiving a processDidResume() message from the UIProcess.
         return;
     }

trunk/Source/WebKit/WebProcess/WebProcess.cpp

@@ -1539 +1539 @@
     SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
     accessibilityProcessSuspendedNotification(false);
+    {
+        LockHolder holder(m_unexpectedlyResumedUIAssertionLock);
+        m_unexpectedlyResumedUIAssertion = nullptr;
+    }
 #endif
 

trunk/Source/WebKit/WebProcess/WebProcess.h

@@ -66 +66 @@
 #if PLATFORM(IOS_FAMILY)
 #include "ProcessTaskStateObserver.h"
+OBJC_CLASS BKSProcessAssertion;
 #endif
 
@@ -535 +536 @@
     WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
     Ref<ProcessTaskStateObserver> m_taskStateObserver;
+    Lock m_unexpectedlyResumedUIAssertionLock;
+    RetainPtr<BKSProcessAssertion> m_unexpectedlyResumedUIAssertion;
 #endif
 

trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

@@ -312 +312 @@
         return;
 
+    LockHolder holder(m_unexpectedlyResumedUIAssertionLock);
+    if (m_unexpectedlyResumedUIAssertion)
+        return;
+
     // We were awakened from suspension unexpectedly. Notify the WebProcessProxy, but take a process assertion on our parent PID
     // to ensure that it too is awakened.
-
-    auto uiProcessAssertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:parentProcessConnection()->remoteProcessID() flags:BKSProcessAssertionPreventTaskSuspend reason:BKSProcessAssertionReasonFinishTask name:@"Unexpectedly resumed" withHandler:nil]);
+    m_unexpectedlyResumedUIAssertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:parentProcessConnection()->remoteProcessID() flags:BKSProcessAssertionPreventTaskSuspend reason:BKSProcessAssertionReasonFinishTask name:@"Unexpectedly resumed" withHandler:nil]);
+
+    auto releaseAssertion = [this] {
+        LockHolder holder(m_unexpectedlyResumedUIAssertionLock);
+        if (!m_unexpectedlyResumedUIAssertion)
+            return;
+
+        [m_unexpectedlyResumedUIAssertion invalidate];
+        m_unexpectedlyResumedUIAssertion = nullptr;
+    };
+
+    m_unexpectedlyResumedUIAssertion.get().invalidationHandler = releaseAssertion;
     parentProcessConnection()->send(Messages::WebProcessProxy::ProcessWasUnexpectedlyUnsuspended(), 0);
-    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC)), dispatch_get_main_queue(), [assertion = WTFMove(uiProcessAssertion)] { [assertion invalidate]; });
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC)), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), releaseAssertion);
 }
 #endif