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

Chris Dumez
Reported 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.
Attachments
Patch (2.98 KB, patch)
2021-07-12 08:56 PDT, Chris Dumez 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (2.99 KB, patch)
2021-07-12 09:04 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.18 KB, patch)
2021-07-12 11:47 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2021-07-12 08:54:58 PDT
<rdar://76972252>
Chris Dumez
Comment 2 2021-07-12 08:56:23 PDT
Created attachment 433323 [details] Patch
Chris Dumez
Comment 3 2021-07-12 09:04:51 PDT
Created attachment 433325 [details] Patch
Geoffrey Garen
Comment 4 2021-07-12 10:12:28 PDT
Comment on attachment 433325 [details] Patch r=me
EWS
Comment 5 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].
Chris Dumez
Comment 6 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>
Chris Dumez
Comment 7 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.
Chris Dumez
Comment 8 2021-07-12 11:47:29 PDT
Created attachment 433338 [details] Patch
Chris Dumez
Comment 9 2021-07-12 14:39:31 PDT
Comment on attachment 433338 [details] Patch Requesting review again because the patch has changed substantially.
Geoffrey Garen
Comment 10 2021-07-13 10:07:01 PDT
Comment on attachment 433338 [details] Patch r=me
EWS
Comment 11 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].
Chris Dumez
Comment 12 2021-07-26 08:43:49 PDT
This was actually rdar://problem/80344055
Note You need to log in before you can comment on or make changes to this bug.