From f84092468243538a76da6f4ac84b33fc7cd8ef98 Mon Sep 17 00:00:00 2001 From: "roc+@cs.cmu.edu" Date: Tue, 4 Dec 2007 18:18:58 -0800 Subject: [PATCH] Bug 404870. Avoid read access to mThread racing with setting mThread to null (which could crash). Also, detect when the thread is no longer accepting events and treat it as the thread not being there anymore in nsSocketTransportService::Dispatch. --- .../base/src/nsSocketTransportService2.cpp | 41 ++++++++++++++++--- netwerk/base/src/nsSocketTransportService2.h | 5 ++- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/netwerk/base/src/nsSocketTransportService2.cpp b/netwerk/base/src/nsSocketTransportService2.cpp index 23a0a850107a..fb0ef5e85641 100644 --- a/netwerk/base/src/nsSocketTransportService2.cpp +++ b/netwerk/base/src/nsSocketTransportService2.cpp @@ -97,20 +97,37 @@ nsSocketTransportService::~nsSocketTransportService() //----------------------------------------------------------------------------- // event queue (any thread) +already_AddRefed +nsSocketTransportService::GetThreadSafely() +{ + nsAutoLock lock(mLock); + nsIThread* result = mThread; + NS_IF_ADDREF(result); + return result; +} + NS_IMETHODIMP nsSocketTransportService::Dispatch(nsIRunnable *event, PRUint32 flags) { LOG(("STS dispatch [%p]\n", event)); - NS_ENSURE_TRUE(mThread, NS_ERROR_NOT_INITIALIZED); - return mThread->Dispatch(event, flags); + nsCOMPtr thread = GetThreadSafely(); + NS_ENSURE_TRUE(thread, NS_ERROR_NOT_INITIALIZED); + nsresult rv = thread->Dispatch(event, flags); + if (rv == NS_ERROR_UNEXPECTED) { + // Thread is no longer accepting events. We must have just shut it + // down on the main thread. Pretend we never saw it. + rv = NS_ERROR_NOT_INITIALIZED; + } + return rv; } NS_IMETHODIMP nsSocketTransportService::IsOnCurrentThread(PRBool *result) { - NS_ENSURE_TRUE(mThread, NS_ERROR_NOT_INITIALIZED); - return mThread->IsOnCurrentThread(result); + nsCOMPtr thread = GetThreadSafely(); + NS_ENSURE_TRUE(thread, NS_ERROR_NOT_INITIALIZED); + return thread->IsOnCurrentThread(result); } //----------------------------------------------------------------------------- @@ -382,8 +399,15 @@ nsSocketTransportService::Init() } } - nsresult rv = NS_NewThread(&mThread, this); + nsCOMPtr thread; + nsresult rv = NS_NewThread(getter_AddRefs(thread), this); if (NS_FAILED(rv)) return rv; + + { + nsAutoLock lock(mLock); + // Install our mThread, protecting against concurrent readers + thread.swap(mThread); + } mInitialized = PR_TRUE; return NS_OK; @@ -416,7 +440,12 @@ nsSocketTransportService::Shutdown() // join with thread mThread->Shutdown(); - NS_RELEASE(mThread); + { + nsAutoLock lock(mLock); + // Drop our reference to mThread and make sure that any concurrent + // readers are excluded + mThread = nsnull; + } mInitialized = PR_FALSE; mShuttingDown = PR_FALSE; diff --git a/netwerk/base/src/nsSocketTransportService2.h b/netwerk/base/src/nsSocketTransportService2.h index 2b5042ec3076..3f1ebc2bf5a8 100644 --- a/netwerk/base/src/nsSocketTransportService2.h +++ b/netwerk/base/src/nsSocketTransportService2.h @@ -178,7 +178,7 @@ private: // misc (any thread) //------------------------------------------------------------------------- - nsIThread *mThread; + nsCOMPtr mThread; // protected by mLock PRFileDesc *mThreadEvent; // protected by mLock. mThreadEvent may change // if the old pollable event is broken. only @@ -190,6 +190,9 @@ private: PRBool mAutodialEnabled; // pref to control autodial code + // Returns mThread, protecting the get-and-addref with mLock + already_AddRefed GetThreadSafely(); + //------------------------------------------------------------------------- // initialization and shutdown (any thread) //-------------------------------------------------------------------------