WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
197893
[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
Summary
[WK2][iOS] UIProcess may get killed because it is taking too long to release ...
Chris Dumez
Reported
2019-05-14 13:49:50 PDT
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 sequentially sends 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.
Attachments
Patch
(14.75 KB, patch)
2019-05-14 14:35 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(14.87 KB, patch)
2019-05-15 10:13 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2019-05-14 13:52:39 PDT
<
rdar://problem/50234105
>
Chris Dumez
Comment 2
2019-05-14 14:30:14 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.
Chris Dumez
Comment 3
2019-05-14 14:35:29 PDT
Created
attachment 369894
[details]
Patch
Alex Christensen
Comment 4
2019-05-15 07:42:03 PDT
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/
Chris Dumez
Comment 5
2019-05-15 08:50:53 PDT
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.
Chris Dumez
Comment 6
2019-05-15 10:13:55 PDT
Created
attachment 369964
[details]
Patch
Chris Dumez
Comment 7
2019-05-15 10:15:48 PDT
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.
WebKit Commit Bot
Comment 8
2019-05-15 11:29:46 PDT
Comment on
attachment 369964
[details]
Patch Clearing flags on attachment: 369964 Committed
r245334
: <
https://trac.webkit.org/changeset/245334
>
WebKit Commit Bot
Comment 9
2019-05-15 11:29:48 PDT
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 10
2019-05-21 10:15:15 PDT
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?
Chris Dumez
Comment 11
2019-05-21 11:26:19 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug