diff --git a/security/manager/ssl/src/nsNSSCallbacks.cpp b/security/manager/ssl/src/nsNSSCallbacks.cpp index a94e9bfae45..79aa5d93269 100644 --- a/security/manager/ssl/src/nsNSSCallbacks.cpp +++ b/security/manager/ssl/src/nsNSSCallbacks.cpp @@ -86,29 +86,15 @@ public: ~nsHTTPDownloadEvent(); NS_IMETHOD Run(); - void Cancel(); nsNSSHttpRequestSession *mRequestSession; // no ownership nsCOMPtr mListener; PRBool mResponsibleForDoneSignal; - -protected: - PRLock *mLock; - - // no nsCOMPtr. When I use it, I get assertions about - // loadgroup not being thread safe. - // So, let's use a raw pointer and ensure we only create and destroy - // it on the network thread ourselves. - nsILoadGroup *mLoadGroup; - PRThread *mLoadGroupOwnerThread; }; nsHTTPDownloadEvent::nsHTTPDownloadEvent() :mResponsibleForDoneSignal(PR_TRUE) -,mLock(nsnull) -,mLoadGroup(nsnull) -,mLoadGroupOwnerThread(nsnull) { } @@ -116,17 +102,11 @@ nsHTTPDownloadEvent::~nsHTTPDownloadEvent() { if (mResponsibleForDoneSignal && mListener) mListener->send_done_signal(); - if (mLock) - PR_DestroyLock(mLock); } NS_IMETHODIMP nsHTTPDownloadEvent::Run() { - mLock = PR_NewLock(); - if (!mLock) - return NS_ERROR_OUT_OF_MEMORY; - if (!mListener) return NS_OK; @@ -141,16 +121,8 @@ nsHTTPDownloadEvent::Run() // Create a loadgroup for this new channel. This way if the channel // is redirected, we'll have a way to cancel the resulting channel. - nsCOMPtr lg; - { - nsAutoLock locker(mLock); - lg = do_CreateInstance(NS_LOADGROUP_CONTRACTID); - mLoadGroup = lg.get(); - NS_ADDREF(mLoadGroup); - mLoadGroupOwnerThread = PR_GetCurrentThread(); - } + nsCOMPtr lg = do_CreateInstance(NS_LOADGROUP_CONTRACTID); chan->SetLoadGroup(lg); - lg = nsnull; if (mRequestSession->mHasPostData) { @@ -179,6 +151,10 @@ nsHTTPDownloadEvent::Run() mResponsibleForDoneSignal = PR_FALSE; mListener->mResponsibleForDoneSignal = PR_TRUE; + mListener->mLoadGroup = lg.get(); + NS_ADDREF(mListener->mLoadGroup); + mListener->mLoadGroupOwnerThread = PR_GetCurrentThread(); + rv = NS_NewStreamLoader(getter_AddRefs(mListener->mLoader), mListener); @@ -188,47 +164,25 @@ nsHTTPDownloadEvent::Run() if (NS_FAILED(rv)) { mListener->mResponsibleForDoneSignal = PR_FALSE; mResponsibleForDoneSignal = PR_TRUE; + + NS_RELEASE(mListener->mLoadGroup); + mListener->mLoadGroup = nsnull; + mListener->mLoadGroupOwnerThread = nsnull; } return NS_OK; } struct nsCancelHTTPDownloadEvent : nsRunnable { - nsRefPtr mEvent; + nsCOMPtr mListener; NS_IMETHOD Run() { - mEvent->Cancel(); - mEvent = nsnull; + mListener->FreeLoadGroup(PR_TRUE); + mListener = nsnull; return NS_OK; } }; -void -nsHTTPDownloadEvent::Cancel() -{ - nsILoadGroup *lg = nsnull; - - if (mLock) { - nsAutoLock locker(mLock); - - if (mLoadGroup) { - if (mLoadGroupOwnerThread != PR_GetCurrentThread()) { - NS_ASSERTION(PR_FALSE, - "attempt to access nsHTTPDownloadEvent::mLoadGroup on multiple threads"); - } - else { - lg = mLoadGroup; - mLoadGroup = nsnull; - } - } - } - - if (lg) { - lg->Cancel(NS_ERROR_ABORT); - NS_RELEASE(lg); - } -} - SECStatus nsNSSHttpServerSession::createSessionFcn(const char *host, PRUint16 portnum, SEC_HTTP_SERVER_SESSION *pSession) @@ -471,7 +425,7 @@ nsNSSHttpRequestSession::internal_send_receive_attempt(PRBool &retryable_error, request_canceled = PR_TRUE; nsRefPtr cancelevent = new nsCancelHTTPDownloadEvent; - cancelevent->mEvent = event; + cancelevent->mListener = mListener; rv = NS_DispatchToMainThread(cancelevent); if (NS_FAILED(rv)) { NS_WARNING("cannot post cancel event"); @@ -583,7 +537,9 @@ nsHTTPListener::nsHTTPListener() mLock(nsnull), mCondition(nsnull), mWaitFlag(PR_TRUE), - mResponsibleForDoneSignal(PR_FALSE) + mResponsibleForDoneSignal(PR_FALSE), + mLoadGroup(nsnull), + mLoadGroupOwnerThread(nsnull) { } @@ -623,6 +579,34 @@ nsHTTPListener::~nsHTTPListener() NS_IMPL_THREADSAFE_ISUPPORTS1(nsHTTPListener, nsIStreamLoaderObserver) +void +nsHTTPListener::FreeLoadGroup(PRBool aCancelLoad) +{ + nsILoadGroup *lg = nsnull; + + if (mLock) { + nsAutoLock locker(mLock); + + if (mLoadGroup) { + if (mLoadGroupOwnerThread != PR_GetCurrentThread()) { + NS_ASSERTION(PR_FALSE, + "attempt to access nsHTTPDownloadEvent::mLoadGroup on multiple threads, leaking it!"); + } + else { + lg = mLoadGroup; + mLoadGroup = nsnull; + } + } + } + + if (lg) { + if (aCancelLoad) { + lg->Cancel(NS_ERROR_ABORT); + } + NS_RELEASE(lg); + } +} + NS_IMETHODIMP nsHTTPListener::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext, @@ -632,6 +616,8 @@ nsHTTPListener::OnStreamComplete(nsIStreamLoader* aLoader, { mResultCode = aStatus; + FreeLoadGroup(PR_FALSE); + nsCOMPtr req; nsCOMPtr hchan; diff --git a/security/manager/ssl/src/nsNSSCallbacks.h b/security/manager/ssl/src/nsNSSCallbacks.h index 68d5f358a0f..abf314df902 100644 --- a/security/manager/ssl/src/nsNSSCallbacks.h +++ b/security/manager/ssl/src/nsNSSCallbacks.h @@ -86,6 +86,14 @@ public: PRBool mResponsibleForDoneSignal; void send_done_signal(); + + // no nsCOMPtr. When I use it, I get assertions about + // loadgroup not being thread safe. + // So, let's use a raw pointer and ensure we only create and destroy + // it on the network thread ourselves. + nsILoadGroup *mLoadGroup; + PRThread *mLoadGroupOwnerThread; + void FreeLoadGroup(PRBool aCancelLoad); }; class nsNSSHttpServerSession