Bug 226145

Summary: Drop WTF::tryHoldLock() as it is incompatible with Clang Thread Safety Analysis
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web Template FrameworkAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, berto, calvaris, cgarcia, changseok, cmarcelo, darin, ddkilzer, dino, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, graouts, gustavo, gyuyoung.kim, hi, hta, jer.noble, joepeck, keith_miller, kkinnunen, kondapallykalyan, mark.lam, menard, msaboff, philipj, pnormand, saam, sam, sergio, tommyw, tzagallo, vjaquez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
Bug Depends on:    
Bug Blocks: 226157    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch darin: review+

Chris Dumez
Reported 2021-05-22 14:56:12 PDT
Drop WTF::tryHoldLock() as it is incompatible with Clang Thread Safety Analysis. Instead, use the following pattern: ``` Locker locker { DeferLock, m_lock }; if (locker.tryLock()) ```
Attachments
Patch (56.77 KB, patch)
2021-05-22 15:37 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (56.98 KB, patch)
2021-05-22 15:49 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (56.77 KB, patch)
2021-05-22 16:00 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (56.78 KB, patch)
2021-05-22 16:16 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (57.67 KB, patch)
2021-05-22 16:23 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (56.82 KB, patch)
2021-05-22 16:32 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (56.78 KB, patch)
2021-05-22 16:43 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (57.04 KB, patch)
2021-05-22 16:47 PDT, Chris Dumez
no flags
Patch (56.81 KB, patch)
2021-05-22 18:08 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (25.77 KB, patch)
2021-05-22 18:51 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (26.00 KB, patch)
2021-05-22 19:01 PDT, Chris Dumez
no flags
Patch (35.15 KB, patch)
2021-05-22 19:18 PDT, Chris Dumez
no flags
Patch (34.80 KB, patch)
2021-05-22 19:20 PDT, Chris Dumez
darin: review+
Chris Dumez
Comment 1 2021-05-22 15:37:40 PDT
EWS Watchlist
Comment 2 2021-05-22 15:38:23 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Chris Dumez
Comment 3 2021-05-22 15:49:43 PDT
Chris Dumez
Comment 4 2021-05-22 16:00:36 PDT
Chris Dumez
Comment 5 2021-05-22 16:16:59 PDT
Chris Dumez
Comment 6 2021-05-22 16:23:56 PDT
Chris Dumez
Comment 7 2021-05-22 16:32:35 PDT
Chris Dumez
Comment 8 2021-05-22 16:43:13 PDT
Chris Dumez
Comment 9 2021-05-22 16:47:56 PDT
Chris Dumez
Comment 10 2021-05-22 16:59:45 PDT
Not sure what's going on, it builds fine locally :/
Chris Dumez
Comment 11 2021-05-22 18:08:53 PDT
Chris Dumez
Comment 12 2021-05-22 18:51:29 PDT
Chris Dumez
Comment 13 2021-05-22 19:01:06 PDT
Chris Dumez
Comment 14 2021-05-22 19:18:59 PDT
Chris Dumez
Comment 15 2021-05-22 19:20:29 PDT
Darin Adler
Comment 16 2021-05-23 12:44:36 PDT
Comment on attachment 429449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429449&action=review > Source/WTF/wtf/Locker.h:59 > +constexpr AdoptLockTag AdoptLock = AdoptLockTag(); I am surprised that we need the "= AdoptLockTag()" here. Can we omit it?
Chris Dumez
Comment 17 2021-05-23 12:53:14 PDT
(In reply to Darin Adler from comment #16) > Comment on attachment 429449 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429449&action=review > > > Source/WTF/wtf/Locker.h:59 > > +constexpr AdoptLockTag AdoptLock = AdoptLockTag(); > > I am surprised that we need the "= AdoptLockTag()" here. Can we omit it? Will check and land without it if it builds. I had copied the definition from <mutex>: /* _LIBCPP_INLINE_VAR */ constexpr adopt_lock_t adopt_lock = adopt_lock_t();
Chris Dumez
Comment 18 2021-05-23 12:55:44 PDT
Radar WebKit Bug Importer
Comment 19 2021-05-23 12:56:30 PDT
Chris Dumez
Comment 20 2021-05-23 12:56:37 PDT
(In reply to Chris Dumez from comment #17) > (In reply to Darin Adler from comment #16) > > Comment on attachment 429449 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=429449&action=review > > > > > Source/WTF/wtf/Locker.h:59 > > > +constexpr AdoptLockTag AdoptLock = AdoptLockTag(); > > > > I am surprised that we need the "= AdoptLockTag()" here. Can we omit it? > > Will check and land without it if it builds. I had copied the definition > from <mutex>: > /* _LIBCPP_INLINE_VAR */ constexpr adopt_lock_t adopt_lock = > adopt_lock_t(); Yes, it built fine without it.
Note You need to log in before you can comment on or make changes to this bug.