From 86601b48c55593fd5e9b7717602b9ed48b96f3a5 Mon Sep 17 00:00:00 2001 From: Doug Thayer Date: Thu, 12 Dec 2019 00:56:53 +0000 Subject: [PATCH] Bug 1602646 - Remove vestigial references to cooperative scheduling r=froydnj GetCurrentPhysicalThread and GetCurrentVirtualThread are, in practice, identical, as the TLS override that GetCurrentVirtualThread depends on is never actually set. This simply removes that and renames some things/ deletes some comments. Rebased across https://hg.mozilla.org/mozilla-central/rev/3f0b4e206853 by Karl Tomlinson . Differential Revision: https://phabricator.services.mozilla.com/D41247 --HG-- extra : moz-landing-system : lando --- dom/canvas/ImageBitmap.cpp | 4 +-- dom/indexedDB/ActorsParent.cpp | 4 +-- dom/media/MediaManager.cpp | 2 +- ipc/glue/MessageChannel.cpp | 4 +-- ipc/glue/MessageChannel.h | 4 +-- tools/fuzzing/ipc/ProtocolFuzzer.cpp | 2 +- xpcom/base/nsISupportsImpl.cpp | 4 +-- xpcom/threads/TaskQueue.cpp | 2 +- xpcom/threads/TaskQueue.h | 4 +-- xpcom/threads/ThreadEventTarget.cpp | 10 +++---- xpcom/threads/nsIEventTarget.idl | 10 +++---- xpcom/threads/nsThread.cpp | 9 ++---- xpcom/threads/nsThread.h | 1 - xpcom/threads/nsThreadManager.cpp | 34 --------------------- xpcom/threads/nsThreadUtils.cpp | 4 +-- xpcom/threads/nsThreadUtils.h | 44 ---------------------------- 16 files changed, 28 insertions(+), 114 deletions(-) diff --git a/dom/canvas/ImageBitmap.cpp b/dom/canvas/ImageBitmap.cpp index 30bc958b590f..f19ad91cb786 100644 --- a/dom/canvas/ImageBitmap.cpp +++ b/dom/canvas/ImageBitmap.cpp @@ -1083,11 +1083,11 @@ class CreateImageBitmapFromBlob final : public CancelableRunnable, mCropRect(aCropRect), mOriginalCropRect(aCropRect), mMainThreadEventTarget(aMainThreadEventTarget), - mThread(GetCurrentVirtualThread()) {} + mThread(PR_GetCurrentThread()) {} virtual ~CreateImageBitmapFromBlob() {} - bool IsCurrentThread() const { return mThread == GetCurrentVirtualThread(); } + bool IsCurrentThread() const { return mThread == PR_GetCurrentThread(); } // Called on the owning thread. nsresult StartMimeTypeAndDecodeAndCropBlob(); diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 93d17122dc9d..7a8c6c1ad0fa 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -5146,7 +5146,7 @@ struct ConnectionPool::DatabaseInfo final { void AssertIsOnConnectionThread() const { MOZ_ASSERT(mDEBUGConnectionThread); - MOZ_ASSERT(GetCurrentPhysicalThread() == mDEBUGConnectionThread); + MOZ_ASSERT(PR_GetCurrentThread() == mDEBUGConnectionThread); } uint64_t TotalTransactionCount() const { @@ -11208,7 +11208,7 @@ nsresult ConnectionPool::GetOrCreateConnection( NS_ConvertUTF16toUTF8(aDatabase->FilePath()).get())); #ifdef DEBUG - dbInfo->mDEBUGConnectionThread = GetCurrentPhysicalThread(); + dbInfo->mDEBUGConnectionThread = PR_GetCurrentThread(); #endif } diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index 8531bc001d23..b4321b50d08b 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -4032,7 +4032,7 @@ void SourceListener::Activate(RefPtr aAudioDevice, MOZ_ASSERT(!mStopped, "Cannot activate stopped source listener"); MOZ_ASSERT(!Activated(), "Already activated"); - mMainThreadCheck = GetCurrentVirtualThread(); + mMainThreadCheck = PR_GetCurrentThread(); if (aAudioDevice) { bool offWhileDisabled = aAudioDevice->GetMediaSource() == MediaSourceEnum::Microphone && diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index f00e56cb8035..9593eb67080b 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -834,7 +834,7 @@ bool MessageChannel::Open(Transport* aTransport, MessageLoop* aIOLoop, mMonitor = new RefCountedMonitor(); mWorkerLoop = MessageLoop::current(); - mWorkerThread = GetCurrentVirtualThread(); + mWorkerThread = PR_GetCurrentThread(); mWorkerLoop->AddDestructionObserver(this); mListener->OnIPCChannelOpened(); @@ -915,7 +915,7 @@ void MessageChannel::OnOpenAsSlave(MessageChannel* aTargetChan, Side aSide) { void MessageChannel::CommonThreadOpenInit(MessageChannel* aTargetChan, Side aSide) { mWorkerLoop = MessageLoop::current(); - mWorkerThread = GetCurrentVirtualThread(); + mWorkerThread = PR_GetCurrentThread(); mWorkerLoop->AddDestructionObserver(this); mListener->OnIPCChannelOpened(); diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index 1de810ded1d3..ae5a45786798 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -551,7 +551,7 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver { // Can be run on either thread void AssertWorkerThread() const { MOZ_ASSERT(mWorkerThread, "Channel hasn't been opened yet"); - MOZ_RELEASE_ASSERT(mWorkerThread == GetCurrentVirtualThread(), + MOZ_RELEASE_ASSERT(mWorkerThread == PR_GetCurrentThread(), "not on worker thread!"); } @@ -569,7 +569,7 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver { // If we aren't a same-thread channel, our "link" thread is _not_ our // worker thread! MOZ_ASSERT(mWorkerThread, "Channel hasn't been opened yet"); - MOZ_RELEASE_ASSERT(mWorkerThread != GetCurrentVirtualThread(), + MOZ_RELEASE_ASSERT(mWorkerThread != PR_GetCurrentThread(), "on worker thread but should not be!"); } diff --git a/tools/fuzzing/ipc/ProtocolFuzzer.cpp b/tools/fuzzing/ipc/ProtocolFuzzer.cpp index 2fc5cc1274c2..f73ef2e6f4ca 100644 --- a/tools/fuzzing/ipc/ProtocolFuzzer.cpp +++ b/tools/fuzzing/ipc/ProtocolFuzzer.cpp @@ -24,7 +24,7 @@ mozilla::dom::ContentParent* ProtocolFuzzerHelper::CreateContentParent( mozilla::dom::ContentParent* aOpener, const nsAString& aRemoteType) { auto* cp = new mozilla::dom::ContentParent(aOpener, aRemoteType); // TODO: this duplicates MessageChannel::Open - cp->GetIPCChannel()->mWorkerThread = GetCurrentVirtualThread(); + cp->GetIPCChannel()->mWorkerThread = PR_GetCurrentThread(); cp->GetIPCChannel()->mMonitor = new RefCountedMonitor(); return cp; } diff --git a/xpcom/base/nsISupportsImpl.cpp b/xpcom/base/nsISupportsImpl.cpp index 156b65dec41b..e676b6176967 100644 --- a/xpcom/base/nsISupportsImpl.cpp +++ b/xpcom/base/nsISupportsImpl.cpp @@ -32,7 +32,7 @@ nsresult NS_FASTCALL NS_TableDrivenQI(void* aThis, REFNSIID aIID, } #ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED -nsAutoOwningThread::nsAutoOwningThread() : mThread(GetCurrentVirtualThread()) {} +nsAutoOwningThread::nsAutoOwningThread() : mThread(PR_GetCurrentThread()) {} void nsAutoOwningThread::AssertCurrentThreadOwnsMe(const char* msg) const { if (MOZ_UNLIKELY(!IsCurrentThread())) { @@ -42,6 +42,6 @@ void nsAutoOwningThread::AssertCurrentThreadOwnsMe(const char* msg) const { } bool nsAutoOwningThread::IsCurrentThread() const { - return mThread == GetCurrentVirtualThread(); + return mThread == PR_GetCurrentThread(); } #endif diff --git a/xpcom/threads/TaskQueue.cpp b/xpcom/threads/TaskQueue.cpp index fb317d89dd13..faeb48825723 100644 --- a/xpcom/threads/TaskQueue.cpp +++ b/xpcom/threads/TaskQueue.cpp @@ -172,7 +172,7 @@ bool TaskQueue::IsEmpty() { } bool TaskQueue::IsCurrentThreadIn() const { - bool in = mRunningThread == GetCurrentPhysicalThread(); + bool in = mRunningThread == PR_GetCurrentThread(); return in; } diff --git a/xpcom/threads/TaskQueue.h b/xpcom/threads/TaskQueue.h index 0b26ceff16bd..050ae585969c 100644 --- a/xpcom/threads/TaskQueue.h +++ b/xpcom/threads/TaskQueue.h @@ -162,13 +162,13 @@ class TaskQueue : public AbstractThread { sCurrentThreadTLS.set(aQueue); MOZ_ASSERT(mQueue->mRunningThread == nullptr); - mQueue->mRunningThread = GetCurrentPhysicalThread(); + mQueue->mRunningThread = PR_GetCurrentThread(); } ~AutoTaskGuard() { DrainDirectTasks(); - MOZ_ASSERT(mQueue->mRunningThread == GetCurrentPhysicalThread()); + MOZ_ASSERT(mQueue->mRunningThread == PR_GetCurrentThread()); mQueue->mRunningThread = nullptr; sCurrentThreadTLS.set(mLastCurrentThread); diff --git a/xpcom/threads/ThreadEventTarget.cpp b/xpcom/threads/ThreadEventTarget.cpp index 8558f61d8a67..668e6f7d1d44 100644 --- a/xpcom/threads/ThreadEventTarget.cpp +++ b/xpcom/threads/ThreadEventTarget.cpp @@ -89,14 +89,12 @@ NS_IMPL_ISUPPORTS_INHERITED(DelayedRunnable, Runnable, nsITimerCallback) ThreadEventTarget::ThreadEventTarget(ThreadTargetSink* aSink, bool aIsMainThread) : mSink(aSink), mIsMainThread(aIsMainThread) { - mVirtualThread = GetCurrentVirtualThread(); + mThread = PR_GetCurrentThread(); } -void ThreadEventTarget::SetCurrentThread() { - mVirtualThread = GetCurrentVirtualThread(); -} +void ThreadEventTarget::SetCurrentThread() { mThread = PR_GetCurrentThread(); } -void ThreadEventTarget::ClearCurrentThread() { mVirtualThread = nullptr; } +void ThreadEventTarget::ClearCurrentThread() { mThread = nullptr; } NS_IMPL_ISUPPORTS(ThreadEventTarget, nsIEventTarget, nsISerialEventTarget) @@ -187,6 +185,6 @@ ThreadEventTarget::IsOnCurrentThread(bool* aIsOnCurrentThread) { NS_IMETHODIMP_(bool) ThreadEventTarget::IsOnCurrentThreadInfallible() { - // Rely on mVirtualThread being correct. + // Rely on mThread being correct. MOZ_CRASH("IsOnCurrentThreadInfallible should never be called on nsIThread"); } diff --git a/xpcom/threads/nsIEventTarget.idl b/xpcom/threads/nsIEventTarget.idl index f709c5f09b81..c0d7bc22726a 100644 --- a/xpcom/threads/nsIEventTarget.idl +++ b/xpcom/threads/nsIEventTarget.idl @@ -79,8 +79,8 @@ interface nsIEventTarget : nsISupports * * The infallible version of IsOnCurrentThread() is optimized to avoid a * virtual call for non-thread event targets. Thread targets should set - * mVirtualThread to their virtual PRThread. Non-thread targets should leave - * mVirtualThread null and implement IsOnCurrentThreadInfallible() to + * mThread to their virtual PRThread. Non-thread targets should leave + * mThread null and implement IsOnCurrentThreadInfallible() to * return the correct answer. * * The fallible version of IsOnCurrentThread may return errors, such as during @@ -91,13 +91,13 @@ interface nsIEventTarget : nsISupports %{C++ public: // Infallible. Defined in nsThreadUtils.cpp. Delegates to - // IsOnCurrentThreadInfallible when mVirtualThread is null. + // IsOnCurrentThreadInfallible when mThread is null. bool IsOnCurrentThread(); protected: - mozilla::Atomic mVirtualThread; + mozilla::Atomic mThread; - nsIEventTarget() : mVirtualThread(nullptr) {} + nsIEventTarget() : mThread(nullptr) {} %} // Note that this method is protected. We define it through IDL, rather than // in a %{C++ block, to ensure that the correct method indices are recorded diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index d893f4a7bb8a..c231c8f33970 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -420,7 +420,6 @@ void nsThread::ThreadFunc(void* aArg) { MOZ_ASSERT(self->mEvents); self->mThread = PR_GetCurrentThread(); - self->mVirtualThread = GetCurrentVirtualThread(); self->mEventTarget->SetCurrentThread(); SetupCurrentThreadForChaosMode(); @@ -508,7 +507,6 @@ void nsThread::ThreadFunc(void* aArg) { // The PRThread will be deleted in PR_JoinThread(), so clear references. self->mThread = nullptr; - self->mVirtualThread = nullptr; self->mEventTarget->ClearCurrentThread(); NS_RELEASE(self); } @@ -600,7 +598,6 @@ nsThread::nsThread(NotNull aQueue, new ThreadEventTarget(mEvents.get(), aMainThread == MAIN_THREAD)), mShutdownContext(nullptr), mScriptObserver(nullptr), - mThread(nullptr), mStackSize(aStackSize), mNestedEventLoopDepth(0), mCurrentEventLoopDepth(MaxValue::value), @@ -624,7 +621,6 @@ nsThread::nsThread() mEventTarget(nullptr), mShutdownContext(nullptr), mScriptObserver(nullptr), - mThread(nullptr), mStackSize(0), mNestedEventLoopDepth(0), mCurrentEventLoopDepth(MaxValue::value), @@ -699,7 +695,6 @@ nsresult nsThread::Init(const nsACString& aName) { nsresult nsThread::InitCurrentThread() { mThread = PR_GetCurrentThread(); - mVirtualThread = GetCurrentVirtualThread(); SetupCurrentThreadForChaosMode(); InitCommon(); @@ -764,13 +759,13 @@ nsThread::IsOnCurrentThread(bool* aResult) { if (mEventTarget) { return mEventTarget->IsOnCurrentThread(aResult); } - *aResult = GetCurrentVirtualThread() == mVirtualThread; + *aResult = PR_GetCurrentThread() == mThread; return NS_OK; } NS_IMETHODIMP_(bool) nsThread::IsOnCurrentThreadInfallible() { - // This method is only going to be called if `mVirtualThread` is null, which + // This method is only going to be called if `mThread` is null, which // only happens when the thread has exited the event loop. Therefore, when // we are called, we can never be on this thread. return false; diff --git a/xpcom/threads/nsThread.h b/xpcom/threads/nsThread.h index 76e96e3692aa..7a6c308c142b 100644 --- a/xpcom/threads/nsThread.h +++ b/xpcom/threads/nsThread.h @@ -214,7 +214,6 @@ class nsThread : public nsIThreadInternal, mozilla::CycleCollectedJSContext* mScriptObserver; - PRThread* mThread; void* mStackBase = nullptr; uint32_t mStackSize; uint32_t mThreadId; diff --git a/xpcom/threads/nsThreadManager.cpp b/xpcom/threads/nsThreadManager.cpp index 9c1090234ee2..ea69b1ce8a19 100644 --- a/xpcom/threads/nsThreadManager.cpp +++ b/xpcom/threads/nsThreadManager.cpp @@ -33,7 +33,6 @@ using namespace mozilla; static MOZ_THREAD_LOCAL(bool) sTLSIsMainThread; -static MOZ_THREAD_LOCAL(PRThread*) gTlsCurrentVirtualThread; bool NS_IsMainThreadTLSInitialized() { return sTLSIsMainThread.initialized(); } @@ -160,18 +159,6 @@ void NS_SetMainThread() { MOZ_ASSERT(NS_IsMainThread()); } -void NS_SetMainThread(PRThread* aVirtualThread) { - MOZ_ASSERT(!gTlsCurrentVirtualThread.get()); - gTlsCurrentVirtualThread.set(aVirtualThread); - NS_SetMainThread(); -} - -void NS_UnsetMainThread() { - sTLSIsMainThread.set(false); - MOZ_ASSERT(!NS_IsMainThread()); - gTlsCurrentVirtualThread.set(nullptr); -} - #ifdef DEBUG namespace mozilla { @@ -315,10 +302,6 @@ nsresult nsThreadManager::Init() { return NS_OK; } - if (!gTlsCurrentVirtualThread.init()) { - return NS_ERROR_UNEXPECTED; - } - if (PR_NewThreadPrivateIndex(&mCurThreadIndex, ReleaseThread) == PR_FAILURE) { return NS_ERROR_FAILURE; } @@ -733,20 +716,3 @@ nsThreadManager::IdleDispatchToMainThread(nsIRunnable* aEvent, return NS_DispatchToThreadQueue(event.forget(), mMainThread, EventQueuePriority::Idle); } - -namespace mozilla { - -PRThread* GetCurrentVirtualThread() { - // We call GetCurrentVirtualThread very early in startup, before the TLS is - // initialized. Make sure we don't assert in that case. - if (gTlsCurrentVirtualThread.initialized()) { - if (gTlsCurrentVirtualThread.get()) { - return gTlsCurrentVirtualThread.get(); - } - } - return PR_GetCurrentThread(); -} - -PRThread* GetCurrentPhysicalThread() { return PR_GetCurrentThread(); } - -} // namespace mozilla diff --git a/xpcom/threads/nsThreadUtils.cpp b/xpcom/threads/nsThreadUtils.cpp index e466b1fee34a..bda437add736 100644 --- a/xpcom/threads/nsThreadUtils.cpp +++ b/xpcom/threads/nsThreadUtils.cpp @@ -614,8 +614,8 @@ size_t GetNumberOfProcessors() { } // namespace mozilla bool nsIEventTarget::IsOnCurrentThread() { - if (mVirtualThread) { - return mVirtualThread == GetCurrentVirtualThread(); + if (mThread) { + return mThread == PR_GetCurrentThread(); } return IsOnCurrentThreadInfallible(); } diff --git a/xpcom/threads/nsThreadUtils.h b/xpcom/threads/nsThreadUtils.h index dc068e6d15ae..b90705d1772f 100644 --- a/xpcom/threads/nsThreadUtils.h +++ b/xpcom/threads/nsThreadUtils.h @@ -1709,50 +1709,6 @@ extern nsresult NS_DispatchBackgroundTask( namespace mozilla { -/** - * Cooperative thread scheduling is governed by two rules: - * - Only one thread in the pool of cooperatively scheduled threads runs at a - * time. - * - Thread switching happens at well-understood safe points. - * - * In some cases we may want to treat all the threads in a cooperative pool as a - * single thread, while other parts of the code may want to view them as - * separate threads. GetCurrentVirtualThread() will return the same value for - * all threads in a cooperative thread pool. GetCurrentPhysicalThread will - * return a different value for each thread in the pool. - * - * Thread safety assertions are a concrete example where GetCurrentVirtualThread - * should be used. An object may want to assert that it only can be used on the - * thread that created it. Such assertions would normally prevent the object - * from being used on different cooperative threads. However, the object might - * really only care that it's used atomically. Cooperative scheduling guarantees - * that it will be (assuming we don't yield in the middle of modifying the - * object). So we can weaken the assertion to compare the virtual thread the - * object was created on to the virtual thread on which it's being used. This - * assertion allows the object to be used across threads in a cooperative thread - * pool while preventing accesses across preemptively scheduled threads (which - * would be unsafe). - */ - -// Returns the PRThread on which this code is running. -PRThread* GetCurrentPhysicalThread(); - -// Returns a "virtual" PRThread that should only be used for comparison with -// other calls to GetCurrentVirtualThread. Two threads in the same cooperative -// thread pool will return the same virtual thread. Threads that are not -// cooperatively scheduled will have their own unique virtual PRThread (which -// will be equal to their physical PRThread). -// -// The return value of GetCurrentVirtualThread() is guaranteed not to change -// throughout the lifetime of a thread. -// -// Note that the original main thread (the first one created in the process) is -// considered as part of the pool of cooperative threads, so the return value of -// GetCurrentVirtualThread() for this thread (throughout its lifetime, even -// during shutdown) is the same as the return value from any other thread in the -// cooperative pool. -PRThread* GetCurrentVirtualThread(); - // These functions return event targets that can be used to dispatch to the // current or main thread. They can also be used to test if you're on those // threads (via IsOnCurrentThread). These functions should be used in preference