diff --git a/netwerk/protocol/http/src/nsAHttpConnection.h b/netwerk/protocol/http/src/nsAHttpConnection.h index 4c6b26dac80..6cfaeab7b7f 100644 --- a/netwerk/protocol/http/src/nsAHttpConnection.h +++ b/netwerk/protocol/http/src/nsAHttpConnection.h @@ -95,13 +95,12 @@ public: // called by a transaction to get the security info from the socket. virtual void GetSecurityInfo(nsISupports **) = 0; - // called by a transaction to remove itself from the connection (eg. when - // it reads premature EOF and must restart itself). - //virtual void DropTransaction(nsAHttpTransaction *) = 0; - // called by a transaction to determine whether or not the connection is // persistent... important in determining the end of a response. virtual PRBool IsPersistent() = 0; + + // called to determine if a connection has been reused. + virtual PRBool IsReused() = 0; // called by a transaction when the transaction reads more from the socket // than it should have (eg. containing part of the next pipelined response). diff --git a/netwerk/protocol/http/src/nsHttpChannel.cpp b/netwerk/protocol/http/src/nsHttpChannel.cpp index 4dd077370da..6febfb629e9 100644 --- a/netwerk/protocol/http/src/nsHttpChannel.cpp +++ b/netwerk/protocol/http/src/nsHttpChannel.cpp @@ -63,7 +63,7 @@ nsHttpChannel::nsHttpChannel() , mLoadFlags(LOAD_NORMAL) , mStatus(NS_OK) , mLogicalOffset(0) - , mCapabilities(0) + , mCaps(0) , mCachedResponseHead(nsnull) , mCacheAccess(0) , mPostID(0) @@ -121,7 +121,7 @@ nsHttpChannel::Init(nsIURI *uri, mURI = uri; mOriginalURI = uri; mDocumentURI = nsnull; - mCapabilities = caps; + mCaps = caps; // // Construct connection info object @@ -416,25 +416,29 @@ nsHttpChannel::HandleAsyncNotModified() nsresult nsHttpChannel::SetupTransaction() { + LOG(("nsHttpChannel::SetupTransaction [this=%x]\n", this)); + NS_ENSURE_TRUE(!mTransaction, NS_ERROR_ALREADY_INITIALIZED); nsresult rv; - // figure out if we should disallow pipelining. disallow if - // the method is not GET or HEAD. we also disallow pipelining - // if this channel corresponds to a toplevel document. - // XXX does the toplevel document check really belong here? - // XXX or, should we push it out entirely to necko consumers? - PRUint8 caps = mCapabilities; - if (!mAllowPipelining || (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) || - !(mRequestHead.Method() == nsHttp::Get || - mRequestHead.Method() == nsHttp::Head)) { - LOG(("nsHttpChannel::SetupTransaction [this=%x] pipelining disallowed\n", this)); - caps &= ~NS_HTTP_ALLOW_PIPELINING; + if (mCaps & NS_HTTP_ALLOW_PIPELINING) { + // + // disable pipelining if: + // (1) pipelining has been explicitly disabled + // (2) request corresponds to a top-level document load (link click) + // (3) request method is non-idempotent + // + // XXX does the toplevel document check really belong here? or, should + // we push it out entirely to necko consumers? + // + if (!mAllowPipelining || (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) || + !(mRequestHead.Method() == nsHttp::Get || + mRequestHead.Method() == nsHttp::Head)) { + LOG((" pipelining disallowed\n")); + mCaps &= ~NS_HTTP_ALLOW_PIPELINING; + } } - - if (mLoadFlags & nsIRequest::LOAD_BACKGROUND) - caps |= NS_HTTP_DONT_REPORT_PROGRESS; // use the URI path if not proxying (transparent proxying such as SSL proxy // does not count here). also, figure out what version we should be speaking. @@ -516,10 +520,10 @@ nsHttpChannel::SetupTransaction() NS_ADDREF(mTransaction); nsCOMPtr responseStream; - rv = mTransaction->Init(caps, mConnectionInfo, &mRequestHead, - mUploadStream, mUploadStreamHasHeaders, - mEventQ, mCallbacks, this, - getter_AddRefs(responseStream)); + rv = mTransaction->Init(mCaps, mConnectionInfo, &mRequestHead, + mUploadStream, mUploadStreamHasHeaders, + mEventQ, mCallbacks, this, + getter_AddRefs(responseStream)); if (NS_FAILED(rv)) return rv; rv = NS_NewInputStreamPump(getter_AddRefs(mTransactionPump), diff --git a/netwerk/protocol/http/src/nsHttpChannel.h b/netwerk/protocol/http/src/nsHttpChannel.h index 5b284dc9c85..dcc46f90e58 100644 --- a/netwerk/protocol/http/src/nsHttpChannel.h +++ b/netwerk/protocol/http/src/nsHttpChannel.h @@ -180,7 +180,7 @@ private: PRUint32 mLoadFlags; PRUint32 mStatus; PRUint32 mLogicalOffset; - PRUint8 mCapabilities; + PRUint8 mCaps; // cache specific data nsCOMPtr mCacheEntry; diff --git a/netwerk/protocol/http/src/nsHttpConnection.cpp b/netwerk/protocol/http/src/nsHttpConnection.cpp index 2343cdd3686..b25e396e5a0 100644 --- a/netwerk/protocol/http/src/nsHttpConnection.cpp +++ b/netwerk/protocol/http/src/nsHttpConnection.cpp @@ -59,9 +59,8 @@ nsHttpConnection::nsHttpConnection() , mTransactionDone(PR_TRUE) , mKeepAlive(PR_TRUE) // assume to keep-alive by default , mKeepAliveMask(PR_TRUE) - //, mWriteDone(0) - //, mReadDone(0) , mSupportsPipelining(PR_FALSE) // assume low-grade server + , mIsReused(PR_FALSE) { LOG(("Creating nsHttpConnection @%x\n", this)); @@ -204,6 +203,14 @@ nsHttpConnection::IsAlive() if (NS_FAILED(rv)) alive = PR_FALSE; +//#define TEST_RESTART_LOGIC +#ifdef TEST_RESTART_LOGIC + if (!alive) { + LOG(("pretending socket is still alive to test restart logic\n")); + alive = PR_TRUE; + } +#endif + return alive; } @@ -455,6 +462,10 @@ nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason) if (NS_FAILED(reason)) Close(reason); + // flag the connection as reused here for convenience sake. certainly + // it might be going away instead ;-) + mIsReused = PR_TRUE; + gHttpHandler->ReclaimConnection(this); } diff --git a/netwerk/protocol/http/src/nsHttpConnection.h b/netwerk/protocol/http/src/nsHttpConnection.h index 637ccc32cd9..79ad4995300 100644 --- a/netwerk/protocol/http/src/nsHttpConnection.h +++ b/netwerk/protocol/http/src/nsHttpConnection.h @@ -98,6 +98,7 @@ public: void GetConnectionInfo(nsHttpConnectionInfo **ci) { NS_IF_ADDREF(*ci = mConnInfo); } void GetSecurityInfo(nsISupports **); PRBool IsPersistent() { return IsKeepAlive(); } + PRBool IsReused() { return mIsReused; } nsresult PushBack(const char *data, PRUint32 length) { NS_NOTREACHED("PushBack"); return NS_ERROR_UNEXPECTED; } nsresult ResumeSend(); nsresult ResumeRecv(); @@ -152,6 +153,7 @@ private: PRPackedBool mKeepAlive; PRPackedBool mKeepAliveMask; PRPackedBool mSupportsPipelining; + PRPackedBool mIsReused; }; #endif // nsHttpConnection_h__ diff --git a/netwerk/protocol/http/src/nsHttpHandler.cpp b/netwerk/protocol/http/src/nsHttpHandler.cpp index 9aaa9ac09c6..4f2920cce8d 100644 --- a/netwerk/protocol/http/src/nsHttpHandler.cpp +++ b/netwerk/protocol/http/src/nsHttpHandler.cpp @@ -264,7 +264,7 @@ nsHttpHandler::StartPruneDeadConnectionsTimer() mTimer = do_CreateInstance("@mozilla.org/timer;1"); NS_ASSERTION(mTimer, "no timer"); // failure to create a timer is not a fatal error, but idle connections - // may not be cleaned up as aggressively. + // will not be cleaned up until we try to use them. if (mTimer) mTimer->Init(this, 15*1000, // every 15 seconds nsITimer::TYPE_REPEATING_SLACK); diff --git a/netwerk/protocol/http/src/nsHttpPipeline.cpp b/netwerk/protocol/http/src/nsHttpPipeline.cpp index 0ee06c11de0..0d151f453e5 100644 --- a/netwerk/protocol/http/src/nsHttpPipeline.cpp +++ b/netwerk/protocol/http/src/nsHttpPipeline.cpp @@ -218,12 +218,6 @@ nsHttpPipeline::GetSecurityInfo(nsISupports **result) mConnection->GetSecurityInfo(result); } -PRBool -nsHttpPipeline::IsPersistent() -{ - return PR_TRUE; // pipelining requires this to be true! -} - nsresult nsHttpPipeline::PushBack(const char *data, PRUint32 length) { diff --git a/netwerk/protocol/http/src/nsHttpPipeline.h b/netwerk/protocol/http/src/nsHttpPipeline.h index 62ef075888c..3f1b438fe8d 100644 --- a/netwerk/protocol/http/src/nsHttpPipeline.h +++ b/netwerk/protocol/http/src/nsHttpPipeline.h @@ -51,7 +51,8 @@ public: void CloseTransaction(nsAHttpTransaction *, nsresult); void GetConnectionInfo(nsHttpConnectionInfo **); void GetSecurityInfo(nsISupports **); - PRBool IsPersistent(); + PRBool IsPersistent() { return PR_TRUE; } // pipelining requires this + PRBool IsReused() { return PR_TRUE; } // pipelining requires this nsresult PushBack(const char *, PRUint32); // nsAHttpTransaction methods: diff --git a/netwerk/protocol/http/src/nsHttpTransaction.cpp b/netwerk/protocol/http/src/nsHttpTransaction.cpp index 1f054f5b93e..9d574198681 100644 --- a/netwerk/protocol/http/src/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/src/nsHttpTransaction.cpp @@ -121,7 +121,7 @@ nsHttpTransaction::nsHttpTransaction() , mResponseIsComplete(PR_FALSE) , mDidContentStart(PR_FALSE) , mNoContent(PR_FALSE) - , mPrematureEOF(PR_FALSE) + , mReceivedData(PR_FALSE) , mDestroying(PR_FALSE) { LOG(("Creating nsHttpTransaction @%x\n", this)); @@ -359,6 +359,7 @@ nsHttpTransaction::WritePipeSegment(nsIOutputStream *stream, if (NS_FAILED(rv)) return rv; // caller didn't want to write anything NS_ASSERTION(*countWritten > 0, "bad writer"); + trans->mReceivedData = PR_TRUE; // now let the transaction "play" with the buffer. it is free to modify // the contents of the buffer and/or modify countWritten. @@ -403,21 +404,26 @@ nsHttpTransaction::Close(nsresult reason) return; } - // we must no longer reference the connection! - NS_IF_RELEASE(mConnection); + // we must no longer reference the connection! find out if the + // connection was being reused before letting it go. + PRBool connReused = PR_FALSE; + if (mConnection) { + connReused = mConnection->IsReused(); + NS_RELEASE(mConnection); + } mConnected = PR_FALSE; - // if the connection was reset before we read any part of the response, - // then we must try to restart the transaction. - if (reason == NS_ERROR_NET_RESET) { - // if some data was read, then mask the reset error, so our listener - // will treat this as a normal failure. XXX we might want to map - // this error to a special error code to indicate that the transfer - // was abnormally interrupted. - if (mContentRead > 0) - reason = NS_ERROR_ABORT; // XXX NS_ERROR_NET_INTERRUPT?? - // if restarting fails, then we must notify our listener. - else if (NS_SUCCEEDED(Restart())) + // + // if the connection was reset or closed before we read any part of the + // response, and if the connection was being reused, then we can assume + // that we wrote to a stale connection and we must therefore repeat the + // request over a new connection. + // + if (!mReceivedData && connReused && (reason == NS_ERROR_NET_RESET || + reason == NS_OK)) { + // if restarting fails, then we must proceed to close the pipe, + // which will notify the channel that the transaction failed. + if (NS_SUCCEEDED(Restart())) return; } diff --git a/netwerk/protocol/http/src/nsHttpTransaction.h b/netwerk/protocol/http/src/nsHttpTransaction.h index 6a78ca6fecf..5593c7bd80d 100644 --- a/netwerk/protocol/http/src/nsHttpTransaction.h +++ b/netwerk/protocol/http/src/nsHttpTransaction.h @@ -203,7 +203,7 @@ private: PRPackedBool mResponseIsComplete; // == mTransactionDone && NS_SUCCEEDED(mStatus) ? PRPackedBool mDidContentStart; PRPackedBool mNoContent; // expecting an empty entity body? - PRPackedBool mPrematureEOF; + PRPackedBool mReceivedData; PRPackedBool mDestroying; };