From 5ddda3bb770f8b698b5490bfcb5a768c0c27ed73 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Mon, 22 May 2017 11:26:39 -0700 Subject: [PATCH] Bug 1361164 - Add infallible IsOnCurrentThread to nsIEventTarget (r=froydnj) MozReview-Commit-ID: 12bk9hQ7Wnv --- dom/base/EventSource.cpp | 6 +++ dom/base/WebSocket.cpp | 6 +++ dom/workers/WorkerPrivate.cpp | 51 +++++++++++++------- dom/workers/WorkerPrivate.h | 4 +- netwerk/base/nsSocketTransportService2.cpp | 8 ++++ netwerk/base/nsStreamTransportService.cpp | 14 ++++++ xpcom/threads/LazyIdleThread.cpp | 10 ++++ xpcom/threads/SchedulerGroup.cpp | 6 +++ xpcom/threads/SharedThreadPool.h | 2 + xpcom/threads/TaskQueue.cpp | 6 +++ xpcom/threads/ThrottledEventQueue.cpp | 15 ++++-- xpcom/threads/nsIEventTarget.idl | 55 ++++++++++++++++++---- xpcom/threads/nsIThread.idl | 2 +- xpcom/threads/nsIThreadInternal.idl | 2 +- xpcom/threads/nsIThreadPool.idl | 2 +- xpcom/threads/nsThread.cpp | 15 ++++++ xpcom/threads/nsThreadPool.cpp | 14 ++++++ xpcom/threads/nsThreadUtils.cpp | 9 ++++ 18 files changed, 192 insertions(+), 35 deletions(-) diff --git a/dom/base/EventSource.cpp b/dom/base/EventSource.cpp index 497f9409771a..81a1b738cb42 100644 --- a/dom/base/EventSource.cpp +++ b/dom/base/EventSource.cpp @@ -963,6 +963,12 @@ EventSourceImpl::IsOnCurrentThread(bool* aResult) return NS_OK; } +NS_IMETHODIMP_(bool) +EventSourceImpl::IsOnCurrentThreadInfallible() +{ + return IsTargetThread(); +} + nsresult EventSourceImpl::GetBaseURI(nsIURI** aBaseURI) { diff --git a/dom/base/WebSocket.cpp b/dom/base/WebSocket.cpp index 3e821e320d1f..990807443a17 100644 --- a/dom/base/WebSocket.cpp +++ b/dom/base/WebSocket.cpp @@ -2880,6 +2880,12 @@ WebSocketImpl::IsOnCurrentThread(bool* aResult) return NS_OK; } +NS_IMETHODIMP_(bool) +WebSocketImpl::IsOnCurrentThreadInfallible() +{ + return IsTargetThread(); +} + bool WebSocketImpl::IsTargetThread() const { diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 97ade4aa5253..3de636355fae 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1718,18 +1718,23 @@ public: return NS_ERROR_NOT_IMPLEMENTED; } + NS_IMETHOD_(bool) IsOnCurrentThreadInfallible() override + { + MutexAutoLock lock(mMutex); + + if (!mWorkerPrivate) { + return false; + } + + return mWorkerPrivate->IsOnCurrentThread(); + } + NS_IMETHOD IsOnCurrentThread(bool* aIsOnCurrentThread) override { MOZ_ASSERT(aIsOnCurrentThread); - MutexAutoLock lock(mMutex); - - if (!mWorkerPrivate) { - *aIsOnCurrentThread = false; - return NS_OK; - } - - return mWorkerPrivate->IsOnCurrentThread(aIsOnCurrentThread); + *aIsOnCurrentThread = IsOnCurrentThreadInfallible(); + return NS_OK; } NS_DECL_THREADSAFE_ISUPPORTS @@ -5360,16 +5365,13 @@ WorkerPrivate::InterruptCallback(JSContext* aCx) return true; } -nsresult -WorkerPrivate::IsOnCurrentThread(bool* aIsOnCurrentThread) +bool +WorkerPrivate::IsOnCurrentThread() { // May be called on any thread! - MOZ_ASSERT(aIsOnCurrentThread); MOZ_ASSERT(mPRThread); - - *aIsOnCurrentThread = PR_GetCurrentThread() == mPRThread; - return NS_OK; + return PR_GetCurrentThread() == mPRThread; } void @@ -7102,12 +7104,25 @@ EventTarget::IsOnCurrentThread(bool* aIsOnCurrentThread) return NS_ERROR_UNEXPECTED; } - nsresult rv = mWorkerPrivate->IsOnCurrentThread(aIsOnCurrentThread); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; + *aIsOnCurrentThread = mWorkerPrivate->IsOnCurrentThread(); + return NS_OK; +} + +template +NS_IMETHODIMP_(bool) +WorkerPrivateParent:: +EventTarget::IsOnCurrentThreadInfallible() +{ + // May be called on any thread! + + MutexAutoLock lock(mMutex); + + if (!mWorkerPrivate) { + NS_WARNING("A worker's event target was used after the worker has !"); + return false; } - return NS_OK; + return mWorkerPrivate->IsOnCurrentThread(); } BEGIN_WORKERS_NAMESPACE diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 373cffed8cd8..00b403397ed9 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -1158,8 +1158,8 @@ public: bool InterruptCallback(JSContext* aCx); - nsresult - IsOnCurrentThread(bool* aIsOnCurrentThread); + bool + IsOnCurrentThread(); bool CloseInternal(JSContext* aCx) diff --git a/netwerk/base/nsSocketTransportService2.cpp b/netwerk/base/nsSocketTransportService2.cpp index cb628479657e..e458262097b3 100644 --- a/netwerk/base/nsSocketTransportService2.cpp +++ b/netwerk/base/nsSocketTransportService2.cpp @@ -173,6 +173,14 @@ nsSocketTransportService::IsOnCurrentThread(bool *result) return thread->IsOnCurrentThread(result); } +NS_IMETHODIMP_(bool) +nsSocketTransportService::IsOnCurrentThreadInfallible() +{ + nsCOMPtr thread = GetThreadSafely(); + NS_ENSURE_TRUE(thread, false); + return thread->IsOnCurrentThread(); +} + //----------------------------------------------------------------------------- // socket api (socket thread only) diff --git a/netwerk/base/nsStreamTransportService.cpp b/netwerk/base/nsStreamTransportService.cpp index 9d74cdeff27f..cba12a0ad754 100644 --- a/netwerk/base/nsStreamTransportService.cpp +++ b/netwerk/base/nsStreamTransportService.cpp @@ -496,6 +496,20 @@ nsStreamTransportService::DelayedDispatch(already_AddRefed, uint32_ return NS_ERROR_NOT_IMPLEMENTED; } +NS_IMETHODIMP_(bool) +nsStreamTransportService::IsOnCurrentThreadInfallible() +{ + nsCOMPtr pool; + { + mozilla::MutexAutoLock lock(mShutdownLock); + pool = mPool; + } + if (!pool) { + return false; + } + return pool->IsOnCurrentThread(); +} + NS_IMETHODIMP nsStreamTransportService::IsOnCurrentThread(bool *result) { diff --git a/xpcom/threads/LazyIdleThread.cpp b/xpcom/threads/LazyIdleThread.cpp index 56264c46e14b..d4dd6aee435d 100644 --- a/xpcom/threads/LazyIdleThread.cpp +++ b/xpcom/threads/LazyIdleThread.cpp @@ -441,6 +441,16 @@ LazyIdleThread::IsOnCurrentThread(bool* aIsOnCurrentThread) return NS_OK; } +NS_IMETHODIMP_(bool) +LazyIdleThread::IsOnCurrentThreadInfallible() +{ + if (mThread) { + return mThread->IsOnCurrentThread(); + } + + return false; +} + NS_IMETHODIMP LazyIdleThread::GetPRThread(PRThread** aPRThread) { diff --git a/xpcom/threads/SchedulerGroup.cpp b/xpcom/threads/SchedulerGroup.cpp index 9a6125f64801..0ef744ebd52c 100644 --- a/xpcom/threads/SchedulerGroup.cpp +++ b/xpcom/threads/SchedulerGroup.cpp @@ -161,6 +161,12 @@ SchedulerEventTarget::IsOnCurrentThread(bool* aIsOnCurrentThread) return NS_OK; } +NS_IMETHODIMP_(bool) +SchedulerEventTarget::IsOnCurrentThreadInfallible() +{ + return NS_IsMainThread(); +} + /* static */ nsresult SchedulerGroup::UnlabeledDispatch(const char* aName, TaskCategory aCategory, diff --git a/xpcom/threads/SharedThreadPool.h b/xpcom/threads/SharedThreadPool.h index 1d4c3de05e33..1d698aaedc30 100644 --- a/xpcom/threads/SharedThreadPool.h +++ b/xpcom/threads/SharedThreadPool.h @@ -77,6 +77,8 @@ public: NS_IMETHOD IsOnCurrentThread(bool *_retval) override { return !mEventTarget ? NS_ERROR_NULL_POINTER : mEventTarget->IsOnCurrentThread(_retval); } + NS_IMETHOD_(bool) IsOnCurrentThreadInfallible() override { return mEventTarget && mEventTarget->IsOnCurrentThread(); } + // Creates necessary statics. Called once at startup. static void InitStatics(); diff --git a/xpcom/threads/TaskQueue.cpp b/xpcom/threads/TaskQueue.cpp index 7d5089ffd64a..6f215af35462 100644 --- a/xpcom/threads/TaskQueue.cpp +++ b/xpcom/threads/TaskQueue.cpp @@ -56,6 +56,12 @@ public: return NS_OK; } + NS_IMETHOD_(bool) + IsOnCurrentThreadInfallible() override + { + return mTaskQueue->mTarget->IsOnCurrentThread(); + } + NS_DECL_THREADSAFE_ISUPPORTS }; diff --git a/xpcom/threads/ThrottledEventQueue.cpp b/xpcom/threads/ThrottledEventQueue.cpp index db8925a84442..63d48be04ce6 100644 --- a/xpcom/threads/ThrottledEventQueue.cpp +++ b/xpcom/threads/ThrottledEventQueue.cpp @@ -363,10 +363,10 @@ public: return NS_ERROR_NOT_IMPLEMENTED; } - nsresult - IsOnCurrentThread(bool* aResult) + bool + IsOnCurrentThread() { - return mBaseTarget->IsOnCurrentThread(aResult); + return mBaseTarget->IsOnCurrentThread(); } NS_DECL_THREADSAFE_ISUPPORTS @@ -450,7 +450,14 @@ ThrottledEventQueue::DelayedDispatch(already_AddRefed aEvent, NS_IMETHODIMP ThrottledEventQueue::IsOnCurrentThread(bool* aResult) { - return mInner->IsOnCurrentThread(aResult); + *aResult = mInner->IsOnCurrentThread(); + return NS_OK; +} + +NS_IMETHODIMP_(bool) +ThrottledEventQueue::IsOnCurrentThreadInfallible() +{ + return mInner->IsOnCurrentThread(); } } // namespace mozilla diff --git a/xpcom/threads/nsIEventTarget.idl b/xpcom/threads/nsIEventTarget.idl index 132a24d3cf56..6e33caef0dac 100644 --- a/xpcom/threads/nsIEventTarget.idl +++ b/xpcom/threads/nsIEventTarget.idl @@ -13,7 +13,7 @@ native alreadyAddRefed_nsIRunnable(already_AddRefed); -[scriptable, uuid(a03b8b63-af8b-4164-b0e5-c41e8b2b7cfa)] +[builtinclass, scriptable, uuid(a03b8b63-af8b-4164-b0e5-c41e8b2b7cfa)] interface nsIEventTarget : nsISupports { /* until we can get rid of all uses, keep the non-alreadyAddRefed<> version */ @@ -53,13 +53,51 @@ interface nsIEventTarget : nsISupports const unsigned long DISPATCH_AT_END = 2; /** - * Check to see if this event target is associated with the current thread. + * IsOnCurrentThread() should return true if events dispatched to this target + * can possibly run on the current thread, and false otherwise. In the case + * of an nsIEventTarget for a thread pool, it should return true on all + * threads in the pool. In the case of a non-thread nsIEventTarget such as + * ThrottledEventQueue, it should return true on the thread where events are + * expected to be processed, even if no events from the queue are actually + * being processed right now. * - * @returns - * A boolean value that if "true" indicates that events dispatched to this - * event target will run on the current thread (i.e., the thread calling - * this method). + * When called on an nsISerialEventTarget, IsOnCurrentThread can be used to + * ensure that no other thread has "ownership" of the event target. As such, + * it's useful for asserting that an object is only used on a particular + * thread. IsOnCurrentThread can't guarantee that the current event has been + * dispatched through a particular event target. + * + * 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 + * return the correct answer. + * + * The fallible version of IsOnCurrentThread may return errors, such as during + * shutdown. If it does not return an error, it should return the same result + * as the infallible version. The infallible method should return the correct + * result regardless of whether the fallible method returns an error. */ + %{C++ +public: + // Infallible. Defined in nsThreadUtils.cpp. Delegates to + // IsOnCurrentThreadInfallible when mVirtualThread is null. + bool IsOnCurrentThread(); + +protected: + PRThread* mVirtualThread; + + nsIEventTarget() : mVirtualThread(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 + // for XPConnect purposes. + [noscript,notxpcom] boolean isOnCurrentThreadInfallible(); + %{C++ +public: + %} + + // Fallible version of IsOnCurrentThread. boolean isOnCurrentThread(); /** @@ -129,6 +167,7 @@ interface nsIEventTarget : nsISupports // Convenient NS_DECL variant that includes some C++-only methods. #define NS_DECL_NSIEVENTTARGET_FULL \ NS_DECL_NSIEVENTTARGET \ - /* Avoid hiding this method */ \ - using nsIEventTarget::Dispatch; + /* Avoid hiding these methods */ \ + using nsIEventTarget::Dispatch; \ + using nsIEventTarget::IsOnCurrentThread; %} diff --git a/xpcom/threads/nsIThread.idl b/xpcom/threads/nsIThread.idl index 100363599dc2..cd950c00f9ab 100644 --- a/xpcom/threads/nsIThread.idl +++ b/xpcom/threads/nsIThread.idl @@ -24,7 +24,7 @@ native alreadyAddRefed_nsIIdlePeriod(already_AddRefed); * * See nsIThreadManager for the API used to create and locate threads. */ -[scriptable, uuid(5801d193-29d1-4964-a6b7-70eb697ddf2b)] +[builtinclass, scriptable, uuid(5801d193-29d1-4964-a6b7-70eb697ddf2b)] interface nsIThread : nsIEventTarget { /** diff --git a/xpcom/threads/nsIThreadInternal.idl b/xpcom/threads/nsIThreadInternal.idl index e5d7cc83ff62..ff8fbe098e06 100644 --- a/xpcom/threads/nsIThreadInternal.idl +++ b/xpcom/threads/nsIThreadInternal.idl @@ -13,7 +13,7 @@ interface nsIThreadObserver; * The XPCOM thread object implements this interface, which allows a consumer * to observe dispatch activity on the thread. */ -[scriptable, uuid(a3a72e5f-71d9-4add-8f30-59a78fb6d5eb)] +[builtinclass, scriptable, uuid(a3a72e5f-71d9-4add-8f30-59a78fb6d5eb)] interface nsIThreadInternal : nsIThread { /** diff --git a/xpcom/threads/nsIThreadPool.idl b/xpcom/threads/nsIThreadPool.idl index d04e8504a6b4..61ce83523a04 100644 --- a/xpcom/threads/nsIThreadPool.idl +++ b/xpcom/threads/nsIThreadPool.idl @@ -27,7 +27,7 @@ interface nsIThreadPoolListener : nsISupports * anonymous (unnamed) worker threads. An event dispatched to the thread pool * will be run on the next available worker thread. */ -[scriptable, uuid(76ce99c9-8e43-489a-9789-f27cc4424965)] +[builtinclass, scriptable, uuid(76ce99c9-8e43-489a-9789-f27cc4424965)] interface nsIThreadPool : nsIEventTarget { /** diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index 9149224ae495..9b1f8e70f8ca 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -459,6 +459,7 @@ nsThread::ThreadFunc(void* aArg) nsThread* self = initData->thread; // strong reference self->mThread = PR_GetCurrentThread(); + self->mVirtualThread = GetCurrentVirtualThread(); SetupCurrentThreadForChaosMode(); if (!initData->name.IsEmpty()) { @@ -705,6 +706,7 @@ nsresult nsThread::InitCurrentThread() { mThread = PR_GetCurrentThread(); + mVirtualThread = GetCurrentVirtualThread(); SetupCurrentThreadForChaosMode(); mIdlePeriod = new IdlePeriod(); @@ -889,6 +891,13 @@ nsThread::IsOnCurrentThread(bool* aResult) return NS_OK; } +NS_IMETHODIMP_(bool) +nsThread::IsOnCurrentThreadInfallible() +{ + // Rely on mVirtualThread being correct. + MOZ_CRASH("IsOnCurrentThreadInfallible should never be called on nsIThread"); +} + //----------------------------------------------------------------------------- // nsIThread @@ -1685,3 +1694,9 @@ nsThread::nsNestedEventTarget::IsOnCurrentThread(bool* aResult) { return mThread->IsOnCurrentThread(aResult); } + +NS_IMETHODIMP_(bool) +nsThread::nsNestedEventTarget::IsOnCurrentThreadInfallible() +{ + return mThread->IsOnCurrentThread(); +} diff --git a/xpcom/threads/nsThreadPool.cpp b/xpcom/threads/nsThreadPool.cpp index 9d29cdca0136..d151967a47d4 100644 --- a/xpcom/threads/nsThreadPool.cpp +++ b/xpcom/threads/nsThreadPool.cpp @@ -282,6 +282,20 @@ nsThreadPool::DelayedDispatch(already_AddRefed, uint32_t) return NS_ERROR_NOT_IMPLEMENTED; } +NS_IMETHODIMP_(bool) +nsThreadPool::IsOnCurrentThreadInfallible() +{ + MutexAutoLock lock(mMutex); + + nsIThread* thread = NS_GetCurrentThread(); + for (uint32_t i = 0; i < static_cast(mThreads.Count()); ++i) { + if (mThreads[i] == thread) { + return true; + } + } + return false; +} + NS_IMETHODIMP nsThreadPool::IsOnCurrentThread(bool* aResult) { diff --git a/xpcom/threads/nsThreadUtils.cpp b/xpcom/threads/nsThreadUtils.cpp index afa29804c3d3..f4dc3cf65b6c 100644 --- a/xpcom/threads/nsThreadUtils.cpp +++ b/xpcom/threads/nsThreadUtils.cpp @@ -545,3 +545,12 @@ GetCurrentPhysicalThread() } } // namespace mozilla + +bool +nsIEventTarget::IsOnCurrentThread() +{ + if (mVirtualThread) { + return mVirtualThread == GetCurrentVirtualThread(); + } + return IsOnCurrentThreadInfallible(); +}