Bug 404870. Don't allow nsThread::PutEvent to succeed when the thread is no longer able to process the event. This stops us leaking the event and anything hanging off it. r+sr=bsmedberg
--- a/xpcom/threads/nsIEventTarget.idl
+++ b/xpcom/threads/nsIEventTarget.idl
@@ -50,16 +50,19 @@ interface nsIEventTarget : nsISupports
* @param event
* The event to dispatch.
* @param flags
* The flags modifying event dispatch. The flags are described in detail
* below.
*
* @throws NS_ERROR_INVALID_ARG
* Indicates that event is null.
+ * @throws NS_ERROR_UNEXPECTED
+ * Indicates that the thread is shutting down and has finished processing
+ * events, so this event would never run and has not been dispatched.
*/
void dispatch(in nsIRunnable event, in unsigned long flags);
/**
* This flag specifies the default mode of event dispatch, whereby the event
* is simply queued for later processing. When this flag is specified,
* dispatch returns immediately after the event is queued.
*/
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -248,17 +248,35 @@ nsThread::ThreadFunc(void *arg)
}
event->Run(); // unblocks nsThread::Init
event = nsnull;
// Now, process incoming events...
while (!self->ShuttingDown())
NS_ProcessNextEvent(self);
- NS_ProcessPendingEvents(self);
+ // Do NS_ProcessPendingEvents but with special handling to set
+ // mEventsAreDoomed atomically with the removal of the last event. The key
+ // invariant here is that we will never permit PutEvent to succeed if the
+ // event would be left in the queue after our final call to
+ // NS_ProcessPendingEvents.
+ while (PR_TRUE) {
+ {
+ nsAutoLock lock(self->mLock);
+ if (!self->mEvents->HasPendingEvent()) {
+ // No events in the queue, so we will stop now. Don't let any more
+ // events be added, since they won't be processed. It is critical
+ // that no PutEvent can occur between testing that the event queue is
+ // empty and setting mEventsAreDoomed!
+ self->mEventsAreDoomed = PR_TRUE;
+ break;
+ }
+ }
+ NS_ProcessPendingEvents(self);
+ }
// Inform the threadmanager that this thread is going away
nsThreadManager::get()->UnregisterCurrentThread(self);
// Dispatch shutdown ACK
event = new nsThreadShutdownAckEvent(self->mShutdownContext);
self->mShutdownContext->joiningThread->Dispatch(event, NS_DISPATCH_NORMAL);
@@ -270,16 +288,17 @@ nsThread::ThreadFunc(void *arg)
nsThread::nsThread()
: mLock(PR_NewLock())
, mEvents(&mEventsRoot)
, mPriority(PRIORITY_NORMAL)
, mThread(nsnull)
, mRunningEvent(0)
, mShutdownContext(nsnull)
, mShutdownRequired(PR_FALSE)
+ , mEventsAreDoomed(PR_FALSE)
{
}
nsThread::~nsThread()
{
if (mLock)
PR_DestroyLock(mLock);
}
@@ -326,70 +345,70 @@ nsThread::InitCurrentThread()
NS_ENSURE_TRUE(mLock, NS_ERROR_OUT_OF_MEMORY);
mThread = PR_GetCurrentThread();
nsThreadManager::get()->RegisterCurrentThread(this);
return NS_OK;
}
-PRBool
+nsresult
nsThread::PutEvent(nsIRunnable *event)
{
- PRBool rv;
{
nsAutoLock lock(mLock);
- rv = mEvents->PutEvent(event);
+ if (mEventsAreDoomed) {
+ NS_WARNING("An event was posted to a thread that will never run it (rejected)");
+ return NS_ERROR_UNEXPECTED;
+ }
+ if (!mEvents->PutEvent(event))
+ return NS_ERROR_OUT_OF_MEMORY;
}
- if (!rv)
- return PR_FALSE;
nsCOMPtr<nsIThreadObserver> obs = GetObserver();
if (obs)
obs->OnDispatchedEvent(this);
- return PR_TRUE;
+ return NS_OK;
}
//-----------------------------------------------------------------------------
// nsIEventTarget
NS_IMETHODIMP
nsThread::Dispatch(nsIRunnable *event, PRUint32 flags)
{
LOG(("THRD(%p) Dispatch [%p %x]\n", this, event, flags));
NS_ENSURE_ARG_POINTER(event);
- PRBool dispatched;
if (flags & DISPATCH_SYNC) {
nsThread *thread = nsThreadManager::get()->GetCurrentThread();
NS_ENSURE_STATE(thread);
// XXX we should be able to do something better here... we should
// be able to monitor the slot occupied by this event and use
// that to tell us when the event has been processed.
nsRefPtr<nsThreadSyncDispatch> wrapper =
new nsThreadSyncDispatch(thread, event);
if (!wrapper)
return NS_ERROR_OUT_OF_MEMORY;
- dispatched = PutEvent(wrapper);
+ nsresult rv = PutEvent(wrapper);
+ // Don't wait for the event to finish if we didn't dispatch it...
+ if (NS_FAILED(rv))
+ return rv;
while (wrapper->IsPending())
NS_ProcessNextEvent(thread);
- } else {
- NS_ASSERTION(flags == NS_DISPATCH_NORMAL, "unexpected dispatch flags");
- dispatched = PutEvent(event);
+ return rv;
}
- if (NS_UNLIKELY(!dispatched))
- return NS_ERROR_OUT_OF_MEMORY;
-
- return NS_OK;
+ NS_ASSERTION(flags == NS_DISPATCH_NORMAL, "unexpected dispatch flags");
+ return PutEvent(event);
}
NS_IMETHODIMP
nsThread::IsOnCurrentThread(PRBool *result)
{
*result = (PR_GetCurrentThread() == mThread);
return NS_OK;
}
@@ -429,16 +448,17 @@ nsThread::Shutdown()
context.joiningThread = nsThreadManager::get()->GetCurrentThread();
context.shutdownAck = PR_FALSE;
// Set mShutdownContext and wake up the thread in case it is waiting for
// events to process.
nsCOMPtr<nsIRunnable> event = new nsThreadShutdownEvent(this, &context);
if (!event)
return NS_ERROR_OUT_OF_MEMORY;
+ // XXXroc What if posting the event fails due to OOM?
PutEvent(event);
// We could still end up with other events being added after the shutdown
// task, but that's okay because we process pending events in ThreadFunc
// after setting mShutdownContext just before exiting.
// Process events on the current thread until we receive a shutdown ACK.
while (!context.shutdownAck)
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -90,17 +90,17 @@ private:
nsThread::GetObserver(&obs);
return already_AddRefed<nsIThreadObserver>(obs);
}
// Wrappers for event queue methods:
PRBool GetEvent(PRBool mayWait, nsIRunnable **event) {
return mEvents->GetEvent(mayWait, event);
}
- PRBool PutEvent(nsIRunnable *event);
+ nsresult PutEvent(nsIRunnable *event);
// Wrapper for nsEventQueue that supports chaining.
class nsChainedEventQueue {
public:
nsChainedEventQueue(nsIThreadEventFilter *filter = nsnull)
: mNext(nsnull), mFilter(filter) {
}
@@ -108,43 +108,49 @@ private:
return mQueue.IsInitialized();
}
PRBool GetEvent(PRBool mayWait, nsIRunnable **event) {
return mQueue.GetEvent(mayWait, event);
}
PRBool PutEvent(nsIRunnable *event);
+
+ PRBool HasPendingEvent() {
+ return mQueue.HasPendingEvent();
+ }
class nsChainedEventQueue *mNext;
private:
nsCOMPtr<nsIThreadEventFilter> mFilter;
nsEventQueue mQueue;
};
- // This lock protects access to mObserver and mEvents. Both of those fields
- // are only modified on the thread itself (never from another thread). This
- // means that we can avoid holding the lock while using mObserver and mEvents
- // on the thread itself. When calling PutEvent on mEvents, we have to hold
- // the lock to synchronize with PopEventQueue.
+ // This lock protects access to mObserver, mEvents and mEventsAreDoomed.
+ // All of those fields are only modified on the thread itself (never from
+ // another thread). This means that we can avoid holding the lock while
+ // using mObserver and mEvents on the thread itself. When calling PutEvent
+ // on mEvents, we have to hold the lock to synchronize with PopEventQueue.
PRLock *mLock;
nsCOMPtr<nsIThreadObserver> mObserver;
nsChainedEventQueue *mEvents; // never null
nsChainedEventQueue mEventsRoot;
PRInt32 mPriority;
PRThread *mThread;
PRUint32 mRunningEvent; // counter
struct nsThreadShutdownContext *mShutdownContext;
PRPackedBool mShutdownRequired;
PRPackedBool mShutdownPending;
+ // Set to true when events posted to this thread will never run.
+ PRPackedBool mEventsAreDoomed;
};
//-----------------------------------------------------------------------------
class nsThreadSyncDispatch : public nsRunnable {
public:
nsThreadSyncDispatch(nsIThread *origin, nsIRunnable *task)
: mOrigin(origin), mSyncTask(task) {