<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">On 11/5/2021 12:32 AM, Randell Jesup
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:cc58a718-872e-8a50-0a86-7ae4c1ab3cbb@mozilla.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div class="moz-cite-prefix">On 11/4/2021 6:55 PM, Aaron Puchert
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:db5e0086-f352-0c9c-5e16-d32a4d244585@alice-dsl.net">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<div class="moz-cite-prefix">Am 04.11.21 um 04:40 schrieb
Randell Jesup:<br>
</div>
<blockquote type="cite"
cite="mid:1d5a8238-fc4f-91cc-d121-e9042f9ff5c3@mozilla.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<p><br>
</p>
Well, the example I gave doesn't match that, I believe. This
is a value only written on Thread 1, and read on Thread 1 and
on other threads. The requirements for locking would be the
lock around all writes (on thread 1), and lock around all
reads from threads *other* than thread 1. Reads from Thread 1
don't need to lock, since they're on the only thread that ever
writes to the value. This is conceptually GUARDED_BY(mMutex
|| OnThread1).<br>
<p>The case you're referencing sounds more like "value is
freely written and read on thread 1, then at some point
later access to it is provided to other threads, and after
that happens access is controlled by a mutex" - where the
protection regime changes over time (and may also correlate
with specific methods, like an Init() method).<br>
</p>
</blockquote>
<p>You're absolutely right, I was misreading your case.</p>
<p>What you're describing sounds in fact like thread 1 always
has a shared lock, which it sometimes promotes to exclusive
lock, then demotes to a shared lock again. The other threads
can only acquire shared locks, because thread 1 doesn't ever
release its shared lock. Now there are multiple ways to write
that down:</p>
</blockquote>
<p><br>
</p>
<p>You're correct, that's a reasonable way to model this pattern
using locks for everything. The pattern I mentioned is
data-race safe (it shouldn't trigger tsan alerts), but doesn't
allow for validation of safety. It depends on the developers
following the rules for that variable.<br>
</p>
</blockquote>
<p><br>
</p>
<p>The downside of using a rwlock is performance and fairness; c++
doesn't guarantee any specific fairness to rwlocks, and they have
lower performance/higher complexity than normal locks/mutexes.
In particular, an implementation could let readers starve
writers... though if the same thread holds a read lock, can it
obtain a write lock? Only I think in rwlocks that allow
upgrades. Otherwise it must release the read lock, grab the write
lock, then release it and get the read lock back.<br>
</p>
<p>The other alternatives are to lock exclusively around each access
(including on the writing thread), or ignore static analysis
warnings if we're on the writing thread, or annotate with an
assertion that quiets the warnings, such as
AssertOnWritingThread(), which has
ASSERT_EXCLUSIVE_LOCK/ASSERT_CAPABILITY (perhaps only in DEBUG
mode).<br>
</p>
<blockquote type="cite"
cite="mid:cc58a718-872e-8a50-0a86-7ae4c1ab3cbb@mozilla.com">
<p> </p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:db5e0086-f352-0c9c-5e16-d32a4d244585@alice-dsl.net">
<blockquote type="cite"
cite="mid:1d5a8238-fc4f-91cc-d121-e9042f9ff5c3@mozilla.com">
<p> </p>
<p>We have another pattern that's interesting, where a method
may use Maybe<MutexAutoLock> lock(std::in_place,
mMutex); (Maybe<> is effectively std::optional).
This allows us to lock.reset() instead of using
MutexAutoUnlock lock(mLock), which will lead to an extra
lock/unlock pair compared to Maybe<> and lock.reset().</p>
</blockquote>
<p>Supporting that would be hard, because we'd need to look into
the Maybe type, and then the analysis isn't local anymore.
However, the analysis supports “premature unlocking” of
scopes, i.e. you can write lock.Unlock(). I've even added
support for “relockable scopes”, where you can lock.Lock()
again. (Have a look at the MutexLocker in <a
class="moz-txt-link-freetext"
href="https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutex-h"
moz-do-not-send="true">https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutex-h</a>
to see what is possible.)</p>
</blockquote>
<p><br>
</p>
<p>I've checked, and the reason for this pattern is basically as I
mentioned; to allow for unlocking of an RAII lock without an
added lock/unlock pair on exit of the scope. If we can do
"MutexAutoLock lock(mMutex); ...; if (foo) { lock.Unlock();
MethodThatWeCantHoldLockIn(); return; } ..." then we don't need
the Maybe<> stuff.</p>
</blockquote>
<p><br>
</p>
<p>Writing a MaybeMutexAutoLock class actually works fairly well for
this case, and avoids Maybe<>/std::optional, and lets us
avoid the extra bool for all the MutexAutoLocks (which appear
~50-100x+ the frequency of MaybeMutexAutoLock).</p>
<p>Randell Jesup, Mozilla<br>
</p>
</body>
</html>