[webkit-reviews] review granted: [Bug 226056] Use CheckedLock more in cases where we try-lock : [Attachment 429290] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 226056: Use CheckedLock more in cases where we try-lock
https://bugs.webkit.org/show_bug.cgi?id=226056

Attachment 429290: Patch

https://bugs.webkit.org/attachment.cgi?id=429290&action=review




--- Comment #11 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 429290
  --> https://bugs.webkit.org/attachment.cgi?id=429290
Patch

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

>>> Source/WTF/wtf/Logger.h:359
>>> +	     Locker locker { AdoptLockTag { }, observerLock() };
>> 
>> Can we not use something similar to tryHoldLock?  I'm not sure if this does
the same thing, but to me it looks like we're giving something else a chance to
get the lock between the call to tryLock and observerLock
> 
> No, tryHoldLock and TryLocker-types objects currently don't work with Clang
thread-safety analysis unfortunately. I agree that the new code is not as nice
but at least we get Clang's thread-safety analysis. As far as I can tell
though, there is no race here. I do not understand your concern on that front.
The new code is just as safe AFAICT.

Now I see.  If the tryLock call returns true then the lock is in a locked
state, and then we adopt it with a raii object.  It would be nice if tryLock
could return the raii object, but that's not how std::unique_lock works.