Bug 225228 – CheckedLock::tryLock() use pattern should be more robust

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: Web Template Framework

[Kimmo Kinnunen]

CheckedLock::tryLock() use pattern should be more robust
From bug 224970 discussions

[Kimmo Kinnunen]

Bug 224970
(In reply to Chris Dumez from comment #15)
> 
> My preference would have been:
> TryLocker locker(lock);
> if (!locker.isLocked())
>   return;
> 
> This is also what Blink uses.
> 
> I dislike any kind of adopted lock. I don't think we should be interacting
> with the lock directly or adopting lock as this is a recipe for mistakes.

Blink MutexTryLocker does not support thread safety analysis.
As such it is equivalent of already implemented auto locker = tryHoldLock().

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/wtf/threading_primitives.h



I failed to write a TryLocker that would support current implementation of thread safety analysis.

Suppose we set aside the preferences for syntactical constructs for some time.



The decision of what syntax to use wrt tryLock is based on which problem are you solving:

A) The problem to solve is that caller forgets to adopt the lock after tryLock().
B) The problem to solve is that caller accesses the locked variables without lock.

My (relatively small) experience leads to think that B is the more important problem to solve.

In patch below I'm trying to communicate that the thread safety analysis already covers all of B and most of A.

[Kimmo Kinnunen]

Created attachment 427412 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/77653579>