Bug 206593

Summary: Background thread with ITP Database should lock when the network process is suspended
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, bfulgham, cdumez, commit-queue, ggaren, katherine_cheney, sihui_liu, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Kate Cheney
Reported 2020-01-22 10:04:00 PST
This is causing WebKit errors when database interaction occurs after the process has been suspended.
Attachments
Patch (34.50 KB, patch)
2020-01-22 10:52 PST, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (34.93 KB, patch)
2020-01-22 17:36 PST, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (5.69 KB, patch)
2020-01-23 13:21 PST, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.65 KB, patch)
2020-01-23 14:06 PST, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-01-22 10:04:33 PST
<rdar://problem/58800397>
Kate Cheney
Comment 2 2020-01-22 10:05:25 PST
The correct radar is <rdar://problem/58713379>
Kate Cheney
Comment 3 2020-01-22 10:52:17 PST
Created attachment 388440 [details] Patch
Kate Cheney
Comment 4 2020-01-22 17:36:56 PST
Created attachment 388496 [details] Patch
Kate Cheney
Comment 5 2020-01-22 17:39:05 PST
Bots were green but this was actually causing flaky API test failures. This patch should fix them.
Chris Dumez
Comment 6 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.
Kate Cheney
Comment 7 2020-01-23 13:21:53 PST
Created attachment 388587 [details] Patch
Chris Dumez
Comment 8 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.
Kate Cheney
Comment 9 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!
Kate Cheney
Comment 10 2020-01-23 14:06:40 PST
Created attachment 388589 [details] Patch for landing
Brent Fulgham
Comment 11 2020-01-23 14:19:51 PST
This is actually: <rdar://problem/58713379>
WebKit Commit Bot
Comment 12 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>
WebKit Commit Bot
Comment 13 2020-01-23 14:49:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.