Bug 227875 - Regression(r279601) ProcessAssertion may get destroyed on a background thread
Summary: Regression(r279601) ProcessAssertion may get destroyed on a background thread
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 225324
  Show dependency treegraph
 
Reported: 2021-07-12 08:54 PDT by Chris Dumez
Modified: 2021-07-26 08:49 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.98 KB, patch)
2021-07-12 08:56 PDT, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (2.99 KB, patch)
2021-07-12 09:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (2.18 KB, patch)
2021-07-12 11:47 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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