Bug 1207753: Basic thread-safety annotations to quiet errors until real annotations land r=nika

Differential Revision: https://phabricator.services.mozilla.com/D141062
This commit is contained in:
Randell Jesup 2022-03-17 18:39:15 +00:00
parent 9bef896e2f
commit c5917ed65a
33 changed files with 83 additions and 26 deletions

View File

@ -505,6 +505,7 @@ void BodyStream::ErrorPropagation(JSContext* aCx,
JS::HandleObject aStream, nsresult aError) {
#endif
AssertIsOnOwningThread();
mMutex.AssertCurrentThreadOwns();
// Nothing to do.
if (mState == eClosed) {
@ -752,6 +753,7 @@ void BodyStream::CloseAndReleaseObjects(JSContext* aCx,
const MutexAutoLock& aProofOfLock,
ReadableStream* aStream) {
AssertIsOnOwningThread();
mMutex.AssertCurrentThreadOwns();
MOZ_DIAGNOSTIC_ASSERT(mState != eClosed);
ReleaseObjects(aProofOfLock);
@ -769,6 +771,7 @@ void BodyStream::CloseAndReleaseObjects(JSContext* aCx,
const MutexAutoLock& aProofOfLock,
JS::HandleObject aStream) {
AssertIsOnOwningThread();
mMutex.AssertCurrentThreadOwns();
MOZ_DIAGNOSTIC_ASSERT(mState != eClosed);
ReleaseObjects(aProofOfLock);

View File

@ -231,6 +231,7 @@ struct ToCubebFormat<AUDIO_FORMAT_S16> {
template <typename Function, typename... Args>
int AudioStream::InvokeCubeb(Function aFunction, Args&&... aArgs) {
mMonitor.AssertCurrentThreadOwns();
MonitorAutoUnlock mon(mMonitor);
return aFunction(mCubebStream.get(), std::forward<Args>(aArgs)...);
}

View File

@ -24,8 +24,10 @@ class MultiWriterQueueReaderLocking_Mutex {
public:
MultiWriterQueueReaderLocking_Mutex()
: mMutex("MultiWriterQueueReaderLocking_Mutex") {}
PUSH_IGNORE_THREAD_SAFETY
void Lock() { mMutex.Lock(); };
void Unlock() { mMutex.Unlock(); };
POP_THREAD_SAFETY
private:
Mutex mMutex MOZ_UNANNOTATED;

View File

@ -257,9 +257,11 @@ GMPMutexImpl::~GMPMutexImpl() { MOZ_COUNT_DTOR(GMPMutexImpl); }
void GMPMutexImpl::Destroy() { delete this; }
PUSH_IGNORE_THREAD_SAFETY
void GMPMutexImpl::Acquire() { mMonitor.Enter(); }
void GMPMutexImpl::Release() { mMonitor.Exit(); }
POP_THREAD_SAFETY
GMPTask* NewGMPTask(std::function<void()>&& aFunction) {
class Task : public GMPTask {

View File

@ -46,7 +46,8 @@ class DeviceInfoAndroid : public DeviceInfoImpl {
char* productUniqueIdUTF8 = 0,
uint32_t productUniqueIdUTF8Length = 0,
pid_t* pid = 0);
virtual int32_t CreateCapabilityMap(const char* deviceUniqueIdUTF8);
virtual int32_t CreateCapabilityMap(const char* deviceUniqueIdUTF8)
RTC_EXCLUSIVE_LOCKS_REQUIRED(_apiLock);
virtual int32_t DisplayCaptureSettingsDialogBox(
const char* /*deviceUniqueIdUTF8*/, const char* /*dialogTitleUTF8*/,

View File

@ -44,7 +44,8 @@ class DeviceInfoIos : public DeviceInfoImpl {
int32_t GetOrientation(const char* deviceUniqueIdUTF8, VideoRotation& orientation) override;
int32_t CreateCapabilityMap(const char* device_unique_id_utf8) override;
int32_t CreateCapabilityMap(const char* device_unique_id_utf8) override
RTC_EXCLUSIVE_LOCKS_REQUIRED(_apiLock);
private:
std::map<std::string, VideoCaptureCapabilities> _capabilitiesMap;

View File

@ -59,6 +59,7 @@ class LockingWithMutex {
protected:
LockingWithMutex() : mMutex("LockingWithMutex::mMutex") {}
PUSH_IGNORE_THREAD_SAFETY
void Lock() { mMutex.Lock(); }
void Unlock() { mMutex.Unlock(); }
@ -71,6 +72,7 @@ class LockingWithMutex {
~AutoLock() { mHost.Unlock(); }
};
POP_THREAD_SAFETY
friend class AutoLock;
};

View File

@ -99,11 +99,13 @@ class SharedMutex {
SharedMutex(const SharedMutex& aOther) = default;
operator Mutex&() { return *mMutex; }
operator Mutex&() RETURN_CAPABILITY(*mMutex) { return *mMutex; }
operator const Mutex&() const { return *mMutex; }
operator const Mutex&() const RETURN_CAPABILITY(*mMutex) { return *mMutex; }
void AssertCurrentThreadOwns() const { mMutex->AssertCurrentThreadOwns(); }
void AssertCurrentThreadOwns() const ASSERT_CAPABILITY(*mMutex) {
mMutex->AssertCurrentThreadOwns();
}
};
nsString ComputeWorkerPrivateId();

View File

@ -980,11 +980,13 @@ class SharedFTFace : public external::AtomicRefCounted<SharedFTFace> {
* If no owner is given, then the user should avoid modifying any state on
* the face so as not to invalidate the prior owner's modification.
*/
PUSH_IGNORE_THREAD_SAFETY
bool Lock(void* aOwner = nullptr) {
mLock.Lock();
return !aOwner || mLastLockOwner.exchange(aOwner) == aOwner;
}
void Unlock() { mLock.Unlock(); }
POP_THREAD_SAFETY
/** Should be called when a lock owner is destroyed so that we don't have
* a dangling pointer to a destroyed owner.

View File

@ -630,9 +630,11 @@ void Factory::ReleaseFTLibrary(FT_Library aFTLibrary) {
FT_Done_FreeType(aFTLibrary);
}
PUSH_IGNORE_THREAD_SAFETY
void Factory::LockFTLibrary(FT_Library aFTLibrary) { mFTLock.Lock(); }
void Factory::UnlockFTLibrary(FT_Library aFTLibrary) { mFTLock.Unlock(); }
POP_THREAD_SAFETY
FT_Face Factory::NewFTFace(FT_Library aFTLibrary, const char* aFileName,
int aFaceIndex) {

View File

@ -159,7 +159,8 @@ uint8_t* SourceSurfaceSkia::GetData() {
return reinterpret_cast<uint8_t*>(pixmap.writable_addr());
}
bool SourceSurfaceSkia::Map(MapType, MappedSurface* aMappedSurface) {
bool SourceSurfaceSkia::Map(MapType, MappedSurface* aMappedSurface)
NO_THREAD_SAFETY_ANALYSIS {
mChangeMutex.Lock();
aMappedSurface->mData = GetData();
aMappedSurface->mStride = Stride();
@ -171,7 +172,7 @@ bool SourceSurfaceSkia::Map(MapType, MappedSurface* aMappedSurface) {
return isMapped;
}
void SourceSurfaceSkia::Unmap() {
void SourceSurfaceSkia::Unmap() NO_THREAD_SAFETY_ANALYSIS {
MOZ_ASSERT(mIsMapped);
mIsMapped = false;
mChangeMutex.Unlock();

View File

@ -3610,6 +3610,7 @@ void APZCTreeManager::AssertOnUpdaterThread() {
GetUpdater()->AssertOnUpdaterThread();
}
PUSH_IGNORE_THREAD_SAFETY
void APZCTreeManager::LockTree() {
AssertOnUpdaterThread();
mTreeLock.Lock();
@ -3619,6 +3620,7 @@ void APZCTreeManager::UnlockTree() {
AssertOnUpdaterThread();
mTreeLock.Unlock();
}
POP_THREAD_SAFETY
void APZCTreeManager::SetDPI(float aDpiValue) {
if (!APZThreadUtils::IsControllerThread()) {

View File

@ -290,6 +290,8 @@ nsresult DataPipeBase::ProcessSegmentsInternal(
while (*aProcessedCount < aCount) {
DataPipeAutoLock lock(*mMutex);
mMutex->AssertCurrentThreadOwns();
MOZ_LOG(gDataPipeLog, LogLevel::Verbose,
("ProcessSegments(%u of %u) %s", *aProcessedCount, aCount,
Describe(lock).get()));

View File

@ -117,6 +117,7 @@ void PortLink::SendMessage(UniquePtr<Message> aMessage) {
PortRef port = mPort;
bool ok = false;
monitor->AssertCurrentThreadOwns();
{
MonitorAutoUnlock guard(*monitor);
ok = node->SendUserMessage(port, std::move(aMessage));

View File

@ -661,6 +661,7 @@ void ScriptPreloader::PrepareCacheWrite() {
// an offset from the start of the block, as specified above.
Result<Ok, nsresult> ScriptPreloader::WriteCache() {
MOZ_ASSERT(!NS_IsMainThread());
mSaveMonitor.AssertCurrentThreadOwns();
if (!mDataPrepared && !mSaveComplete) {
MonitorAutoUnlock mau(mSaveMonitor);

View File

@ -1513,8 +1513,10 @@ class nsAsyncBridgeRequest final : public nsPACManCallback {
mCondVar.Notify();
}
PUSH_IGNORE_THREAD_SAFETY
void Lock() { mMutex.Lock(); }
void Unlock() { mMutex.Unlock(); }
POP_THREAD_SAFETY
void Wait() { mCondVar.Wait(TimeDuration::FromSeconds(3)); }
private:

View File

@ -140,6 +140,7 @@ class CacheFile final : public CacheFileChunkListener,
virtual ~CacheFile();
PUSH_IGNORE_THREAD_SAFETY
void Lock() { mLock->Lock().Lock(); }
void Unlock() {
// move the elements out of mObjsToRelease
@ -148,6 +149,7 @@ class CacheFile final : public CacheFileChunkListener,
mLock->Lock().Unlock();
}
POP_THREAD_SAFETY
void AssertOwnsLock() const { mLock->Lock().AssertCurrentThreadOwns(); }
void ReleaseOutsideLock(RefPtr<nsISupports> aObject);

View File

@ -493,6 +493,7 @@ void CacheIOThread::ThreadFunc() {
}
void CacheIOThread::LoopOneLevel(uint32_t aLevel) {
mMonitor.AssertCurrentThreadOwns();
EventQueue events = std::move(mEventQueue[aLevel]);
EventQueue::size_type length = events.Length();

View File

@ -2692,6 +2692,7 @@ void CacheIndex::BuildIndex(const StaticMutexAutoLock& aProofOfLock) {
if (!mDirEnumerator) {
{
// Do not do IO under the lock.
sLock.AssertCurrentThreadOwns();
StaticMutexAutoUnlock unlock(sLock);
rv = SetupDirectoryEnumerator();
}
@ -2719,6 +2720,7 @@ void CacheIndex::BuildIndex(const StaticMutexAutoLock& aProofOfLock) {
nsCOMPtr<nsIFile> file;
{
// Do not do IO under the lock.
sLock.AssertCurrentThreadOwns();
StaticMutexAutoUnlock unlock(sLock);
rv = mDirEnumerator->GetNextFile(getter_AddRefs(file));
@ -2926,6 +2928,7 @@ void CacheIndex::UpdateIndex(const StaticMutexAutoLock& aProofOfLock) {
LOG(("CacheIndex::UpdateIndex()"));
MOZ_ASSERT(mPendingUpdates.Count() == 0);
sLock.AssertCurrentThreadOwns();
nsresult rv;

View File

@ -399,7 +399,7 @@ class MOZ_STACK_CLASS MutexSettableAutoUnlock final {
public:
MutexSettableAutoUnlock() = default;
void Acquire(mozilla::Mutex& aMutex) {
void Acquire(mozilla::Mutex& aMutex) NO_THREAD_SAFETY_ANALYSIS {
MOZ_ASSERT(!mMutex);
mMutex = &aMutex;
mMutex->Lock();

View File

@ -40,8 +40,11 @@ class nsHttpRequestHead {
// copying headers. If you use it be careful to do it only under
// nsHttpRequestHead lock!!!
const nsHttpHeaderArray& Headers() const;
PUSH_IGNORE_THREAD_SAFETY
void Enter() const { mRecursiveMutex.Lock(); }
void Exit() const { mRecursiveMutex.Unlock(); }
POP_THREAD_SAFETY
void SetHeaders(const nsHttpHeaderArray& aHeaders);

View File

@ -43,8 +43,10 @@ class nsHttpResponseHead {
nsHttpResponseHead(const nsHttpResponseHead& aOther);
nsHttpResponseHead& operator=(const nsHttpResponseHead& aOther);
PUSH_IGNORE_THREAD_SAFETY
void Enter() const { mRecursiveMutex.Lock(); }
void Exit() const { mRecursiveMutex.Unlock(); }
POP_THREAD_SAFETY
HttpVersion Version();
uint16_t Status() const;

View File

@ -2365,9 +2365,9 @@ void DataChannelConnection::HandleNotification(
}
}
int DataChannelConnection::ReceiveCallback(struct socket* sock, void* data,
size_t datalen,
struct sctp_rcvinfo rcv, int flags) {
int DataChannelConnection::ReceiveCallback(
struct socket* sock, void* data, size_t datalen, struct sctp_rcvinfo rcv,
int flags) NO_THREAD_SAFETY_ANALYSIS {
ASSERT_WEBRTC(!NS_IsMainThread());
DC_DEBUG(("In ReceiveCallback"));

View File

@ -363,7 +363,7 @@ bool StartupCache::HasEntry(const char* id) {
}
nsresult StartupCache::GetBuffer(const char* id, const char** outbuf,
uint32_t* length) {
uint32_t* length) NO_THREAD_SAFETY_ANALYSIS {
AUTO_PROFILER_LABEL("StartupCache::GetBuffer", OTHER);
NS_ASSERTION(NS_IsMainThread(),
@ -450,7 +450,7 @@ nsresult StartupCache::GetBuffer(const char* id, const char** outbuf,
// Makes a copy of the buffer, client retains ownership of inbuf.
nsresult StartupCache::PutBuffer(const char* id, UniquePtr<char[]>&& inbuf,
uint32_t len) {
uint32_t len) NO_THREAD_SAFETY_ANALYSIS {
NS_ASSERTION(NS_IsMainThread(),
"Startup cache only available on main thread");
if (StartupCache::gShutdownInitiated) {
@ -469,7 +469,10 @@ nsresult StartupCache::PutBuffer(const char* id, UniquePtr<char[]>&& inbuf,
if (!mTableLock.TryLock()) {
return NS_ERROR_NOT_AVAILABLE;
}
auto lockGuard = MakeScopeExit([&] { mTableLock.Unlock(); });
auto lockGuard = MakeScopeExit([&] {
mTableLock.AssertCurrentThreadOwns();
mTableLock.Unlock();
});
// putNew returns false on alloc failure - in the very unlikely event we hit
// that and aren't going to crash elsewhere, there's no reason we need to

View File

@ -3256,9 +3256,9 @@ static void MaybeAnnotateDumperError(const ClientInfo& aClientInfo,
#endif
}
static void OnChildProcessDumpRequested(void* aContext,
const ClientInfo& aClientInfo,
const xpstring& aFilePath) {
static void OnChildProcessDumpRequested(
void* aContext, const ClientInfo& aClientInfo,
const xpstring& aFilePath) NO_THREAD_SAFETY_ANALYSIS {
nsCOMPtr<nsIFile> minidump;
// Hold the mutex until the current dump request is complete, to
@ -3303,8 +3303,8 @@ static void OnChildProcessDumpRequested(void* aContext,
}
}
static void OnChildProcessDumpWritten(void* aContext,
const ClientInfo& aClientInfo) {
static void OnChildProcessDumpWritten(
void* aContext, const ClientInfo& aClientInfo) NO_THREAD_SAFETY_ANALYSIS {
ProcessId pid = aClientInfo.pid();
ChildProcessData* pd = pidToMinidump->GetEntry(pid);
MOZ_ASSERT(pd);

View File

@ -599,9 +599,11 @@ class MOZ_HEAP_CLASS NativeWeakPtrControlBlock final {
return nativeImpl;
}
PUSH_IGNORE_THREAD_SAFETY
void Lock() const { mLock.ReadLock(); }
void Unlock() const { mLock.ReadUnlock(); }
POP_THREAD_SAFETY
#if defined(DEBUG)
// This is kind of expensive, so we only support it in debug builds.

View File

@ -1615,7 +1615,7 @@ void nsChildView::ConfigureAPZCTreeManager() { nsBaseWidget::ConfigureAPZCTreeMa
void nsChildView::ConfigureAPZControllerThread() { nsBaseWidget::ConfigureAPZControllerThread(); }
bool nsChildView::PreRender(WidgetRenderingContext* aContext) {
bool nsChildView::PreRender(WidgetRenderingContext* aContext) NO_THREAD_SAFETY_ANALYSIS {
// The lock makes sure that we don't attempt to tear down the view while
// compositing. That would make us unable to call postRender on it when the
// composition is done, thus keeping the GL context locked forever.
@ -1628,7 +1628,9 @@ bool nsChildView::PreRender(WidgetRenderingContext* aContext) {
return true;
}
void nsChildView::PostRender(WidgetRenderingContext* aContext) { mCompositingLock.Unlock(); }
void nsChildView::PostRender(WidgetRenderingContext* aContext) NO_THREAD_SAFETY_ANALYSIS {
mCompositingLock.Unlock();
}
RefPtr<layers::NativeLayerRoot> nsChildView::GetNativeLayerRoot() { return mNativeLayerRoot; }

View File

@ -606,7 +606,8 @@ static bool moz_container_wayland_surface_create_locked(
return true;
}
struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container) {
struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container)
NO_THREAD_SAFETY_ANALYSIS {
// LOGWAYLAND("%s [%p] surface %p ready_to_draw %d\n", __FUNCTION__,
// (void*)container, (void*)container->wl_container.surface,
// container->wl_container.ready_to_draw);
@ -619,7 +620,8 @@ struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container) {
}
void moz_container_wayland_surface_unlock(MozContainer* container,
struct wl_surface** surface) {
struct wl_surface** surface)
NO_THREAD_SAFETY_ANALYSIS {
// Temporarily disabled to avoid log noise
// LOGWAYLAND("%s [%p] surface %p\n", __FUNCTION__, (void*)container,
// (void*)container->wl_container.surface);
@ -643,11 +645,13 @@ struct wl_surface* moz_container_wayland_get_surface_locked(
return container->wl_container.surface;
}
void moz_container_wayland_lock(MozContainer* container) {
void moz_container_wayland_lock(MozContainer* container)
NO_THREAD_SAFETY_ANALYSIS {
container->wl_container.container_lock->Lock();
}
void moz_container_wayland_unlock(MozContainer* container) {
void moz_container_wayland_unlock(MozContainer* container)
NO_THREAD_SAFETY_ANALYSIS {
container->wl_container.container_lock->Unlock();
}

View File

@ -131,6 +131,7 @@ void WaylandVsyncSource::MaybeUpdateSource(
void WaylandVsyncSource::Refresh(const MutexAutoLock& aProofOfLock) {
LOG("WaylandVsyncSource::Refresh fps %f\n", GetFPS(mVsyncRate));
mMutex.AssertCurrentThreadOwns();
if (!(mContainer || mNativeLayerRoot) || !mMonitorEnabled || !mVsyncEnabled ||
mCallbackRequested) {

View File

@ -613,6 +613,7 @@ void nsComponentManagerImpl::RegisterCIDEntryLocked(
#endif
mFactories.WithEntryHandle(aEntry->cid, [&](auto&& entry) {
mLock.AssertCurrentThreadOwns();
if (entry) {
nsFactoryEntry* f = entry.Data();
NS_WARNING("Re-registering a CID?");
@ -1285,6 +1286,7 @@ nsresult nsComponentManagerImpl::GetServiceLocked(Maybe<MonitorAutoLock>& aLock,
}
});
nsresult rv;
mLock.AssertCurrentThreadOwns();
{
MonitorAutoUnlock unlock(mLock);
AUTO_PROFILER_MARKER_TEXT(

View File

@ -1390,6 +1390,8 @@ nsMultiplexInputStream::AsyncLengthWait(nsIInputStreamLengthCallback* aCallback,
void nsMultiplexInputStream::AsyncWaitCompleted(
int64_t aLength, const MutexAutoLock& aProofOfLock) {
mLock.AssertCurrentThreadOwns();
nsCOMPtr<nsIInputStreamLengthCallback> callback;
callback.swap(mAsyncWaitLengthCallback);

View File

@ -238,7 +238,7 @@ TEST(Synchronization, AutoLock)
// AutoTryLock tests
//
TEST(Synchronization, AutoTryLock)
{
NO_THREAD_SAFETY_ANALYSIS {
Mutex l1 MOZ_UNANNOTATED("autotrylock");
MutexAutoTryLock autol1(l1);

View File

@ -605,6 +605,7 @@ uint64_t TaskController::PendingMainthreadTaskCountIncludingSuspended() {
bool TaskController::ExecuteNextTaskOnlyMainThreadInternal(
const MutexAutoLock& aProofOfLock) {
mGraphMutex.AssertCurrentThreadOwns();
// Block to make it easier to jump to our cleanup.
bool taskRan = false;
do {
@ -674,6 +675,8 @@ bool TaskController::ExecuteNextTaskOnlyMainThreadInternal(
bool TaskController::DoExecuteNextTaskOnlyMainThreadInternal(
const MutexAutoLock& aProofOfLock) {
mGraphMutex.AssertCurrentThreadOwns();
nsCOMPtr<nsIThread> mainIThread;
NS_GetMainThread(getter_AddRefs(mainIThread));