Bug 206593 - Background thread with ITP Database should lock when the network process is suspended
Summary: Background thread with ITP Database should lock when the network process is s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-22 10:04 PST by Kate Cheney
Modified: 2020-01-23 14:49 PST (History)
9 users (show)

See Also:


Attachments
Patch (34.50 KB, patch)
2020-01-22 10:52 PST, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (34.93 KB, patch)
2020-01-22 17:36 PST, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (5.69 KB, patch)
2020-01-23 13:21 PST, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch for landing (5.65 KB, patch)
2020-01-23 14:06 PST, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kate Cheney 2020-01-22 10:04:00 PST
This is causing WebKit errors when database interaction occurs after the process has been suspended.
Comment 1 Radar WebKit Bug Importer 2020-01-22 10:04:33 PST
<rdar://problem/58800397>
Comment 2 Kate Cheney 2020-01-22 10:05:25 PST
The correct radar is <rdar://problem/58713379>
Comment 3 Kate Cheney 2020-01-22 10:52:17 PST
Created attachment 388440 [details]
Patch
Comment 4 Kate Cheney 2020-01-22 17:36:56 PST
Created attachment 388496 [details]
Patch
Comment 5 Kate Cheney 2020-01-22 17:39:05 PST
Bots were green but this was actually causing flaky API test failures. This patch should fix them.
Comment 6 Chris Dumez 2020-01-23 12:54:37 PST
I suggest we just queue a task to hang the background queue, like we do in StorageManagerSet::suspend(). I believe this would make things a lot simpler.
Comment 7 Kate Cheney 2020-01-23 13:21:53 PST
Created attachment 388587 [details]
Patch
Comment 8 Chris Dumez 2020-01-23 13:49:32 PST
Comment on attachment 388587 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:1167
> +    postTask([this, protectedThis = makeRef(*this), completionHandler = completionHandlerCaller.release()] () mutable {

protectedThis is not needed, postTask() does this for you.
Comment 9 Kate Cheney 2020-01-23 14:01:45 PST
(In reply to Chris Dumez from comment #8)
> Comment on attachment 388587 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388587&action=review
> 
> > Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:1167
> > +    postTask([this, protectedThis = makeRef(*this), completionHandler = completionHandlerCaller.release()] () mutable {
> 
> protectedThis is not needed, postTask() does this for you.

Got it, I'll remove before landing. Thanks!
Comment 10 Kate Cheney 2020-01-23 14:06:40 PST
Created attachment 388589 [details]
Patch for landing
Comment 11 Brent Fulgham 2020-01-23 14:19:51 PST
This is actually:
<rdar://problem/58713379>
Comment 12 WebKit Commit Bot 2020-01-23 14:49:19 PST
Comment on attachment 388589 [details]
Patch for landing

Clearing flags on attachment: 388589

Committed r255039: <https://trac.webkit.org/changeset/255039>
Comment 13 WebKit Commit Bot 2020-01-23 14:49:21 PST
All reviewed patches have been landed.  Closing bug.