Bug 227875

Summary: Regression(r279601) ProcessAssertion may get destroyed on a background thread
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, darin, ggaren, kkinnunen, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=228281
Bug Depends on:    
Bug Blocks: 225324    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Chris Dumez 2021-07-12 08:54:45 PDT 
r279601 added an internal WorkQueue to ProcessAssertion, so that we could acquire the RunningBoard assertion asynchronously on the background queue. When dispatching to the background queue, we capture |protectedThis|, which means that ProcessAssertion may now get destroyed on the background queue. ProcessAssertion is a main thread object and destroying it on a non-main thread can lead to crashes.
Comment 1 Chris Dumez 2021-07-12 08:54:58 PDT 
<rdar://76972252>
Comment 2 Chris Dumez 2021-07-12 08:56:23 PDT 
Created attachment 433323 [details]
Patch
Comment 3 Chris Dumez 2021-07-12 09:04:51 PDT 
Created attachment 433325 [details]
Patch
Comment 4 Geoffrey Garen 2021-07-12 10:12:28 PDT 
Comment on attachment 433325 [details]
Patch

r=me
Comment 5 EWS 2021-07-12 10:26:07 PDT 
Committed r279835 (239595@main): <https://commits.webkit.org/239595@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433325 [details].
Comment 6 Chris Dumez 2021-07-12 11:07:07 PDT 
Reverted r279835 for reason:

Newly added assertions are wrong

Committed r279840 (239599@main): <https://commits.webkit.org/239599@main>
Comment 7 Chris Dumez 2021-07-12 11:11:04 PDT 
Comment on attachment 433325 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433325&action=review

> Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:324
> +    ASSERT(isMainRunLoop());

WebSQLiteDatabaseTracker takes and releases assertions on a non-main thread (with a lock) so those assertions hit in debug. I am reworking the patch.
Comment 8 Chris Dumez 2021-07-12 11:47:29 PDT 
Created attachment 433338 [details]
Patch
Comment 9 Chris Dumez 2021-07-12 14:39:31 PDT 
Comment on attachment 433338 [details]
Patch

Requesting review again because the patch has changed substantially.
Comment 10 Geoffrey Garen 2021-07-13 10:07:01 PDT 
Comment on attachment 433338 [details]
Patch

r=me
Comment 11 EWS 2021-07-13 10:10:19 PDT 
Committed r279877 (239630@main): <https://commits.webkit.org/239630@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433338 [details].
Comment 12 Chris Dumez 2021-07-26 08:43:49 PDT 
This was actually rdar://problem/80344055