Title: [269256] trunk/Source/WebKit
- Revision
- 269256
- Author
- [email protected]
- Date
- 2020-11-02 11:01:54 -0800 (Mon, 02 Nov 2020)
Log Message
Crash under ProcessThrottler::setAssertionType()
https://bugs.webkit.org/show_bug.cgi?id=218448
<rdar://problem/67419221>
Reviewed by Geoffrey Garen.
A ProcessThrottler object is owned by its associated AuxiliaryProcessProxy. The crash was happening
in ProcessThrottler::setAssertionType(), we would replace m_assertion with a new "Suspended"
assertion and then crash on the next line when using m_assertion. The reason we crash is that
when we replaced m_assertion with a new assertion, the destruction of the previous assertion
caused the UIProcess's background task to get released (because this was the last non-suspended
process assertion). When we release the UIProcess' background task, we call
WebProcessPool::notifyProcessPoolsApplicationIsAboutToSuspend(), which destroys non-critical
WebProcesses (e.g. WebProcesses in the back/forward cache), which in turns destroy their
ProcessThrottler. As a result, when replacing m_assertion, the ProcessAssertion may get
destroyed, which is why we would crash on the next line when trying to use m_assertion.
To address the issue, we now release the UIProcess's background task asynchronously when
releasing the last non-suspended ProcessAssertion, making sure we still need to release
the assertion beforehand. This has 2 benefits:
- The ProcessThrottler can no longer get destroyed synchronously when releasing its
ProcessAssertion.
- In the case where the ProcessThrottler replaces a foreground assertion with a background
assertion (or vice-versa) and this is the last non-suspended assertion, this avoids
unnecessarily releasing and retaking the UIProcess's background task. This also avoids
killing non-critical processes unnecessarily.
* UIProcess/ios/ProcessAssertionIOS.mm:
(-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (269255 => 269256)
--- trunk/Source/WebKit/ChangeLog 2020-11-02 19:01:36 UTC (rev 269255)
+++ trunk/Source/WebKit/ChangeLog 2020-11-02 19:01:54 UTC (rev 269256)
@@ -1,3 +1,35 @@
+2020-11-02 Chris Dumez <[email protected]>
+
+ Crash under ProcessThrottler::setAssertionType()
+ https://bugs.webkit.org/show_bug.cgi?id=218448
+ <rdar://problem/67419221>
+
+ Reviewed by Geoffrey Garen.
+
+ A ProcessThrottler object is owned by its associated AuxiliaryProcessProxy. The crash was happening
+ in ProcessThrottler::setAssertionType(), we would replace m_assertion with a new "Suspended"
+ assertion and then crash on the next line when using m_assertion. The reason we crash is that
+ when we replaced m_assertion with a new assertion, the destruction of the previous assertion
+ caused the UIProcess's background task to get released (because this was the last non-suspended
+ process assertion). When we release the UIProcess' background task, we call
+ WebProcessPool::notifyProcessPoolsApplicationIsAboutToSuspend(), which destroys non-critical
+ WebProcesses (e.g. WebProcesses in the back/forward cache), which in turns destroy their
+ ProcessThrottler. As a result, when replacing m_assertion, the ProcessAssertion may get
+ destroyed, which is why we would crash on the next line when trying to use m_assertion.
+
+ To address the issue, we now release the UIProcess's background task asynchronously when
+ releasing the last non-suspended ProcessAssertion, making sure we still need to release
+ the assertion beforehand. This has 2 benefits:
+ - The ProcessThrottler can no longer get destroyed synchronously when releasing its
+ ProcessAssertion.
+ - In the case where the ProcessThrottler replaces a foreground assertion with a background
+ assertion (or vice-versa) and this is the last non-suspended assertion, this avoids
+ unnecessarily releasing and retaking the UIProcess's background task. This also avoids
+ killing non-critical processes unnecessarily.
+
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
+
2020-11-02 Brent Fulgham <[email protected]>
[macOS] Remove unneeded shmem access to ColorSync
Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (269255 => 269256)
--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2020-11-02 19:01:36 UTC (rev 269255)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2020-11-02 19:01:54 UTC (rev 269256)
@@ -179,8 +179,14 @@
RELEASE_LOG_ERROR(ProcessSuspension, "WKProcessAssertionBackgroundTaskManager: Failed to acquire FinishTaskInterruptable assertion for own process, error: %{public}@", acquisitionError);
else
RELEASE_LOG(ProcessSuspension, "WKProcessAssertionBackgroundTaskManager: Successfully took a FinishTaskInterruptable assertion for own process");
- } else if (_assertionsNeedingBackgroundTask.computesEmpty())
- [self _releaseBackgroundTask];
+ } else if (_assertionsNeedingBackgroundTask.computesEmpty()) {
+ // Release the background task asynchronously because releasing the background task may destroy the ProcessThrottler and we don't
+ // want it to get destroyed while in the middle of updating its assertion.
+ dispatch_async(dispatch_get_main_queue(), ^{
+ if (_assertionsNeedingBackgroundTask.computesEmpty())
+ [self _releaseBackgroundTask];
+ });
+ }
}
- (void)assertionWillInvalidate:(RBSAssertion *)assertion
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
