From e364e4019d3caa77b85dc1a97b7d22bffb2db02a Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Fri, 7 Jan 2011 11:11:22 -0500 Subject: [PATCH] bug 623921: Out of an abundance of caution back out feature: 592284 - HTTP Syn Retry r=honzab a=backout --- modules/libpref/src/init/all.js | 5 - netwerk/base/src/nsSocketTransport2.cpp | 17 +- netwerk/protocol/http/nsAHttpTransaction.h | 7 +- netwerk/protocol/http/nsHttpConnection.cpp | 329 +++--------------- netwerk/protocol/http/nsHttpConnection.h | 44 +-- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 80 ++--- netwerk/protocol/http/nsHttpConnectionMgr.h | 7 +- netwerk/protocol/http/nsHttpHandler.cpp | 7 - netwerk/protocol/http/nsHttpHandler.h | 2 - netwerk/protocol/http/nsHttpPipeline.cpp | 10 +- netwerk/protocol/http/nsHttpTransaction.cpp | 5 +- 11 files changed, 81 insertions(+), 432 deletions(-) diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 048e771eba3..a0f0c68dcdc 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -736,11 +736,6 @@ pref("network.http.prompt-temp-redirect", true); // Section 4.8 "High-Throughput Data Service Class" pref("network.http.qos", 0); -// The number of milliseconds after sending a SYN for an HTTP connection, -// to wait before trying a different connection. 0 means do not use a second -// connection. -pref("network.http.connection-retry-timeout", 250); - // default values for FTP // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594, // Section 4.8 "High-Throughput Data Service Class", and 80 (0x50, or AF22) diff --git a/netwerk/base/src/nsSocketTransport2.cpp b/netwerk/base/src/nsSocketTransport2.cpp index 43b68d9bf8d..3f95dfdde2a 100644 --- a/netwerk/base/src/nsSocketTransport2.cpp +++ b/netwerk/base/src/nsSocketTransport2.cpp @@ -1778,20 +1778,9 @@ nsSocketTransport::GetSecurityCallbacks(nsIInterfaceRequestor **callbacks) NS_IMETHODIMP nsSocketTransport::SetSecurityCallbacks(nsIInterfaceRequestor *callbacks) { - nsCOMPtr secinfo; - { - nsAutoLock lock(mLock); - mCallbacks = callbacks; - SOCKET_LOG(("Reset callbacks for secinfo=%p callbacks=%p\n", mSecInfo.get(), mCallbacks.get())); - - secinfo = mSecInfo; - } - - // don't call into PSM while holding mLock!! - nsCOMPtr secCtrl(do_QueryInterface(secinfo)); - if (secCtrl) - secCtrl->SetNotificationCallbacks(callbacks); - + nsAutoLock lock(mLock); + mCallbacks = callbacks; + // XXX should we tell PSM about this? return NS_OK; } diff --git a/netwerk/protocol/http/nsAHttpTransaction.h b/netwerk/protocol/http/nsAHttpTransaction.h index 8c11c62c88b..9ec6078a921 100644 --- a/netwerk/protocol/http/nsAHttpTransaction.h +++ b/netwerk/protocol/http/nsAHttpTransaction.h @@ -44,7 +44,6 @@ class nsAHttpConnection; class nsAHttpSegmentReader; class nsAHttpSegmentWriter; class nsIInterfaceRequestor; -class nsIEventTarget; //---------------------------------------------------------------------------- // Abstract base class for a HTTP transaction: @@ -63,8 +62,7 @@ public: // called by the connection to get security callbacks to set on the // socket transport. - virtual void GetSecurityCallbacks(nsIInterfaceRequestor **, - nsIEventTarget **) = 0; + virtual void GetSecurityCallbacks(nsIInterfaceRequestor **) = 0; // called to report socket status (see nsITransportEventSink) virtual void OnTransportStatus(nsresult status, PRUint64 progress) = 0; @@ -90,8 +88,7 @@ public: #define NS_DECL_NSAHTTPTRANSACTION \ void SetConnection(nsAHttpConnection *); \ - void GetSecurityCallbacks(nsIInterfaceRequestor **, \ - nsIEventTarget **); \ + void GetSecurityCallbacks(nsIInterfaceRequestor **); \ void OnTransportStatus(nsresult status, PRUint64 progress); \ PRBool IsDone(); \ nsresult Status(); \ diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index 11491f99f36..80f5cd84c69 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -51,7 +51,6 @@ #include "netCore.h" #include "nsNetCID.h" #include "nsAutoLock.h" -#include "nsProxyRelease.h" #include "prmem.h" #ifdef DEBUG @@ -61,17 +60,6 @@ extern PRThread *gSocketThread; static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID); - -// Statistics - only update on gSocketThread -// currently uncollected - -static PRUint32 sCreateTransport1 = 0; -static PRUint32 sCreateTransport2 = 0; -static PRUint32 sSuccessTransport1 = 0; -static PRUint32 sSuccessTransport2 = 0; -static PRUint32 sUnNecessaryTransport2 = 0; -static PRUint32 sWastedReuseCount = 0; - //----------------------------------------------------------------------------- // nsHttpConnection //----------------------------------------------------------------------------- @@ -87,7 +75,6 @@ nsHttpConnection::nsHttpConnection() , mSupportsPipelining(PR_FALSE) // assume low-grade server , mIsReused(PR_FALSE) , mCompletedSSLConnect(PR_FALSE) - , mActivationCount(0) { LOG(("Creating nsHttpConnection @%x\n", this)); @@ -99,15 +86,9 @@ nsHttpConnection::nsHttpConnection() nsHttpConnection::~nsHttpConnection() { LOG(("Destroying nsHttpConnection @%x\n", this)); - - CancelSynTimer(); - if (mBackupConnection) { - gHttpHandler->ReclaimConnection(mBackupConnection); - mBackupConnection = nsnull; - } - - ReleaseCallbacks(); + NS_IF_RELEASE(mConnInfo); + NS_IF_RELEASE(mTransaction); if (mLock) { PR_DestroyLock(mLock); @@ -119,25 +100,6 @@ nsHttpConnection::~nsHttpConnection() NS_RELEASE(handler); } -void -nsHttpConnection::ReleaseCallbacks() -{ - if (mCallbacks) { - nsIInterfaceRequestor *cbs = nsnull; - mCallbacks.swap(cbs); - NS_ProxyRelease(mCallbackTarget, cbs); - } -} - -void -nsHttpConnection::CancelSynTimer() -{ - if (mIdleSynTimer) { - mIdleSynTimer->Cancel(); - mIdleSynTimer = nsnull; - } -} - nsresult nsHttpConnection::Init(nsHttpConnectionInfo *info, PRUint16 maxHangTime) { @@ -158,59 +120,12 @@ nsHttpConnection::Init(nsHttpConnectionInfo *info, PRUint16 maxHangTime) return NS_OK; } -void -nsHttpConnection::IdleSynTimeout(nsITimer *timer, void *closure) -{ - // nsITimer is guaranteed to execute timer on same thread it - // was initialized on - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - - nsHttpConnection *self = (nsHttpConnection *)closure; - NS_ABORT_IF_FALSE(timer == self->mIdleSynTimer, "wrong timer"); - self->mIdleSynTimer = nsnull; - - if (!self->mSocketTransport) { - NS_ABORT_IF_FALSE(self->mSocketTransport1 && !self->mSocketTransport2, - "establishing backup tranport"); - - LOG(("SocketTransport hit idle timer - starting backup socket")); - - // if we have already cleared ::CloseTransaction, then we do not - // need to create the backup connection - if (!self->mTransaction) - return; - - gHttpHandler->ConnMgr()->GetConnection(self->mConnInfo, - self->mSocketCaps, - getter_AddRefs( - self->mBackupConnection)); - if (!self->mBackupConnection) - return; - nsresult rv = - self->CreateTransport(self->mSocketCaps, - getter_AddRefs(self->mSocketTransport2), - getter_AddRefs(self->mSocketIn2), - getter_AddRefs(self->mSocketOut2)); - if (NS_SUCCEEDED(rv)) { - sCreateTransport2++; - self->mTransaction-> - GetSecurityCallbacks( - getter_AddRefs(self->mCallbacks), - getter_AddRefs(self->mCallbackTarget)); - self->mSocketOut2->AsyncWait(self, 0, 0, nsnull); - } - } - - return; -} - // called on the socket thread nsresult nsHttpConnection::Activate(nsAHttpTransaction *trans, PRUint8 caps) { nsresult rv; - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); LOG(("nsHttpConnection::Activate [this=%x trans=%x caps=%x]\n", this, trans, caps)); @@ -219,47 +134,32 @@ nsHttpConnection::Activate(nsAHttpTransaction *trans, PRUint8 caps) // take ownership of the transaction mTransaction = trans; - mActivationCount++; - ReleaseCallbacks(); + NS_ADDREF(mTransaction); // set mKeepAlive according to what will be requested mKeepAliveMask = mKeepAlive = (caps & NS_HTTP_ALLOW_KEEPALIVE); + // if we don't have a socket transport then create a new one + if (!mSocketTransport) { + rv = CreateTransport(caps); + if (NS_FAILED(rv)) + goto loser; + } + // need to handle SSL proxy CONNECT if this is the first time. if (mConnInfo->UsingSSL() && mConnInfo->UsingHttpProxy() && !mCompletedSSLConnect) { rv = SetupSSLProxyConnect(); if (NS_FAILED(rv)) - goto failed_activation; + goto loser; } - // if we don't have a socket transport then create a new one - if (!mSocketTransport) { - rv = CreateTransport(caps); - } - else { - NS_ABORT_IF_FALSE(mSocketOut && mSocketIn, - "Socket Transport and SocketOut mismatch"); - - // If this is the first transaction on this connection, but - // we already have a socket that means the socket was created - // speculatively in the past, not used at that time, and - // given to the connection manager. - if (mActivationCount == 1) { - sWastedReuseCount++; - rv = mSocketTransport->SetEventSink(this, nsnull); - NS_ENSURE_SUCCESS(rv, rv); - rv = mSocketTransport->SetSecurityCallbacks(this); - NS_ENSURE_SUCCESS(rv, rv); - } - rv = mSocketOut->AsyncWait(this, 0, 0, nsnull); - } - -failed_activation: - if (NS_FAILED(rv)) { - mTransaction = nsnull; - CancelSynTimer(); - } + // wait for the output stream to be readable + rv = mSocketOut->AsyncWait(this, 0, 0, nsnull); + if (NS_SUCCEEDED(rv)) + return rv; +loser: + NS_RELEASE(mTransaction); return rv; } @@ -268,7 +168,7 @@ nsHttpConnection::Close(nsresult reason) { LOG(("nsHttpConnection::Close [this=%x reason=%x]\n", this, reason)); - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); + NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread"); if (NS_FAILED(reason)) { if (mSocketTransport) { @@ -276,18 +176,6 @@ nsHttpConnection::Close(nsresult reason) mSocketTransport->SetEventSink(nsnull, nsnull); mSocketTransport->Close(reason); } - - if (mSocketTransport1) { - mSocketTransport1->SetSecurityCallbacks(nsnull); - mSocketTransport1->SetEventSink(nsnull, nsnull); - mSocketTransport1->Close(reason); - } - - if (mSocketTransport2) { - mSocketTransport2->SetSecurityCallbacks(nsnull); - mSocketTransport2->SetEventSink(nsnull, nsnull); - mSocketTransport2->Close(reason); - } mKeepAlive = PR_FALSE; } } @@ -518,7 +406,7 @@ nsHttpConnection::OnHeadersAvailable(nsAHttpTransaction *trans, // processing a transaction pipeline until after the first HTTP/1.1 // response. nsHttpTransaction *trans = - static_cast(mTransaction.get()); + static_cast(mTransaction); trans->SetSSLConnectFailed(); } } @@ -572,51 +460,9 @@ nsHttpConnection::ResumeRecv() nsresult nsHttpConnection::CreateTransport(PRUint8 caps) { - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - NS_ABORT_IF_FALSE(!mSocketTransport, "unexpected"); - nsresult rv; - mSocketCaps = caps; - sCreateTransport1++; - - PRUint16 timeout = gHttpHandler->GetIdleSynTimeout(); - if (timeout) { - // Setup the timer that will establish a backup socket - // if we do not get a writable event on the main one. - // We do this because a lost SYN takes a very long time - // to repair at the TCP level. - // - // Failure to setup the timer is something we can live with, - // so don't return an error in that case. - - mIdleSynTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv); - if (NS_SUCCEEDED(rv)) - mIdleSynTimer->InitWithFuncCallback(IdleSynTimeout, this, - timeout, - nsITimer::TYPE_ONE_SHOT); - } - - rv = CreateTransport(mSocketCaps, - getter_AddRefs(mSocketTransport1), - getter_AddRefs(mSocketIn1), - getter_AddRefs(mSocketOut1)); - if (NS_FAILED(rv)) - return rv; - - // wait for the output stream to be readable or timeout to occur - return mSocketOut1->AsyncWait(this, 0, 0, nsnull); -} - -nsresult -nsHttpConnection::CreateTransport(PRUint8 caps, - nsISocketTransport **sock, - nsIAsyncInputStream **instream, - nsIAsyncOutputStream **outstream) -{ - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - - nsresult rv; + NS_PRECONDITION(!mSocketTransport, "unexpected"); nsCOMPtr sts = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv); @@ -668,21 +514,9 @@ nsHttpConnection::CreateTransport(PRUint8 caps, getter_AddRefs(sin)); if (NS_FAILED(rv)) return rv; - strans.forget(sock); - CallQueryInterface(sin, instream); - CallQueryInterface(sout, outstream); - - return NS_OK; -} - -nsresult -nsHttpConnection::AssignTransport(nsISocketTransport *sock, - nsIAsyncOutputStream *outs, - nsIAsyncInputStream *ins) -{ - mSocketTransport = sock; - mSocketOut = outs; - mSocketIn = ins; + mSocketTransport = strans; + mSocketIn = do_QueryInterface(sin); + mSocketOut = do_QueryInterface(sout); return NS_OK; } @@ -700,7 +534,9 @@ nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason) reason = NS_OK; mTransaction->Close(reason); - mTransaction = nsnull; + + NS_RELEASE(mTransaction); + mTransaction = 0; if (NS_FAILED(reason)) Close(reason); @@ -911,8 +747,7 @@ nsHttpConnection::SetupSSLProxyConnect() // NOTE: this cast is valid since this connection cannot be processing a // transaction pipeline until after the first HTTP/1.1 response. - nsHttpTransaction *trans = - static_cast(mTransaction.get()); + nsHttpTransaction *trans = static_cast(mTransaction); val = trans->RequestHead()->PeekHeader(nsHttp::Host); if (val) { @@ -935,85 +770,6 @@ nsHttpConnection::SetupSSLProxyConnect() return NS_NewCStringInputStream(getter_AddRefs(mSSLProxyConnectStream), buf); } -void -nsHttpConnection::ReleaseBackupTransport(nsISocketTransport *sock, - nsIAsyncOutputStream *outs, - nsIAsyncInputStream *ins) -{ - // We need to establish a small non-zero idle timeout so the connection - // mgr perceives this socket as suitable for persistent connection reuse - NS_ABORT_IF_FALSE(sock && outs && ins, "release Backup precond"); - mBackupConnection->mIdleTimeout = NS_MIN((PRUint16) 5, - gHttpHandler->IdleTimeout()); - mBackupConnection->mIsReused = PR_TRUE; - nsresult rv = mBackupConnection->AssignTransport(sock, outs, ins); - if (NS_SUCCEEDED(rv)) - rv = gHttpHandler->ReclaimConnection(mBackupConnection); - if (NS_FAILED(rv)) - NS_WARNING("Backup nsHttpConnection could not be reclaimed"); - mBackupConnection = nsnull; -} - -void -nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out) -{ - LOG(("nsHttpConnection::SelectPrimaryTransport(out=%p), mSocketOut1=%p, mSocketOut2=%p, mSocketOut=%p", - out, mSocketOut1.get(), mSocketOut2.get(), mSocketOut.get())); - - if (!mSocketOut) { - // Setup the Main Socket - - CancelSynTimer(); - - if (out == mSocketOut1) { - sSuccessTransport1++; - mSocketTransport.swap(mSocketTransport1); - mSocketOut.swap(mSocketOut1); - mSocketIn.swap(mSocketIn1); - - if (mSocketTransport2) - sUnNecessaryTransport2++; - } - else if (out == mSocketOut2) { - NS_ABORT_IF_FALSE(mSocketOut1, - "backup socket without primary being tested"); - - sSuccessTransport2++; - mSocketTransport.swap(mSocketTransport2); - mSocketOut.swap(mSocketOut2); - mSocketIn.swap(mSocketIn2); - } - else { - NS_ABORT_IF_FALSE(0, "setup on unexpected socket"); - return; - } - } - else if (out == mSocketOut1) { - // Socket2 became the primary socket but Socket1 is now valid - give it - // to the connection manager - - ReleaseBackupTransport(mSocketTransport1, - mSocketOut1, - mSocketIn1); - sSuccessTransport1++; - mSocketTransport1 = nsnull; - mSocketOut1 = nsnull; - mSocketIn1 = nsnull; - } - else if (out == mSocketOut2) { - // Socket1 became the primary socket but Socket2 is now valid - give it - // to the connectionmanager - - ReleaseBackupTransport(mSocketTransport2, - mSocketOut2, - mSocketIn2); - sSuccessTransport2++; - mSocketTransport2 = nsnull; - mSocketOut2 = nsnull; - mSocketIn2 = nsnull; - } -} - //----------------------------------------------------------------------------- // nsHttpConnection::nsISupports //----------------------------------------------------------------------------- @@ -1055,13 +811,8 @@ nsHttpConnection::OnInputStreamReady(nsIAsyncInputStream *in) NS_IMETHODIMP nsHttpConnection::OnOutputStreamReady(nsIAsyncOutputStream *out) { - NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - - NS_ABORT_IF_FALSE(out == mSocketOut || - out == mSocketOut1 || - out == mSocketOut2 , "unexpected socket"); - if (out != mSocketOut) - SelectPrimaryTransport(out); + NS_ASSERTION(out == mSocketOut, "unexpected stream"); + NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread"); // if the transaction was dropped... if (!mTransaction) { @@ -1069,12 +820,9 @@ nsHttpConnection::OnOutputStreamReady(nsIAsyncOutputStream *out) return NS_OK; } - if (mSocketOut == out) { - NS_ABORT_IF_FALSE(!mIdleSynTimer,"IdleSynTimer should not be set"); - nsresult rv = OnSocketWritable(); - if (NS_FAILED(rv)) - CloseTransaction(mTransaction, rv); - } + nsresult rv = OnSocketWritable(); + if (NS_FAILED(rv)) + CloseTransaction(mTransaction, rv); return NS_OK; } @@ -1107,12 +855,13 @@ nsHttpConnection::GetInterface(const nsIID &iid, void **result) // have to worry about the possibility of mTransaction going away // part-way through this function call. See CloseTransaction. NS_ASSERTION(PR_GetCurrentThread() != gSocketThread, "wrong thread"); - - nsCOMPtr callbacks = mCallbacks; - if (!callbacks && mTransaction) - mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks), nsnull); - if (callbacks) - return callbacks->GetInterface(iid, result); + + if (mTransaction) { + nsCOMPtr callbacks; + mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks)); + if (callbacks) + return callbacks->GetInterface(iid, result); + } return NS_ERROR_NO_INTERFACE; } diff --git a/netwerk/protocol/http/nsHttpConnection.h b/netwerk/protocol/http/nsHttpConnection.h index 156fe24eae9..a8d36425c5c 100644 --- a/netwerk/protocol/http/nsHttpConnection.h +++ b/netwerk/protocol/http/nsHttpConnection.h @@ -53,8 +53,6 @@ #include "nsIAsyncInputStream.h" #include "nsIAsyncOutputStream.h" #include "nsIInterfaceRequestor.h" -#include "nsIEventTarget.h" -#include "nsITimer.h" //----------------------------------------------------------------------------- // nsHttpConnection - represents a connection to a HTTP server (or proxy) @@ -133,14 +131,6 @@ private: nsresult ProxyStartSSL(); nsresult CreateTransport(PRUint8 caps); - nsresult CreateTransport(PRUint8 caps, - nsISocketTransport **sock, - nsIAsyncInputStream **instream, - nsIAsyncOutputStream **outstream); - nsresult AssignTransport(nsISocketTransport *sock, - nsIAsyncOutputStream *outs, - nsIAsyncInputStream *ins); - nsresult OnTransactionDone(nsresult reason); nsresult OnSocketWritable(); nsresult OnSocketReadable(); @@ -150,13 +140,6 @@ private: PRBool IsAlive(); PRBool SupportsPipelining(nsHttpResponseHead *); - static void IdleSynTimeout(nsITimer *, void *); - void SelectPrimaryTransport(nsIAsyncOutputStream *out); - void ReleaseBackupTransport(nsISocketTransport *sock, - nsIAsyncOutputStream *outs, - nsIAsyncInputStream *ins); - void CancelSynTimer(); - void ReleaseCallbacks(); private: nsCOMPtr mSocketTransport; nsCOMPtr mSocketIn; @@ -168,16 +151,7 @@ private: nsCOMPtr mSSLProxyConnectStream; nsCOMPtr mRequestStream; - // mTransaction only points to the HTTP Transaction callbacks if the - // transaction is open, otherwise it is null. - nsRefPtr mTransaction; - - // The security callbacks are only stored if we initiate a - // backup connection because they need to be proxy released - // on the main thread. - nsCOMPtr mCallbacks; - nsCOMPtr mCallbackTarget; - + nsAHttpTransaction *mTransaction; // hard ref nsHttpConnectionInfo *mConnInfo; // hard ref PRLock *mLock; @@ -191,22 +165,6 @@ private: PRPackedBool mSupportsPipelining; PRPackedBool mIsReused; PRPackedBool mCompletedSSLConnect; - - PRUint32 mActivationCount; - - // These items are used to implement a parallel connection opening - // attempt when network.http.connection-retry-timeout has expired - PRUint8 mSocketCaps; - nsCOMPtr mIdleSynTimer; - nsRefPtr mBackupConnection; - - nsCOMPtr mSocketTransport1; - nsCOMPtr mSocketIn1; - nsCOMPtr mSocketOut1; - - nsCOMPtr mSocketTransport2; - nsCOMPtr mSocketIn2; - nsCOMPtr mSocketOut2; }; #endif // nsHttpConnection_h__ diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 6aedf73c919..7a949925b43 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -400,21 +400,13 @@ nsHttpConnectionMgr::ProcessOneTransactionCB(nsHashKey *key, void *data, void *c return kHashEnumerateNext; } -// If the global number of idle connections is preventing the opening of -// new connections to a host without idle connections, then -// close them regardless of their TTL PRIntn -nsHttpConnectionMgr::PurgeExcessIdleConnectionsCB(nsHashKey *key, - void *data, void *closure) +nsHttpConnectionMgr::PurgeOneIdleConnectionCB(nsHashKey *key, void *data, void *closure) { nsHttpConnectionMgr *self = (nsHttpConnectionMgr *) closure; nsConnectionEntry *ent = (nsConnectionEntry *) data; - while (self->mNumIdleConns + self->mNumActiveConns + 1 >= self->mMaxConns) { - if (!ent->mIdleConns.Length()) { - // There are no idle conns left in this connection entry - return kHashEnumerateNext; - } + if (ent->mIdleConns.Length() > 0) { nsHttpConnection *conn = ent->mIdleConns[0]; ent->mIdleConns.RemoveElementAt(0); conn->Close(NS_ERROR_ABORT); @@ -422,8 +414,10 @@ nsHttpConnectionMgr::PurgeExcessIdleConnectionsCB(nsHashKey *key, self->mNumIdleConns--; if (0 == self->mNumIdleConns) self->StopPruneDeadConnectionsTimer(); + return kHashEnumerateStop; } - return kHashEnumerateStop; + + return kHashEnumerateNext; } PRIntn @@ -598,8 +592,8 @@ nsHttpConnectionMgr::AtActiveConnectionLimit(nsConnectionEntry *ent, PRUint8 cap LOG(("nsHttpConnectionMgr::AtActiveConnectionLimit [ci=%s caps=%x]\n", ci->HashKey().get(), caps)); - // If there are more active connections than the global limit, then we're - // done. Purging idle connections won't get us below it. + // If we have more active connections than the limit, then we're done -- + // purging idle connections won't get us below it. if (mNumActiveConns >= mMaxConns) { LOG((" num active conns == max conns\n")); return PR_TRUE; @@ -636,18 +630,6 @@ nsHttpConnectionMgr::AtActiveConnectionLimit(nsConnectionEntry *ent, PRUint8 cap (persistCount >= maxPersistConns) ); } -void -nsHttpConnectionMgr::GetConnection(nsHttpConnectionInfo *ci, - PRUint8 caps, - nsHttpConnection **result) -{ - NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread"); - nsCStringKey key(ci->HashKey()); - nsConnectionEntry *ent = (nsConnectionEntry *) mCT.Get(&key); - if (!ent) return; - GetConnection(ent, caps, result); -} - void nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps, nsHttpConnection **result) @@ -655,19 +637,12 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps, LOG(("nsHttpConnectionMgr::GetConnection [ci=%s caps=%x]\n", ent->mConnInfo->HashKey().get(), PRUint32(caps))); - // First, see if an idle persistent connection may be reused instead of - // establishing a new socket. We do not need to check the connection limits - // yet as they govern the maximum number of open connections and reusing - // an old connection never increases that. - *result = nsnull; nsHttpConnection *conn = nsnull; if (caps & NS_HTTP_ALLOW_KEEPALIVE) { - // search the idle connection list. Each element in the list - // has a reference, so if we remove it from the list into a local - // ptr, that ptr now owns the reference + // search the idle connection list while (!conn && (ent->mIdleConns.Length() > 0)) { conn = ent->mIdleConns[0]; // we check if the connection can be reused before even checking if @@ -697,7 +672,7 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps, // XXX this just purges a random idle connection. we should instead // enumerate the entire hash table to find the eldest idle connection. if (mNumIdleConns && mNumIdleConns + mNumActiveConns + 1 >= mMaxConns) - mCT.Enumerate(PurgeExcessIdleConnectionsCB, this); + mCT.Enumerate(PurgeOneIdleConnectionCB, this); // Need to make a new TCP connection. First, we check if we've hit // either the maximum connection limit globally or for this particular @@ -708,7 +683,10 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps, } conn = new nsHttpConnection(); + if (!conn) + return; NS_ADDREF(conn); + nsresult rv = conn->Init(ent->mConnInfo, mMaxRequestDelay); if (NS_FAILED(rv)) { NS_RELEASE(conn); @@ -716,11 +694,6 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps, } } - // hold an owning ref to this connection - ent->mActiveConns.AppendElement(conn); - mNumActiveConns++; - NS_ADDREF(conn); - *result = conn; } @@ -745,6 +718,11 @@ nsHttpConnectionMgr::DispatchTransaction(nsConnectionEntry *ent, trans = pipeline; } + // hold an owning ref to this connection + ent->mActiveConns.AppendElement(conn); + mNumActiveConns++; + NS_ADDREF(conn); + // give the transaction the indirect reference to the connection. trans->SetConnection(handle); @@ -859,6 +837,15 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans) // destroy connection handle. trans->SetConnection(nsnull); + + // remove sticky connection from active connection list; we'll add it + // right back in DispatchTransaction. + if (ent->mActiveConns.RemoveElement(conn)) + mNumActiveConns--; + else { + NS_ERROR("sticky connection not found in active list"); + return NS_ERROR_UNEXPECTED; + } } else GetConnection(ent, caps, &conn); @@ -1011,22 +998,13 @@ nsHttpConnectionMgr::OnMsgReclaimConnection(PRInt32, void *param) NS_ASSERTION(ent, "no connection entry"); if (ent) { - // If the connection is in the active list, remove that entry - // and the reference held by the mActiveConns list. - // This is never the final reference on conn as the event context - // is also holding one that is released at the end of this function. - if (ent->mActiveConns.RemoveElement(conn)) { - nsHttpConnection *temp = conn; - NS_RELEASE(temp); - mNumActiveConns--; - } - + ent->mActiveConns.RemoveElement(conn); + mNumActiveConns--; if (conn->CanReuse()) { LOG((" adding connection to idle list\n")); // hold onto this connection in the idle list. we push it to // the end of the list so as to ensure that we'll visit older // connections first before getting to this one. - NS_ADDREF(conn); ent->mIdleConns.AppendElement(conn); mNumIdleConns++; // If the added connection was first idle connection or has shortest @@ -1040,6 +1018,8 @@ nsHttpConnectionMgr::OnMsgReclaimConnection(PRInt32, void *param) LOG((" connection cannot be reused; closing connection\n")); // make sure the connection is closed and release our reference. conn->Close(NS_ERROR_ABORT); + nsHttpConnection *temp = conn; + NS_RELEASE(temp); } } diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.h b/netwerk/protocol/http/nsHttpConnectionMgr.h index 67027fc3cce..569ed8d7efe 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.h +++ b/netwerk/protocol/http/nsHttpConnectionMgr.h @@ -137,9 +137,6 @@ public: // preference to the specified connection. nsresult ProcessPendingQ(nsHttpConnectionInfo *); - // called to reserve a nshttpconnection object with the manager - void GetConnection(nsHttpConnectionInfo *, PRUint8 caps, - nsHttpConnection **); private: virtual ~nsHttpConnectionMgr(); @@ -209,10 +206,10 @@ private: //------------------------------------------------------------------------- static PRIntn ProcessOneTransactionCB(nsHashKey *, void *, void *); - + static PRIntn PurgeOneIdleConnectionCB(nsHashKey *, void *, void *); static PRIntn PruneDeadConnectionsCB(nsHashKey *, void *, void *); static PRIntn ShutdownPassCB(nsHashKey *, void *, void *); - static PRIntn PurgeExcessIdleConnectionsCB(nsHashKey *, void *, void *); + PRBool ProcessPendingQForEntry(nsConnectionEntry *); PRBool AtActiveConnectionLimit(nsConnectionEntry *, PRUint8 caps); void GetConnection(nsConnectionEntry *, PRUint8 caps, nsHttpConnection **); diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 9669cbe99ce..71d7cdd304e 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -178,7 +178,6 @@ nsHttpHandler::nsHttpHandler() , mIdleTimeout(10) , mMaxRequestAttempts(10) , mMaxRequestDelay(10) - , mIdleSynTimeout(250) , mMaxConnections(24) , mMaxConnectionsPerServer(8) , mMaxPersistentConnectionsPerServer(2) @@ -919,12 +918,6 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref) mRedirectionLimit = (PRUint8) NS_CLAMP(val, 0, 0xff); } - if (PREF_CHANGED(HTTP_PREF("connection-retry-timeout"))) { - rv = prefs->GetIntPref(HTTP_PREF("connection-retry-timeout"), &val); - if (NS_SUCCEEDED(rv)) - mIdleSynTimeout = (PRUint16) NS_CLAMP(val, 0, 3000); - } - if (PREF_CHANGED(HTTP_PREF("version"))) { nsXPIDLCString httpVersion; prefs->GetCharPref(HTTP_PREF("version"), getter_Copies(httpVersion)); diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 3ad8503633c..af84c9fb599 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -107,7 +107,6 @@ public: nsIIDNService *IDNConverter() { return mIDNConverter; } PRUint32 PhishyUserPassLength() { return mPhishyUserPassLength; } PRUint8 GetQoSBits() { return mQoSBits; } - PRUint16 GetIdleSynTimeout() { return mIdleSynTimeout; } PRBool IsPersistentHttpsCachingEnabled() { return mEnablePersistentHttpsCaching; } @@ -264,7 +263,6 @@ private: PRUint16 mIdleTimeout; PRUint16 mMaxRequestAttempts; PRUint16 mMaxRequestDelay; - PRUint16 mIdleSynTimeout; PRUint16 mMaxConnections; PRUint8 mMaxConnectionsPerServer; diff --git a/netwerk/protocol/http/nsHttpPipeline.cpp b/netwerk/protocol/http/nsHttpPipeline.cpp index f1b0c7e2cfd..98b6f0018d1 100644 --- a/netwerk/protocol/http/nsHttpPipeline.cpp +++ b/netwerk/protocol/http/nsHttpPipeline.cpp @@ -310,20 +310,16 @@ nsHttpPipeline::SetConnection(nsAHttpConnection *conn) } void -nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result, - nsIEventTarget **target) +nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result) { NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread"); // return security callbacks from first request nsAHttpTransaction *trans = Request(0); if (trans) - trans->GetSecurityCallbacks(result, target); - else { + trans->GetSecurityCallbacks(result); + else *result = nsnull; - if (target) - *target = nsnull; - } } void diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index 0052d8a421a..c0289a9d9a8 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -328,12 +328,9 @@ nsHttpTransaction::SetConnection(nsAHttpConnection *conn) } void -nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb, - nsIEventTarget **target) +nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb) { NS_IF_ADDREF(*cb = mCallbacks); - if (target) - NS_IF_ADDREF(*target = mConsumerTarget); } void