From 06d7c103e1af4368b263cc2e21041616258b5818 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Fri, 16 Oct 2015 11:36:59 -0400 Subject: [PATCH] bug 1215552 - nsHttpConnectionMgr::PostEvent shouldnt manually ref count r=hurley --- netwerk/base/ARefBase.h | 30 ++ netwerk/base/EventTokenBucket.cpp | 2 - netwerk/base/EventTokenBucket.h | 9 +- .../protocol/http/ConnectionDiagnostics.cpp | 2 +- netwerk/protocol/http/nsHttpConnection.h | 2 + netwerk/protocol/http/nsHttpConnectionInfo.h | 5 +- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 369 +++++++++--------- netwerk/protocol/http/nsHttpConnectionMgr.h | 118 ++---- netwerk/protocol/http/nsHttpTransaction.h | 2 + 9 files changed, 250 insertions(+), 289 deletions(-) create mode 100644 netwerk/base/ARefBase.h diff --git a/netwerk/base/ARefBase.h b/netwerk/base/ARefBase.h new file mode 100644 index 000000000000..8b9582268ae6 --- /dev/null +++ b/netwerk/base/ARefBase.h @@ -0,0 +1,30 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ + +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_net_ARefBase_h +#define mozilla_net_ARefBase_h + +namespace mozilla { namespace net { + +// This is an abstract class that can be pointed to by either +// nsCOMPtr or nsRefPtr. nsHttpConnectionMgr uses it for generic +// objects that need to be reference counted - similiar to nsISupports +// but it may or may not be xpcom. + +class ARefBase +{ +public: + ARefBase() {} + virtual ~ARefBase() {} + + NS_IMETHOD_ (MozExternalRefCountType) AddRef() = 0; + NS_IMETHOD_ (MozExternalRefCountType) Release() = 0; +}; + +} // namespace net +} // namespace mozilla + +#endif diff --git a/netwerk/base/EventTokenBucket.cpp b/netwerk/base/EventTokenBucket.cpp index 088b7636d09e..22b7dc0d6ae9 100644 --- a/netwerk/base/EventTokenBucket.cpp +++ b/netwerk/base/EventTokenBucket.cpp @@ -90,7 +90,6 @@ EventTokenBucket::EventTokenBucket(uint32_t eventsPerSecond, , mFineGrainResetTimerArmed(false) #endif { - MOZ_COUNT_CTOR(EventTokenBucket); mLastUpdate = TimeStamp::Now(); MOZ_ASSERT(NS_IsMainThread()); @@ -112,7 +111,6 @@ EventTokenBucket::~EventTokenBucket() SOCKET_LOG(("EventTokenBucket::dtor %p events=%d\n", this, mEvents.GetSize())); - MOZ_COUNT_DTOR(EventTokenBucket); if (mTimer && mTimerArmed) mTimer->Cancel(); diff --git a/netwerk/base/EventTokenBucket.h b/netwerk/base/EventTokenBucket.h index eba755251422..fe760e76d2ff 100644 --- a/netwerk/base/EventTokenBucket.h +++ b/netwerk/base/EventTokenBucket.h @@ -7,6 +7,7 @@ #ifndef NetEventTokenBucket_h__ #define NetEventTokenBucket_h__ +#include "ARefBase.h" #include "nsCOMPtr.h" #include "nsDeque.h" #include "nsITimer.h" @@ -59,7 +60,7 @@ namespace net { class EventTokenBucket; -class ATokenBucketEvent +class ATokenBucketEvent { public: virtual void OnTokenBucketAdmitted() = 0; @@ -67,10 +68,8 @@ public: class TokenBucketCancelable; -class EventTokenBucket : public nsITimerCallback +class EventTokenBucket : public nsITimerCallback, public ARefBase { - virtual ~EventTokenBucket(); - public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_NSITIMERCALLBACK @@ -93,6 +92,8 @@ public: nsresult SubmitEvent(ATokenBucketEvent *event, nsICancelable **cancelable); private: + virtual ~EventTokenBucket(); + friend class RunNotifyEvent; friend class SetTimerEvent; diff --git a/netwerk/protocol/http/ConnectionDiagnostics.cpp b/netwerk/protocol/http/ConnectionDiagnostics.cpp index 28873c760808..a331e3dbfabd 100644 --- a/netwerk/protocol/http/ConnectionDiagnostics.cpp +++ b/netwerk/protocol/http/ConnectionDiagnostics.cpp @@ -28,7 +28,7 @@ nsHttpConnectionMgr::PrintDiagnostics() } void -nsHttpConnectionMgr::OnMsgPrintDiagnostics(int32_t, void *) +nsHttpConnectionMgr::OnMsgPrintDiagnostics(int32_t, ARefBase *) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); diff --git a/netwerk/protocol/http/nsHttpConnection.h b/netwerk/protocol/http/nsHttpConnection.h index ff40a39e1ff5..623a8307ee51 100644 --- a/netwerk/protocol/http/nsHttpConnection.h +++ b/netwerk/protocol/http/nsHttpConnection.h @@ -14,6 +14,7 @@ #include "prinrval.h" #include "TunnelUtils.h" #include "mozilla/Mutex.h" +#include "ARefBase.h" #include "nsIAsyncInputStream.h" #include "nsIAsyncOutputStream.h" @@ -43,6 +44,7 @@ class nsHttpConnection final : public nsAHttpSegmentReader , public nsITransportEventSink , public nsIInterfaceRequestor , public NudgeTunnelCallback + , public ARefBase { virtual ~nsHttpConnection(); diff --git a/netwerk/protocol/http/nsHttpConnectionInfo.h b/netwerk/protocol/http/nsHttpConnectionInfo.h index 01bbc25489cb..3b075a4113e0 100644 --- a/netwerk/protocol/http/nsHttpConnectionInfo.h +++ b/netwerk/protocol/http/nsHttpConnectionInfo.h @@ -12,6 +12,7 @@ #include "nsCOMPtr.h" #include "nsStringFwd.h" #include "mozilla/Logging.h" +#include "ARefBase.h" extern PRLogModuleInfo *gHttpLog; @@ -30,7 +31,7 @@ extern PRLogModuleInfo *gHttpLog; namespace mozilla { namespace net { -class nsHttpConnectionInfo +class nsHttpConnectionInfo: public ARefBase { public: nsHttpConnectionInfo(const nsACString &originHost, @@ -162,7 +163,7 @@ private: bool mUsingConnect; // if will use CONNECT with http proxy nsCString mNPNToken; -// for nsRefPtr +// for RefPtr NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsHttpConnectionInfo) }; diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 14dfe11e9401..d80c1e89da66 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -147,12 +147,25 @@ nsHttpConnectionMgr::Init(uint16_t maxConns, return EnsureSocketThreadTarget(); } +class BoolWrapper : public ARefBase +{ +public: + BoolWrapper() : mBool(false) {} + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(BoolWrapper) + +public: // intentional! + bool mBool; + +private: + virtual ~BoolWrapper() {} +}; + nsresult nsHttpConnectionMgr::Shutdown() { LOG(("nsHttpConnectionMgr::Shutdown\n")); - bool shutdown = false; + RefPtr shutdownWrapper = new BoolWrapper(); { ReentrantMonitorAutoEnter mon(mReentrantMonitor); @@ -161,7 +174,7 @@ nsHttpConnectionMgr::Shutdown() return NS_OK; nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgShutdown, - 0, &shutdown); + 0, shutdownWrapper); // release our reference to the STS to prevent further events // from being posted. this is how we indicate that we are @@ -176,14 +189,41 @@ nsHttpConnectionMgr::Shutdown() } // wait for shutdown event to complete - while (!shutdown) + while (!shutdownWrapper->mBool) { NS_ProcessNextEvent(NS_GetCurrentThread()); + } return NS_OK; } +class ConnEvent : public nsRunnable +{ +public: + ConnEvent(nsHttpConnectionMgr *mgr, + nsConnEventHandler handler, int32_t iparam, ARefBase *vparam) + : mMgr(mgr) + , mHandler(handler) + , mIParam(iparam) + , mVParam(vparam) {} + + NS_IMETHOD Run() + { + (mMgr->*mHandler)(mIParam, mVParam); + return NS_OK; + } + +private: + virtual ~ConnEvent() {} + + RefPtr mMgr; + nsConnEventHandler mHandler; + int32_t mIParam; + RefPtr mVParam; +}; + nsresult -nsHttpConnectionMgr::PostEvent(nsConnEventHandler handler, int32_t iparam, void *vparam) +nsHttpConnectionMgr::PostEvent(nsConnEventHandler handler, + int32_t iparam, ARefBase *vparam) { EnsureSocketThreadTarget(); @@ -195,7 +235,7 @@ nsHttpConnectionMgr::PostEvent(nsConnEventHandler handler, int32_t iparam, void rv = NS_ERROR_NOT_INITIALIZED; } else { - nsCOMPtr event = new nsConnEvent(this, handler, iparam, vparam); + nsCOMPtr event = new ConnEvent(this, handler, iparam, vparam); rv = mSocketThreadTarget->Dispatch(event, NS_DISPATCH_NORMAL); } return rv; @@ -293,37 +333,22 @@ nsresult nsHttpConnectionMgr::AddTransaction(nsHttpTransaction *trans, int32_t priority) { LOG(("nsHttpConnectionMgr::AddTransaction [trans=%p %d]\n", trans, priority)); - - NS_ADDREF(trans); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgNewTransaction, priority, trans); - if (NS_FAILED(rv)) - NS_RELEASE(trans); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgNewTransaction, priority, trans); } nsresult nsHttpConnectionMgr::RescheduleTransaction(nsHttpTransaction *trans, int32_t priority) { LOG(("nsHttpConnectionMgr::RescheduleTransaction [trans=%p %d]\n", trans, priority)); - - NS_ADDREF(trans); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgReschedTransaction, priority, trans); - if (NS_FAILED(rv)) - NS_RELEASE(trans); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgReschedTransaction, priority, trans); } nsresult nsHttpConnectionMgr::CancelTransaction(nsHttpTransaction *trans, nsresult reason) { LOG(("nsHttpConnectionMgr::CancelTransaction [trans=%p reason=%x]\n", trans, reason)); - - NS_ADDREF(trans); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgCancelTransaction, - static_cast(reason), trans); - if (NS_FAILED(rv)) - NS_RELEASE(trans); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgCancelTransaction, + static_cast(reason), trans); } nsresult @@ -350,30 +375,18 @@ nsHttpConnectionMgr::VerifyTraffic() return PostEvent(&nsHttpConnectionMgr::OnMsgVerifyTraffic); } - nsresult nsHttpConnectionMgr::DoShiftReloadConnectionCleanup(nsHttpConnectionInfo *aCI) { - RefPtr connInfo(aCI); - - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup, - 0, connInfo); - if (NS_SUCCEEDED(rv)) - unused << connInfo.forget(); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup, + 0, aCI); } -class SpeculativeConnectArgs +class SpeculativeConnectArgs : public ARefBase { - virtual ~SpeculativeConnectArgs() {} - public: SpeculativeConnectArgs() { mOverridesOK = false; } - - // Added manually so we can use nsRefPtr without inheriting from - // nsISupports - NS_IMETHOD_(MozExternalRefCountType) AddRef(void); - NS_IMETHOD_(MozExternalRefCountType) Release(void); + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SpeculativeConnectArgs) public: // intentional! RefPtr mTrans; @@ -385,16 +398,11 @@ public: // intentional! bool mIsFromPredictor; bool mAllow1918; - // As above, added manually so we can use nsRefPtr without inheriting from - // nsISupports -protected: - ThreadSafeAutoRefCnt mRefCnt; +private: + virtual ~SpeculativeConnectArgs() {} NS_DECL_OWNINGTHREAD }; -NS_IMPL_ADDREF(SpeculativeConnectArgs) -NS_IMPL_RELEASE(SpeculativeConnectArgs) - nsresult nsHttpConnectionMgr::SpeculativeConnect(nsHttpConnectionInfo *ci, nsIInterfaceRequestor *callbacks, @@ -444,11 +452,7 @@ nsHttpConnectionMgr::SpeculativeConnect(nsHttpConnectionInfo *ci, overrider->GetAllow1918(&args->mAllow1918); } - nsresult rv = - PostEvent(&nsHttpConnectionMgr::OnMsgSpeculativeConnect, 0, args); - if (NS_SUCCEEDED(rv)) - unused << args.forget(); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgSpeculativeConnect, 0, args); } nsresult @@ -465,58 +469,49 @@ nsresult nsHttpConnectionMgr::ReclaimConnection(nsHttpConnection *conn) { LOG(("nsHttpConnectionMgr::ReclaimConnection [conn=%p]\n", conn)); - - NS_ADDREF(conn); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgReclaimConnection, 0, conn); - if (NS_FAILED(rv)) - NS_RELEASE(conn); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgReclaimConnection, 0, conn); } // A structure used to marshall 2 pointers across the various necessary // threads to complete an HTTP upgrade. -class nsCompleteUpgradeData +class nsCompleteUpgradeData : public ARefBase { public: -nsCompleteUpgradeData(nsAHttpConnection *aConn, - nsIHttpUpgradeListener *aListener) - : mConn(aConn), mUpgradeListener(aListener) {} + nsCompleteUpgradeData(nsAHttpConnection *aConn, + nsIHttpUpgradeListener *aListener) + : mConn(aConn) + , mUpgradeListener(aListener) { } + + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsCompleteUpgradeData) RefPtr mConn; nsCOMPtr mUpgradeListener; +private: + virtual ~nsCompleteUpgradeData() { } }; nsresult nsHttpConnectionMgr::CompleteUpgrade(nsAHttpConnection *aConn, nsIHttpUpgradeListener *aUpgradeListener) { - nsCompleteUpgradeData *data = + RefPtr data = new nsCompleteUpgradeData(aConn, aUpgradeListener); - nsresult rv; - rv = PostEvent(&nsHttpConnectionMgr::OnMsgCompleteUpgrade, 0, data); - if (NS_FAILED(rv)) - delete data; - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgCompleteUpgrade, 0, data); } nsresult nsHttpConnectionMgr::UpdateParam(nsParamName name, uint16_t value) { uint32_t param = (uint32_t(name) << 16) | uint32_t(value); - return PostEvent(&nsHttpConnectionMgr::OnMsgUpdateParam, 0, - (void *)(uintptr_t) param); + return PostEvent(&nsHttpConnectionMgr::OnMsgUpdateParam, + static_cast(param), nullptr); } nsresult nsHttpConnectionMgr::ProcessPendingQ(nsHttpConnectionInfo *ci) { LOG(("nsHttpConnectionMgr::ProcessPendingQ [ci=%s]\n", ci->HashKey().get())); - - NS_ADDREF(ci); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgProcessPendingQ, 0, ci); - if (NS_FAILED(rv)) - NS_RELEASE(ci); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgProcessPendingQ, 0, ci); } nsresult @@ -527,25 +522,19 @@ nsHttpConnectionMgr::ProcessPendingQ() } void -nsHttpConnectionMgr::OnMsgUpdateRequestTokenBucket(int32_t, void *param) +nsHttpConnectionMgr::OnMsgUpdateRequestTokenBucket(int32_t, ARefBase *param) { - RefPtr tokenBucket = - dont_AddRef(static_cast(param)); + EventTokenBucket *tokenBucket = static_cast(param); gHttpHandler->SetRequestTokenBucket(tokenBucket); } nsresult nsHttpConnectionMgr::UpdateRequestTokenBucket(EventTokenBucket *aBucket) { - RefPtr bucket(aBucket); - // Call From main thread when a new EventTokenBucket has been made in order // to post the new value to the socket thread. - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgUpdateRequestTokenBucket, - 0, bucket); - if (NS_SUCCEEDED(rv)) - unused << bucket.forget(); - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgUpdateRequestTokenBucket, + 0, aBucket); } PLDHashOperator @@ -1196,7 +1185,7 @@ nsHttpConnectionMgr::SupportsPipelining(nsHttpConnectionInfo *ci) // nsHttpPipelineFeedback used to hold references across events -class nsHttpPipelineFeedback +class nsHttpPipelineFeedback : public ARefBase { public: nsHttpPipelineFeedback(nsHttpConnectionInfo *ci, @@ -1209,14 +1198,14 @@ public: { } - ~nsHttpPipelineFeedback() - { - } RefPtr mConnInfo; RefPtr mConn; nsHttpConnectionMgr::PipelineFeedbackInfoType mInfo; uint32_t mData; +private: + ~nsHttpPipelineFeedback() {} + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsHttpPipelineFeedback) }; void @@ -1230,18 +1219,13 @@ nsHttpConnectionMgr::PipelineFeedbackInfo(nsHttpConnectionInfo *ci, // Post this to the socket thread if we are not running there already if (PR_GetCurrentThread() != gSocketThread) { - nsHttpPipelineFeedback *fb = new nsHttpPipelineFeedback(ci, info, - conn, data); - - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgProcessFeedback, - 0, fb); - if (NS_FAILED(rv)) - delete fb; + RefPtr fb = + new nsHttpPipelineFeedback(ci, info, conn, data); + PostEvent(&nsHttpConnectionMgr::OnMsgProcessFeedback, 0, fb); return; } nsConnectionEntry *ent = mCT.Get(ci->HashKey()); - if (ent) ent->OnPipelineFeedbackInfo(info, conn, data); } @@ -1924,6 +1908,44 @@ nsHttpConnectionMgr::DispatchTransaction(nsConnectionEntry *ent, return rv; } +//----------------------------------------------------------------------------- +// ConnectionHandle +// +// thin wrapper around a real connection, used to keep track of references +// to the connection to determine when the connection may be reused. the +// transaction (or pipeline) owns a reference to this handle. this extra +// layer of indirection greatly simplifies consumer code, avoiding the +// need for consumer code to know when to give the connection back to the +// connection manager. +// +class ConnectionHandle : public nsAHttpConnection +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSAHTTPCONNECTION(mConn) + + explicit ConnectionHandle(nsHttpConnection *conn) { NS_ADDREF(mConn = conn); } + nsHttpConnection *mConn; + +private: + virtual ~ConnectionHandle(); +}; + +nsAHttpConnection * +nsHttpConnectionMgr::MakeConnectionHandle(nsHttpConnection *aWrapped) +{ + return new ConnectionHandle(aWrapped); +} + +ConnectionHandle::~ConnectionHandle() +{ + if (mConn) { + gHttpHandler->ReclaimConnection(mConn); + NS_RELEASE(mConn); + } +} + +NS_IMPL_ISUPPORTS0(ConnectionHandle) // Use this method for dispatching nsAHttpTransction's. It can only safely be // used upon first use of a connection when NPN has not negotiated SPDY vs @@ -1963,7 +1985,7 @@ nsHttpConnectionMgr::DispatchAbstractTransaction(nsConnectionEntry *ent, transaction = aTrans; } - RefPtr handle = new nsConnectionHandle(conn); + RefPtr handle = new ConnectionHandle(conn); // give the transaction the indirect reference to the connection. transaction->SetConnection(handle); @@ -2253,7 +2275,7 @@ nsHttpConnectionMgr::ProcessSpdyPendingQCB(const nsACString &key, } void -nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ(int32_t, void *) +nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ(int32_t, ARefBase *) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ\n")); @@ -2294,7 +2316,7 @@ nsHttpConnectionMgr::GetSpdyPreferredConn(nsConnectionEntry *ent) //----------------------------------------------------------------------------- void -nsHttpConnectionMgr::OnMsgShutdown(int32_t, void *param) +nsHttpConnectionMgr::OnMsgShutdown(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgShutdown\n")); @@ -2317,41 +2339,40 @@ nsHttpConnectionMgr::OnMsgShutdown(int32_t, void *param) // signal shutdown complete nsCOMPtr runnable = - new nsConnEvent(this, &nsHttpConnectionMgr::OnMsgShutdownConfirm, - 0, param); + new ConnEvent(this, &nsHttpConnectionMgr::OnMsgShutdownConfirm, + 0, param); NS_DispatchToMainThread(runnable); } void -nsHttpConnectionMgr::OnMsgShutdownConfirm(int32_t priority, void *param) +nsHttpConnectionMgr::OnMsgShutdownConfirm(int32_t priority, ARefBase *param) { MOZ_ASSERT(NS_IsMainThread()); LOG(("nsHttpConnectionMgr::OnMsgShutdownConfirm\n")); - bool *shutdown = static_cast(param); - *shutdown = true; + BoolWrapper *shutdown = static_cast(param); + shutdown->mBool = true; } void -nsHttpConnectionMgr::OnMsgNewTransaction(int32_t priority, void *param) +nsHttpConnectionMgr::OnMsgNewTransaction(int32_t priority, ARefBase *param) { LOG(("nsHttpConnectionMgr::OnMsgNewTransaction [trans=%p]\n", param)); - nsHttpTransaction *trans = (nsHttpTransaction *) param; + nsHttpTransaction *trans = static_cast(param); trans->SetPriority(priority); nsresult rv = ProcessNewTransaction(trans); if (NS_FAILED(rv)) trans->Close(rv); // for whatever its worth - NS_RELEASE(trans); } void -nsHttpConnectionMgr::OnMsgReschedTransaction(int32_t priority, void *param) +nsHttpConnectionMgr::OnMsgReschedTransaction(int32_t priority, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgReschedTransaction [trans=%p]\n", param)); - nsHttpTransaction *trans = (nsHttpTransaction *) param; + nsHttpTransaction *trans = static_cast(param); trans->SetPriority(priority); nsConnectionEntry *ent = LookupConnectionEntry(trans->ConnectionInfo(), @@ -2364,19 +2385,16 @@ nsHttpConnectionMgr::OnMsgReschedTransaction(int32_t priority, void *param) InsertTransactionSorted(ent->mPendingQ, trans); } } - - NS_RELEASE(trans); } void -nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, void *param) +nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgCancelTransaction [trans=%p]\n", param)); nsresult closeCode = static_cast(reason); - RefPtr trans = - dont_AddRef(static_cast(param)); + nsHttpTransaction *trans = static_cast(param); // // if the transaction owns a connection and the transaction is not done, @@ -2394,7 +2412,7 @@ nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, void *param) int32_t index = ent->mPendingQ.IndexOf(trans); if (index >= 0) { LOG(("nsHttpConnectionMgr::OnMsgCancelTransaction [trans=%p]" - " found in pending queue\n", trans.get())); + " found in pending queue\n", trans)); ent->mPendingQ.RemoveElementAt(index); nsHttpTransaction *temp = trans; NS_RELEASE(temp); // b/c NS_RELEASE nulls its argument! @@ -2429,7 +2447,7 @@ nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, void *param) if (liveTransaction && liveTransaction->IsNullTransaction()) { LOG(("nsHttpConnectionMgr::OnMsgCancelTransaction [trans=%p] " "also canceling Null Transaction %p on conn %p\n", - trans.get(), liveTransaction, activeConn)); + trans, liveTransaction, activeConn)); activeConn->CloseTransaction(liveTransaction, closeCode); } } @@ -2437,10 +2455,10 @@ nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, void *param) } void -nsHttpConnectionMgr::OnMsgProcessPendingQ(int32_t, void *param) +nsHttpConnectionMgr::OnMsgProcessPendingQ(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); - nsHttpConnectionInfo *ci = (nsHttpConnectionInfo *) param; + nsHttpConnectionInfo *ci = static_cast(param); if (!ci) { LOG(("nsHttpConnectionMgr::OnMsgProcessPendingQ [ci=nullptr]\n")); @@ -2459,30 +2477,22 @@ nsHttpConnectionMgr::OnMsgProcessPendingQ(int32_t, void *param) // for the specified connection info. walk the connection table... mCT.Enumerate(ProcessOneTransactionCB, this); } - - NS_RELEASE(ci); } nsresult -nsHttpConnectionMgr::CancelTransactions(nsHttpConnectionInfo *aCI, nsresult code) +nsHttpConnectionMgr::CancelTransactions(nsHttpConnectionInfo *ci, nsresult code) { - RefPtr ci(aCI); LOG(("nsHttpConnectionMgr::CancelTransactions %s\n",ci->HashKey().get())); int32_t intReason = static_cast(code); - nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgCancelTransactions, intReason, ci); - if (NS_SUCCEEDED(rv)) { - unused << ci.forget(); - } - return rv; + return PostEvent(&nsHttpConnectionMgr::OnMsgCancelTransactions, intReason, ci); } void -nsHttpConnectionMgr::OnMsgCancelTransactions(int32_t code, void *param) +nsHttpConnectionMgr::OnMsgCancelTransactions(int32_t code, ARefBase *param) { nsresult reason = static_cast(code); - RefPtr ci = - dont_AddRef(static_cast(param)); + nsHttpConnectionInfo *ci = static_cast(param); nsConnectionEntry *ent = mCT.Get(ci->HashKey()); LOG(("nsHttpConnectionMgr::OnMsgCancelTransactions %s %p\n", ci->HashKey().get(), ent)); @@ -2502,7 +2512,7 @@ nsHttpConnectionMgr::OnMsgCancelTransactions(int32_t code, void *param) } void -nsHttpConnectionMgr::OnMsgPruneDeadConnections(int32_t, void *) +nsHttpConnectionMgr::OnMsgPruneDeadConnections(int32_t, ARefBase *) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgPruneDeadConnections\n")); @@ -2517,7 +2527,7 @@ nsHttpConnectionMgr::OnMsgPruneDeadConnections(int32_t, void *) } void -nsHttpConnectionMgr::OnMsgPruneNoTraffic(int32_t, void *) +nsHttpConnectionMgr::OnMsgPruneNoTraffic(int32_t, ARefBase *) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgPruneNoTraffic\n")); @@ -2529,7 +2539,7 @@ nsHttpConnectionMgr::OnMsgPruneNoTraffic(int32_t, void *) } void -nsHttpConnectionMgr::OnMsgVerifyTraffic(int32_t, void *) +nsHttpConnectionMgr::OnMsgVerifyTraffic(int32_t, ARefBase *) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgVerifyTraffic\n")); @@ -2561,13 +2571,12 @@ nsHttpConnectionMgr::OnMsgVerifyTraffic(int32_t, void *) } void -nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup(int32_t, void *param) +nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup(int32_t, ARefBase *param) { LOG(("nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup\n")); MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); - RefPtr ci = - dont_AddRef(static_cast(param)); + nsHttpConnectionInfo *ci = static_cast(param); mCT.Enumerate(ClosePersistentConnectionsCB, this); if (ci) @@ -2575,12 +2584,12 @@ nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup(int32_t, void *param) } void -nsHttpConnectionMgr::OnMsgReclaimConnection(int32_t, void *param) +nsHttpConnectionMgr::OnMsgReclaimConnection(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); LOG(("nsHttpConnectionMgr::OnMsgReclaimConnection [conn=%p]\n", param)); - nsHttpConnection *conn = (nsHttpConnection *) param; + nsHttpConnection *conn = static_cast(param); // // 1) remove the connection from the active list @@ -2601,8 +2610,7 @@ nsHttpConnectionMgr::OnMsgReclaimConnection(int32_t, void *param) } MOZ_ASSERT(ent); - nsHttpConnectionInfo *ci = nullptr; - NS_ADDREF(ci = ent->mConnInfo); + RefPtr ci(ent->mConnInfo); // If the connection is in the active list, remove that entry // and the reference held by the mActiveConns list. @@ -2668,15 +2676,14 @@ nsHttpConnectionMgr::OnMsgReclaimConnection(int32_t, void *param) conn->Close(NS_ERROR_ABORT); } - OnMsgProcessPendingQ(0, ci); // releases |ci| - NS_RELEASE(conn); + OnMsgProcessPendingQ(0, ci); } void -nsHttpConnectionMgr::OnMsgCompleteUpgrade(int32_t, void *param) +nsHttpConnectionMgr::OnMsgCompleteUpgrade(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); - nsCompleteUpgradeData *data = (nsCompleteUpgradeData *) param; + nsCompleteUpgradeData *data = static_cast(param); LOG(("nsHttpConnectionMgr::OnMsgCompleteUpgrade " "this=%p conn=%p listener=%p\n", this, data->mConn.get(), data->mUpgradeListener.get())); @@ -2694,14 +2701,14 @@ nsHttpConnectionMgr::OnMsgCompleteUpgrade(int32_t, void *param) data->mUpgradeListener->OnTransportAvailable(socketTransport, socketIn, socketOut); - delete data; } void -nsHttpConnectionMgr::OnMsgUpdateParam(int32_t, void *param) +nsHttpConnectionMgr::OnMsgUpdateParam(int32_t inParam, ARefBase *) { - uint16_t name = (NS_PTR_TO_INT32(param) & 0xFFFF0000) >> 16; - uint16_t value = NS_PTR_TO_INT32(param) & 0x0000FFFF; + uint32_t param = static_cast(inParam); + uint16_t name = ((param) & 0xFFFF0000) >> 16; + uint16_t value = param & 0x0000FFFF; switch (name) { case MAX_CONNECTIONS: @@ -2735,13 +2742,11 @@ nsHttpConnectionMgr::nsConnectionEntry::~nsConnectionEntry() } void -nsHttpConnectionMgr::OnMsgProcessFeedback(int32_t, void *param) +nsHttpConnectionMgr::OnMsgProcessFeedback(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); - nsHttpPipelineFeedback *fb = (nsHttpPipelineFeedback *)param; - + nsHttpPipelineFeedback *fb = static_cast(param); PipelineFeedbackInfo(fb->mConnInfo, fb->mInfo, fb->mConn, fb->mData); - delete fb; } // Read Timeout Tick handlers @@ -2854,19 +2859,6 @@ nsHttpConnectionMgr::TimeoutTickCB(const nsACString &key, return PL_DHASH_NEXT; } -//----------------------------------------------------------------------------- -// nsHttpConnectionMgr::nsConnectionHandle - -nsHttpConnectionMgr::nsConnectionHandle::~nsConnectionHandle() -{ - if (mConn) { - gHttpHandler->ReclaimConnection(mConn); - NS_RELEASE(mConn); - } -} - -NS_IMPL_ISUPPORTS0(nsHttpConnectionMgr::nsConnectionHandle) - // GetOrCreateConnectionEntry finds a ent for a particular CI for use in // dispatching a transaction according to these rules // 1] use an ent that matches the ci that can be dispatched immediately @@ -2907,36 +2899,34 @@ nsHttpConnectionMgr::GetOrCreateConnectionEntry(nsHttpConnectionInfo *specificCI } nsresult -nsHttpConnectionMgr::nsConnectionHandle::OnHeadersAvailable(nsAHttpTransaction *trans, - nsHttpRequestHead *req, - nsHttpResponseHead *resp, - bool *reset) +ConnectionHandle::OnHeadersAvailable(nsAHttpTransaction *trans, + nsHttpRequestHead *req, + nsHttpResponseHead *resp, + bool *reset) { return mConn->OnHeadersAvailable(trans, req, resp, reset); } void -nsHttpConnectionMgr::nsConnectionHandle::CloseTransaction(nsAHttpTransaction *trans, nsresult reason) +ConnectionHandle::CloseTransaction(nsAHttpTransaction *trans, nsresult reason) { mConn->CloseTransaction(trans, reason); } nsresult -nsHttpConnectionMgr:: -nsConnectionHandle::TakeTransport(nsISocketTransport **aTransport, - nsIAsyncInputStream **aInputStream, - nsIAsyncOutputStream **aOutputStream) +ConnectionHandle::TakeTransport(nsISocketTransport **aTransport, + nsIAsyncInputStream **aInputStream, + nsIAsyncOutputStream **aOutputStream) { return mConn->TakeTransport(aTransport, aInputStream, aOutputStream); } void -nsHttpConnectionMgr::OnMsgSpeculativeConnect(int32_t, void *param) +nsHttpConnectionMgr::OnMsgSpeculativeConnect(int32_t, ARefBase *param) { MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread); - RefPtr args = - dont_AddRef(static_cast(param)); + SpeculativeConnectArgs *args = static_cast(param); LOG(("nsHttpConnectionMgr::OnMsgSpeculativeConnect [ci=%s]\n", args->mTrans->ConnectionInfo()->HashKey().get())); @@ -2980,25 +2970,25 @@ nsHttpConnectionMgr::OnMsgSpeculativeConnect(int32_t, void *param) } bool -nsHttpConnectionMgr::nsConnectionHandle::IsPersistent() +ConnectionHandle::IsPersistent() { return mConn->IsPersistent(); } bool -nsHttpConnectionMgr::nsConnectionHandle::IsReused() +ConnectionHandle::IsReused() { return mConn->IsReused(); } void -nsHttpConnectionMgr::nsConnectionHandle::DontReuse() +ConnectionHandle::DontReuse() { mConn->DontReuse(); } nsresult -nsHttpConnectionMgr::nsConnectionHandle::PushBack(const char *buf, uint32_t bufLen) +ConnectionHandle::PushBack(const char *buf, uint32_t bufLen) { return mConn->PushBack(buf, bufLen); } @@ -3451,10 +3441,7 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out) // otherwise just put this in the persistent connection pool LOG(("nsHalfOpenSocket::OnOutputStreamReady no transaction match " "returning conn %p to pool\n", conn.get())); - RefPtr copy(conn); - // forget() to effectively addref because onmsg*() will drop a ref - gHttpHandler->ConnMgr()->OnMsgReclaimConnection( - 0, conn.forget().take()); + gHttpHandler->ConnMgr()->OnMsgReclaimConnection(0, conn); } } @@ -3569,7 +3556,7 @@ nsHttpConnectionMgr::nsHalfOpenSocket::GetInterface(const nsIID &iid, nsHttpConnection * -nsHttpConnectionMgr::nsConnectionHandle::TakeHttpConnection() +ConnectionHandle::TakeHttpConnection() { // return our connection object to the caller and clear it internally // do not drop our reference - the caller now owns it. @@ -3581,19 +3568,19 @@ nsHttpConnectionMgr::nsConnectionHandle::TakeHttpConnection() } uint32_t -nsHttpConnectionMgr::nsConnectionHandle::CancelPipeline(nsresult reason) +ConnectionHandle::CancelPipeline(nsresult reason) { // no pipeline to cancel return 0; } nsAHttpTransaction::Classifier -nsHttpConnectionMgr::nsConnectionHandle::Classification() +ConnectionHandle::Classification() { if (mConn) return mConn->Classification(); - LOG(("nsConnectionHandle::Classification this=%p " + LOG(("ConnectionHandle::Classification this=%p " "has null mConn using CLASS_SOLO default", this)); return nsAHttpTransaction::CLASS_SOLO; } diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.h b/netwerk/protocol/http/nsHttpConnectionMgr.h index a2ba84fe52b7..ef543e5f707e 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.h +++ b/netwerk/protocol/http/nsHttpConnectionMgr.h @@ -17,6 +17,7 @@ #include "mozilla/TimeStamp.h" #include "mozilla/Attributes.h" #include "AlternateServices.h" +#include "ARefBase.h" #include "nsIObserver.h" #include "nsITimer.h" @@ -31,6 +32,10 @@ struct HttpRetParams; //----------------------------------------------------------------------------- +// message handlers have this signature +class nsHttpConnectionMgr; +typedef void (nsHttpConnectionMgr:: *nsConnEventHandler)(int32_t, ARefBase *); + class nsHttpConnectionMgr final : public nsIObserver , public AltSvcCache { @@ -389,32 +394,9 @@ private: void ResetIPFamilyPreference(); }; - // nsConnectionHandle - // - // thin wrapper around a real connection, used to keep track of references - // to the connection to determine when the connection may be reused. the - // transaction (or pipeline) owns a reference to this handle. this extra - // layer of indirection greatly simplifies consumer code, avoiding the - // need for consumer code to know when to give the connection back to the - // connection manager. - // - class nsConnectionHandle : public nsAHttpConnection - { - virtual ~nsConnectionHandle(); - - public: - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_NSAHTTPCONNECTION(mConn) - - explicit nsConnectionHandle(nsHttpConnection *conn) { NS_ADDREF(mConn = conn); } - - nsHttpConnection *mConn; - }; public: - static nsAHttpConnection *MakeConnectionHandle(nsHttpConnection *aWrapped) - { - return new nsConnectionHandle(aWrapped); - } + static nsAHttpConnection *MakeConnectionHandle(nsHttpConnection *aWrapped); + private: // nsHalfOpenSocket is used to hold the state of an opening TCP socket @@ -522,7 +504,7 @@ private: uint16_t mMaxRequestDelay; // in seconds uint16_t mMaxPipelinedRequests; uint16_t mMaxOptimisticPipelinedRequests; - bool mIsShuttingDown; + Atomic mIsShuttingDown; //------------------------------------------------------------------------- // NOTE: these members are only accessed on the socket transport thread @@ -594,72 +576,30 @@ private: const nsACString &key, nsAutoPtr &ent, void *closure); - // message handlers have this signature - typedef void (nsHttpConnectionMgr:: *nsConnEventHandler)(int32_t, void *); - - // nsConnEvent - // - // subclass of nsRunnable used to marshall events to the socket transport - // thread. this class is used to implement PostEvent. - // - class nsConnEvent; - friend class nsConnEvent; - class nsConnEvent : public nsRunnable - { - public: - nsConnEvent(nsHttpConnectionMgr *mgr, - nsConnEventHandler handler, - int32_t iparam, - void *vparam) - : mMgr(mgr) - , mHandler(handler) - , mIParam(iparam) - , mVParam(vparam) - { - NS_ADDREF(mMgr); - } - - NS_IMETHOD Run() - { - (mMgr->*mHandler)(mIParam, mVParam); - return NS_OK; - } - - private: - virtual ~nsConnEvent() - { - NS_RELEASE(mMgr); - } - - nsHttpConnectionMgr *mMgr; - nsConnEventHandler mHandler; - int32_t mIParam; - void *mVParam; - }; - + // used to marshall events to the socket transport thread. nsresult PostEvent(nsConnEventHandler handler, int32_t iparam = 0, - void *vparam = nullptr); + ARefBase *vparam = nullptr); // message handlers - void OnMsgShutdown (int32_t, void *); - void OnMsgShutdownConfirm (int32_t, void *); - void OnMsgNewTransaction (int32_t, void *); - void OnMsgReschedTransaction (int32_t, void *); - void OnMsgCancelTransaction (int32_t, void *); - void OnMsgCancelTransactions (int32_t, void *); - void OnMsgProcessPendingQ (int32_t, void *); - void OnMsgPruneDeadConnections (int32_t, void *); - void OnMsgSpeculativeConnect (int32_t, void *); - void OnMsgReclaimConnection (int32_t, void *); - void OnMsgCompleteUpgrade (int32_t, void *); - void OnMsgUpdateParam (int32_t, void *); - void OnMsgDoShiftReloadConnectionCleanup (int32_t, void *); - void OnMsgProcessFeedback (int32_t, void *); - void OnMsgProcessAllSpdyPendingQ (int32_t, void *); - void OnMsgUpdateRequestTokenBucket (int32_t, void *); - void OnMsgVerifyTraffic (int32_t, void *); - void OnMsgPruneNoTraffic (int32_t, void *); + void OnMsgShutdown (int32_t, ARefBase *); + void OnMsgShutdownConfirm (int32_t, ARefBase *); + void OnMsgNewTransaction (int32_t, ARefBase *); + void OnMsgReschedTransaction (int32_t, ARefBase *); + void OnMsgCancelTransaction (int32_t, ARefBase *); + void OnMsgCancelTransactions (int32_t, ARefBase *); + void OnMsgProcessPendingQ (int32_t, ARefBase *); + void OnMsgPruneDeadConnections (int32_t, ARefBase *); + void OnMsgSpeculativeConnect (int32_t, ARefBase *); + void OnMsgReclaimConnection (int32_t, ARefBase *); + void OnMsgCompleteUpgrade (int32_t, ARefBase *); + void OnMsgUpdateParam (int32_t, ARefBase *); + void OnMsgDoShiftReloadConnectionCleanup (int32_t, ARefBase *); + void OnMsgProcessFeedback (int32_t, ARefBase *); + void OnMsgProcessAllSpdyPendingQ (int32_t, ARefBase *); + void OnMsgUpdateRequestTokenBucket (int32_t, ARefBase *); + void OnMsgVerifyTraffic (int32_t, ARefBase *); + void OnMsgPruneNoTraffic (int32_t, ARefBase *); // Total number of active connections in all of the ConnectionEntry objects // that are accessed from mCT connection table. @@ -711,7 +651,7 @@ private: void *closure); // For diagnostics - void OnMsgPrintDiagnostics(int32_t, void *); + void OnMsgPrintDiagnostics(int32_t, ARefBase *); static PLDHashOperator PrintDiagnosticsCB(const nsACString &key, nsAutoPtr &ent, void *closure); diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h index d08577cff634..ddbbc63b02de 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -16,6 +16,7 @@ #include "TimingStruct.h" #include "Http2Push.h" #include "mozilla/net/DNS.h" +#include "ARefBase.h" #ifdef MOZ_WIDGET_GONK #include "nsINetworkInterface.h" @@ -45,6 +46,7 @@ class nsHttpTransaction final : public nsAHttpTransaction , public ATokenBucketEvent , public nsIInputStreamCallback , public nsIOutputStreamCallback + , public ARefBase { public: NS_DECL_THREADSAFE_ISUPPORTS