Summary: | [WK2][iOS] UIProcess may get killed because it is taking too long to release its background task after expiration | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, beidson, commit-queue, ggaren, sihui_liu, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2019-05-14 13:49:50 PDT
Logging with the patch enabled, when our background assertion expires: 2019-05-14 14:26:18.125569-0700 0x3747f Error 0x95005 9797 0 MobileSafari: (WebKit) [com.apple.WebKit:ProcessSuspension] Background task expired while holding WebKit ProcessAssertion (isMainThread? 1). 2019-05-14 14:26:18.127250-0700 0x3747f Default 0x95005 9797 0 MobileSafari: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x123a2bb10 - WKProcessAssertionBackgroundTaskManager - _scheduleReleaseTask because the expiration handler has been called 2019-05-14 14:26:18.128797-0700 0x374a5 Default 0x0 9798 0 com.apple.WebKit.Networking: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x1011f87f8 - NetworkProcess::processWillSuspendImminently() BEGIN 2019-05-14 14:26:18.128898-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x145e15a40 - WebProcess::processWillSuspendImminently() BEGIN 2019-05-14 14:26:18.131060-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::processWillSuspendImminently() BEGIN 2019-05-14 14:26:18.153307-0700 0x374a5 Default 0x0 9798 0 com.apple.WebKit.Networking: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x1011f87f8 - NetworkProcess::processWillSuspendImminently() END 2019-05-14 14:26:18.159562-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] WebProcess 9800 is freezing all layer trees 2019-05-14 14:26:18.159613-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::destroyRenderingResources() took 0.00ms 2019-05-14 14:26:18.161355-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::updateFreezerStatus() isFreezable: 0, success 2019-05-14 14:26:18.161811-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::markAllLayersVolatile() 2019-05-14 14:26:18.161868-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::markAllLayersVolatile() Successfuly marked all layers as volatile 2019-05-14 14:26:18.161909-0700 0x374ba Default 0x0 9800 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x100e134d0 - WebProcess::processWillSuspendImminently() END 2019-05-14 14:26:18.177892-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] WebProcess 9799 is freezing all layer trees 2019-05-14 14:26:18.177937-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x146810008 - WebPage (PageID=1) - Adding a reason 4 to freeze layer tree; current reasons are 2 2019-05-14 14:26:18.177986-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x145e15a40 - WebProcess::destroyRenderingResources() took 0.01ms 2019-05-14 14:26:18.179802-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x145e15a40 - WebProcess::updateFreezerStatus() isFreezable: 1, success 2019-05-14 14:26:18.179952-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x145e15a40 - WebProcess::markAllLayersVolatile() 2019-05-14 14:26:18.180721-0700 0x374aa Default 0x0 9799 0 com.apple.WebKit.WebContent: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x145e15a40 - WebProcess::processWillSuspendImminently() END 2019-05-14 14:26:20.282348-0700 0x3747f Default 0x95005 9797 0 MobileSafari: (WebKit) [com.apple.WebKit:ProcessSuspension] 0x123a2bb10 - WKProcessAssertionBackgroundTaskManager - endBackgroundTask Looks good. Created attachment 369894 [details]
Patch
Comment on attachment 369894 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369894&action=review r=me > Source/WebKit/ChangeLog:15 > + 1. Send the ProcessWillSuspendImminently asynchronously so that all processes can do their processing in parallel \o/ > Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:123 > + _pendingTaskReleaseTask = dispatch_block_create((dispatch_block_flags_t)0, ^{ Do we need to do any freeing of this block? I also don't understand why you used dispatch_block_create instead of just a BlockPtr and a regular block and setting it to null when you would've called dispatch_block_cancel below > Source/WebKit/WebProcess/WebProcess.h:-421 > - void didReceiveSyncWebProcessMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&); \o/ Comment on attachment 369894 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369894&action=review >> Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:123 >> + _pendingTaskReleaseTask = dispatch_block_create((dispatch_block_flags_t)0, ^{ > > Do we need to do any freeing of this block? I also don't understand why you used dispatch_block_create instead of just a BlockPtr and a regular block and setting it to null when you would've called dispatch_block_cancel below I followed the documentation for dispatch_block_cancel: https://developer.apple.com/documentation/dispatch/1431058-dispatch_block_cancel """ The result of passing NULL or a block object not returned by the dispatch_block_create or dispatch_block_create_with_qos_class function is undefined. """ So it seems I have to use dispatch_block_create() if I want to use dispatch_block_cancel(). I am not 100% sure if I need to do some cleanup to free the block though, I will confirm before landing. Created attachment 369964 [details]
Patch
Comment on attachment 369964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369964&action=review > Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:130 > + Block_release(_pendingTaskReleaseTask); Looks like you're right that I am supposed to release a block constructed by dispatch_block_create() if we're not using ARC so I added a call to Block_release() here. Please take a look. Comment on attachment 369964 [details] Patch Clearing flags on attachment: 369964 Committed r245334: <https://trac.webkit.org/changeset/245334> All reviewed patches have been landed. Closing bug. Comment on attachment 369964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369964&action=review >> Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:130 >> + Block_release(_pendingTaskReleaseTask); > > Looks like you're right that I am supposed to release a block constructed by dispatch_block_create() if we're not using ARC so I added a call to Block_release() here. > Please take a look. Maybe use BlockPtr here? (In reply to Geoffrey Garen from comment #10) > Comment on attachment 369964 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=369964&action=review > > >> Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:130 > >> + Block_release(_pendingTaskReleaseTask); > > > > Looks like you're right that I am supposed to release a block constructed by dispatch_block_create() if we're not using ARC so I added a call to Block_release() here. > > Please take a look. > > Maybe use BlockPtr here? I considered it but I need to call dispatch_block_create() and BlockPtr did not seem to have a way to adopt a block. It always refs the blocks. |