mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-12-01 08:42:13 +00:00
c50cb528fc
Differential Revision: https://phabricator.services.mozilla.com/D152575
355 lines
14 KiB
ReStructuredText
355 lines
14 KiB
ReStructuredText
**Thread safety analysis in Gecko**
|
||
===================================
|
||
|
||
Clang thread-safety analysis is supported in Gecko. This means
|
||
builds will generate warnings when static analysis detects an issue with
|
||
locking of mutex/monitor-protected members and data structures. Note
|
||
that Chrome uses the same feature. An example warning: ::
|
||
|
||
warning: dom/media/AudioStream.cpp:504:22 [-Wthread-safety-analysis]
|
||
reading variable 'mDataSource' requires holding mutex 'mMonitor'
|
||
|
||
If your patch causes warnings like this, you’ll need to resolve them;
|
||
they will be errors on checkin.
|
||
|
||
This analysis depends on thread-safety attributions in the source. These
|
||
have been added to Mozilla’s Mutex and Monitor classes and subclasses,
|
||
but in practice the analysis is largely dependent on additions to the
|
||
code being checked, in particular adding MOZ_GUARDED_BY(mutex) attributions
|
||
on the definitions of member variables. Like this: ::
|
||
|
||
mozilla::Mutex mLock;
|
||
bool mShutdown MOZ_GUARDED_BY(mLock);
|
||
|
||
For background on Clang’s thread-safety support, see `their
|
||
documentation <https://clang.llvm.org/docs/ThreadSafetyAnalysis.html>`__.
|
||
|
||
Newly added Mutexes and Monitors **MUST** use thread-safety annotations,
|
||
and we are enabling static checks to verify this. Legacy uses of Mutexes
|
||
and Monitors are marked with MOZ_UNANNOTATED.
|
||
|
||
If you’re modifying code that has been annotated with
|
||
MOZ_GUARDED_BY()/MOZ_REQUIRES()/etc, you should **make sure that the annotations
|
||
are updated properly**; e.g. if you change the thread-usage of a member
|
||
variable or method you should mark it accordingly, comment, and resolve
|
||
any warnings. Since the warnings will be errors in autoland/m-c, you
|
||
won’t be able to land code with active warnings.
|
||
|
||
**Annotating locking and usage requirements in class definitions**
|
||
------------------------------------------------------------------
|
||
|
||
Values that require a lock to access, or which are simply used from more
|
||
than one thread, should always have documentation in the definition
|
||
about the locking requirements and/or what threads it’s touched from: ::
|
||
|
||
// This array is accessed from both the direct video thread, and the graph
|
||
// thread. Protected by mMutex.
|
||
|
||
nsTArray<std::pair<ImageContainer::FrameID, VideoChunk>> mFrames
|
||
MOZ_GUARDED_BY(mMutex);
|
||
|
||
// Set on MainThread, deleted on either MainThread mainthread, used on
|
||
// MainThread or IO Thread in DoStopSession
|
||
nsCOMPtr<nsITimer> mReconnectDelayTimer MOZ_GUARDED_BY(mMutex);
|
||
|
||
It’s strongly recommended to group values by access pattern, but it’s
|
||
**critical** to make it clear what the requirements to access a value
|
||
are. With values protected by Mutexes and Monitors, adding a
|
||
MOZ_GUARDED_BY(mutex/monitor) should be sufficient, though you may want to
|
||
also document what threads access it, and if they read or write to it.
|
||
|
||
Values which have more complex access requirements (see single-writer
|
||
and time-based-locking below) need clear documentation where they’re
|
||
defined: ::
|
||
|
||
MutexSingleWriter mMutex;
|
||
|
||
// mResource should only be modified on the main thread with the lock.
|
||
// So it can be read without lock on the main thread or on other threads
|
||
// with the lock.
|
||
RefPtr<ChannelMediaResource> mResource MOZ_GUARDED_BY(mMutex);
|
||
|
||
**WARNING:** thread-safety analysis is not magic; it depends on you telling
|
||
it the requirements around access. If you don’t mark something as
|
||
MOZ_GUARDED_BY() it won’t figure it out for you, and you can end up with a data
|
||
race. When writing multithreaded code, you should always be thinking about
|
||
which threads can access what and when, and document this.
|
||
|
||
**How to annotate different locking patterns in Gecko**
|
||
-------------------------------------------------------
|
||
|
||
Gecko uses a number of different locking patterns. They include:
|
||
|
||
- **Always Lock** -
|
||
Multiple threads may read and write the value
|
||
|
||
- **Single Writer** -
|
||
One thread does all the writing, other threads
|
||
read the value, but code on the writing thread also reads it
|
||
without the lock
|
||
|
||
- **Out-of-band invariants** -
|
||
A value may be accessed from other threads,
|
||
but only after or before certain other events or in a certain state,
|
||
like when after a listener has been added or before a processing
|
||
thread has been shut down.
|
||
|
||
The simplest and easiest to check with static analysis is **Always
|
||
Lock**, and generally you should prefer this pattern. This is very
|
||
simple; you add MOZ_GUARDED_BY(mutex/monitor), and must own the lock to
|
||
access the value. This can be implemented by some combination of direct
|
||
Lock/AutoLock calls in the method; an assertion that the lock is already
|
||
held by the current thread, or annotating the method as requiring the
|
||
lock (MOZ_REQUIRES(mutex)) in the method definition: ::
|
||
|
||
// Ensures mSize is initialized, if it can be.
|
||
// mLock must be held when this is called, and mInput must be non-null.
|
||
void EnsureSizeInitialized() MOZ_REQUIRES(mLock);
|
||
...
|
||
// This lock protects mSeekable, mInput, mSize, and mSizeInitialized.
|
||
Mutex mLock;
|
||
int64_t mSize MOZ_GUARDED_BY(mLock);
|
||
|
||
**Single Writer** is tricky for static analysis normally, since it
|
||
doesn’t know what thread an access will occur on. In general, you should
|
||
prefer using Always Lock in non-performance-sensitive code, especially
|
||
since these mutexes are almost always uncontended and therefore cheap to
|
||
lock.
|
||
|
||
To support this fairly common pattern in Mozilla code, we’ve added
|
||
MutexSingleWriter and MonitorSingleWriter subclasses. To use these, you
|
||
need to subclass SingleWriterLockOwner on one object (typically the
|
||
object containing the Mutex), implement ::OnWritingThread(), and pass
|
||
the object to the constructor for MutexSingleWriter. In code that
|
||
accesses the guarded value from the writing thread, you need to add
|
||
mMutex.AssertIsOnWritingThread(), which both does a debug-only runtime
|
||
assertion by calling OnWritingThread(), and also asserts to the static
|
||
analyzer that the lock is held (which it isn’t).
|
||
|
||
There is one case this causes problems with: when a method needs to
|
||
access the value (without the lock), and then decides to write to the
|
||
value from the same method, taking the lock. To the static analyzer,
|
||
this looks like a double-lock. Either you will need to add
|
||
MOZ_NO_THREAD_SAFETY_ANALYSIS to the method, move the write into another
|
||
method you call, or locally disable the warning with
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY and MOZ_POP_THREAD_SAFETY. We’re discussing with
|
||
the clang static analysis developers how to better handle this.
|
||
|
||
Note also that this provides no checking that the lock is taken to write
|
||
to the value: ::
|
||
|
||
MutexSingleWriter mMutex;
|
||
// mResource should only be modified on the main thread with the lock.
|
||
// So it can be read without lock on the main thread or on other threads
|
||
// with the lock.
|
||
RefPtr<ChannelMediaResource> mResource MOZ_GUARDED_BY(mMutex);
|
||
...
|
||
nsresult ChannelMediaResource::Listener::OnStartRequest(nsIRequest *aRequest) {
|
||
mMutex.AssertOnWritingThread();
|
||
|
||
// Read from the only writing thread; no lock needed
|
||
if (!mResource) {
|
||
return NS_OK;
|
||
}
|
||
return mResource->OnStartRequest(aRequest, mOffset);
|
||
}
|
||
|
||
If you need to assert you’re on the writing thread, then later take a
|
||
lock to modify a value, it will cause a warning: ”acquiring mutex
|
||
'mMutex' that is already held”. You can resolve this by turning off
|
||
thread-safety analysis for the lock: ::
|
||
|
||
mMutex.AssertOnWritingThread();
|
||
...
|
||
{
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY
|
||
MutexSingleWriterAutoLock lock(mMutex);
|
||
MOZ_POP_THREAD_SAFETY
|
||
|
||
**Out-of-band Invariants** is used in a number of places (and may be
|
||
combined with either of the above patterns). It's using other knowledge
|
||
about the execution pattern of the code to assert that it's safe to avoid
|
||
taking certain locks. A primary example is when a value can
|
||
only be accessed from a single thread for part of its lifetime (this can
|
||
also be referred to as "time-based locking").
|
||
|
||
Note that thread-safety analysis always ignores constructors and destructors
|
||
(which shouldn’t have races with other threads barring really odd usages).
|
||
Since only a single thread can access during those time periods, locking is
|
||
not required there. However, if a method is called from a constructor,
|
||
that method may generate warnings since the compiler doesn't know if it
|
||
might be called from elsewhere: ::
|
||
|
||
...
|
||
class nsFoo {
|
||
public:
|
||
nsFoo() {
|
||
mBar = true; // Ok since we're in the constructor, no warning
|
||
Init();
|
||
}
|
||
void Init() { // we're only called from the constructor
|
||
// This causes a thread-safety warning, since the compiler
|
||
// can't prove that Init() is only called from the constructor
|
||
mQuerty = true;
|
||
}
|
||
...
|
||
mMutex mMutex;
|
||
uint32_t mBar MOZ_GUARDED_BY(mMutex);
|
||
uint32_t mQuerty MOZ_GUARDED_BY(mMutex);
|
||
}
|
||
|
||
Another example might be a value that’s used from other threads, but only
|
||
if an observer has been installed. Thus code that always runs before the
|
||
observer is installed, or after it’s removed, does not need to lock.
|
||
|
||
These patterns are impossible to statically check in most cases. If all
|
||
the periods where it’s accessed from one thread only are on the same
|
||
thread, you could use the Single Writer pattern support to cover this
|
||
case. You would add AssertIsOnWritingThread() calls to methods that meet
|
||
the criteria that only a single thread can access the value (but only if
|
||
that holds). Unlike regular uses of SingleWriter, however, there’s no way
|
||
to check if you added such an assertion to code that runs on the “right”
|
||
thread, but during a period where another thread might modify it.
|
||
|
||
For this reason, we **strongly** suggest that you convert cases of
|
||
Out-of-band-invariants/Time-based-locking to Always Lock if you’re
|
||
refactoring the code or making major modifications. This is far less prone
|
||
to error, and also to future changes breaking the assumptions about other
|
||
threads accessing the value. In all but a few cases where code is on a very
|
||
‘hot’ path, this will have no impact on performance - taking an uncontended
|
||
lock is cheap.
|
||
|
||
To quiet warnings where these patterns are in use, you'll need to either
|
||
add locks (preferred), or suppress the warnings with MOZ_NO_THREAD_SAFETY_ANALYSIS or
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY/MOZ_POP_THREAD_SAFETY.
|
||
|
||
**This pattern especially needs good documentation in the code as to what
|
||
threads will access what members under what conditions!**::
|
||
|
||
// Can't be accessed by multiple threads yet
|
||
nsresult nsAsyncStreamCopier::InitInternal(nsIInputStream* source,
|
||
nsIOutputStream* sink,
|
||
nsIEventTarget* target,
|
||
uint32_t chunkSize,
|
||
bool closeSource,
|
||
bool closeSink)
|
||
MOZ_NO_THREAD_SAFETY_ANALYSIS {
|
||
|
||
and::
|
||
|
||
// We can't be accessed by another thread because this hasn't been
|
||
// added to the public list yet
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY
|
||
mRestrictedPortList.AppendElement(gBadPortList[i]);
|
||
MOZ_POP_THREAD_SAFETY
|
||
|
||
and::
|
||
|
||
// This is called on entries in another entry's mCallback array, under the lock
|
||
// of that other entry. No other threads can access this entry at this time.
|
||
bool CacheEntry::Callback::DeferDoom(bool* aDoom) const {
|
||
|
||
**Known limitations**
|
||
---------------------
|
||
|
||
**Static analysis can’t handle all reasonable patterns.** In particular,
|
||
per their documentation, it can’t handle conditional locks, like: ::
|
||
|
||
if (OnMainThread()) {
|
||
mMutex.Lock();
|
||
}
|
||
|
||
You should resolve this either via MOZ_NO_THREAD_SAFETY_ANALYSIS on the
|
||
method, or MOZ_PUSH_IGNORE_THREAD_SAFETY/MOZ_POP_THREAD_SAFETY.
|
||
|
||
**Sometimes the analyzer can’t figure out that two objects are both the
|
||
same Mutex**, and it will warn you. You may be able to resolve this by
|
||
making sure you’re using the same pattern to access the mutex: ::
|
||
|
||
mChan->mMonitor->AssertCurrentThreadOwns();
|
||
...
|
||
{
|
||
- MonitorAutoUnlock guard(*monitor);
|
||
+ MonitorAutoUnlock guard(*(mChan->mMonitor.get())); // avoids mutex warning
|
||
ok = node->SendUserMessage(port, std::move(aMessage));
|
||
}
|
||
|
||
**Maybe<MutexAutoLock>** doesn’t tell the static analyzer when the mutex
|
||
is owned or freed; follow locking via the MayBe<> by
|
||
**mutex->AssertCurrentThreadOwns();** (and ditto for Monitors): ::
|
||
|
||
Maybe<MonitorAutoLock> lock(std::in_place, *mMonitor);
|
||
mMonitor->AssertCurrentThreadOwns(); // for threadsafety analysis
|
||
|
||
If you reset() the Maybe<>, you may need to surround it with
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY and MOZ_POP_THREAD_SAFETY macros: ::
|
||
|
||
MOZ_PUSH_IGNORE_THREAD_SAFETY
|
||
aLock.reset();
|
||
MOZ_POP_THREAD_SAFETY
|
||
|
||
**Passing a protected value by-reference** sometimes will confuse the
|
||
analyzer. Use MOZ_PUSH_IGNORE_THREAD_SAFETY and MOZ_POP_THREAD_SAFETY macros to
|
||
resolve this.
|
||
|
||
**Classes which need thread-safety annotations**
|
||
------------------------------------------------
|
||
|
||
- Mutex
|
||
|
||
- StaticMutex
|
||
|
||
- RecursiveMutex
|
||
|
||
- BaseProfilerMutex
|
||
|
||
- Monitor
|
||
|
||
- StaticMonitor
|
||
|
||
- ReentrantMonitor
|
||
|
||
- RWLock
|
||
|
||
- Anything that hides an internal Mutex/etc and presents a Mutex-like
|
||
API (::Lock(), etc).
|
||
|
||
**Additional Notes**
|
||
--------------------
|
||
|
||
Some code passes **Proof-of-Lock** AutoLock parameters, as a poor form of
|
||
static analysis. While it’s hard to make mistakes if you pass an AutoLock
|
||
reference, it is possible to pass a lock to the wrong Mutex/Monitor.
|
||
|
||
Proof-of-lock is basically redundant to MOZ_REQUIRES() and obsolete, and
|
||
depends on the optimizer to remove it, and per above it can be misused,
|
||
with effort. With MOZ_REQUIRES(), any proof-of-lock parameters can be removed,
|
||
though you don't have to do so immediately.
|
||
|
||
In any method taking an aProofOfLock parameter, add a MOZ_REQUIRES(mutex) to
|
||
the definition (and optionally remove the proof-of-lock), or add a
|
||
mMutex.AssertCurrentThreadOwns() to the method: ::
|
||
|
||
nsresult DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
|
||
- nsIEventTarget* aSyncLoopTarget,
|
||
- const MutexAutoLock& aProofOfLock);
|
||
+ nsIEventTarget* aSyncLoopTarget) MOZ_REQUIRES(mMutex);
|
||
|
||
or (if for some reason it's hard to specify the mutex in the header)::
|
||
|
||
nsresult DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
|
||
- nsIEventTarget* aSyncLoopTarget,
|
||
- const MutexAutoLock& aProofOfLock);
|
||
+ nsIEventTarget* aSyncLoopTarget) {
|
||
+ mMutex.AssertCurrentThreadOwns();
|
||
|
||
In addition to MOZ_GUARDED_BY() there’s also MOZ_PT_GUARDED_BY(), which says
|
||
that the pointer isn’t guarded, but the data pointed to by the pointer
|
||
is.
|
||
|
||
Classes that expose a Mutex-like interface can be annotated like Mutex;
|
||
see some of the examples in the tree that use MOZ_CAPABILITY and
|
||
MOZ_ACQUIRE()/MOZ_RELEASE().
|
||
|
||
Shared locks are supported, though we don’t use them much. See RWLock.
|