Bug 221027 - REGRESSION(r271929): MobileSafari hangs upon launch due to a trivial deadlock in IOSurfacePool::setPoolSize
Summary: REGRESSION(r271929): MobileSafari hangs upon launch due to a trivial deadlock...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 221010
  Show dependency tree / graph
 
Reported: 2021-01-27 00:48 PST by Ryosuke Niwa
Modified: 2021-01-27 13:17 PST (History)
5 users (show)

See Also:

Attachments
Patch (2.63 KB, patch)
2021-01-27 00:53 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2021-01-27 00:48:30 PST 
We hang inside IOSurfacePool::setPoolSize(unsigned long) via IOSurfacePool::discardAllSurfacesInternal()
as the latter tries to grab the lock while setPoolSize holds a locker.
Comment 1 Ryosuke Niwa 2021-01-27 00:53:27 PST 
Created attachment 418511 [details]
Patch
Comment 2 Ryosuke Niwa 2021-01-27 03:06:22 PST 
Comment on attachment 418511 [details]
Patch

Clearing flags on attachment: 418511

Committed r271943: <https://trac.webkit.org/changeset/271943>
Comment 3 Ryosuke Niwa 2021-01-27 03:06:25 PST 
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2021-01-27 03:07:14 PST 
<rdar://problem/73656115>
Comment 5 Chris Dumez 2021-01-27 07:38:12 PST 
Comment on attachment 418511 [details]
Patch

Thanks for fixing.
Comment 6 Saam Barati 2021-01-27 13:17:02 PST 
Comment on attachment 418511 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp:332
> +void IOSurfacePool::discardAllSurfacesInternal()

nit: A style I like in JSC is we do this by keeping the same function name, and requiring a locker to be passed in.

So, we'd have:

void discardAllSurfaces()
and
void discardAllSurfaces(const LockHolder&)

and the former would call the latter with the locker as an argument. Same with `evict`