Fixes bug 82241 "assertion & crash from pthreads, multiple monitor entry
[crash @nsHttpConnection::ActivateConnection]" + several reported hangs. r=bbaetz, sr=waterson, a=asa. git-svn-id: svn://10.0.0.236/trunk@97518 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
darin%netscape.com
parent
c8d9954774
commit
d3e456c7a7
@ -33,6 +33,7 @@
|
||||
#include "nsIStringStream.h"
|
||||
#include "netCore.h"
|
||||
#include "nsNetCID.h"
|
||||
#include "nsAutoLock.h"
|
||||
#include "prmem.h"
|
||||
#include "plevent.h"
|
||||
|
||||
@ -45,11 +46,14 @@ static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
|
||||
nsHttpConnection::nsHttpConnection()
|
||||
: mTransaction(0)
|
||||
, mConnectionInfo(0)
|
||||
, mLock(nsnull)
|
||||
, mReuseCount(0)
|
||||
, mMaxReuseCount(0)
|
||||
, mIdleTimeout(0)
|
||||
, mLastActiveTime(0)
|
||||
, mKeepAlive(0)
|
||||
, mWriteDone(0)
|
||||
, mReadDone(0)
|
||||
{
|
||||
LOG(("Creating nsHttpConnection @%x\n", this));
|
||||
|
||||
@ -62,6 +66,11 @@ nsHttpConnection::~nsHttpConnection()
|
||||
|
||||
NS_IF_RELEASE(mConnectionInfo);
|
||||
NS_IF_RELEASE(mTransaction);
|
||||
|
||||
if (mLock) {
|
||||
PR_DestroyLock(mLock);
|
||||
mLock = nsnull;
|
||||
}
|
||||
}
|
||||
|
||||
nsresult
|
||||
@ -72,6 +81,10 @@ nsHttpConnection::Init(nsHttpConnectionInfo *info)
|
||||
NS_ENSURE_ARG_POINTER(info);
|
||||
NS_ENSURE_TRUE(!mConnectionInfo, NS_ERROR_ALREADY_INITIALIZED);
|
||||
|
||||
mLock = PR_NewLock();
|
||||
if (!mLock)
|
||||
return NS_ERROR_OUT_OF_MEMORY;
|
||||
|
||||
mConnectionInfo = info;
|
||||
NS_ADDREF(mConnectionInfo);
|
||||
|
||||
@ -107,10 +120,7 @@ nsHttpConnection::SetTransaction(nsHttpTransaction *transaction)
|
||||
getter_AddRefs(mProgressSink));
|
||||
}
|
||||
}
|
||||
|
||||
// assign ourselves to the transaction
|
||||
mTransaction->SetConnection(this);
|
||||
|
||||
|
||||
return ActivateConnection();
|
||||
}
|
||||
|
||||
@ -188,33 +198,29 @@ nsHttpConnection::OnHeadersAvailable(nsHttpTransaction *trans, PRBool *reset)
|
||||
|
||||
// called from any thread
|
||||
nsresult
|
||||
nsHttpConnection::OnTransactionComplete(nsresult status)
|
||||
nsHttpConnection::OnTransactionComplete(nsHttpTransaction *trans, nsresult status)
|
||||
{
|
||||
LOG(("nsHttpConnection::OnTransactionComplete [this=%x status=%x]\n",
|
||||
this, status));
|
||||
LOG(("nsHttpConnection::OnTransactionComplete [this=%x trans=%x status=%x]\n",
|
||||
this, trans, status));
|
||||
|
||||
NS_ENSURE_TRUE(mSocketTransport, NS_ERROR_UNEXPECTED);
|
||||
// trans may not be mTransaction
|
||||
if (trans != mTransaction)
|
||||
return NS_OK;
|
||||
|
||||
// be warned: trans may not be mTransaction
|
||||
nsCOMPtr<nsIRequest> writeReq, readReq;
|
||||
|
||||
// clear the read/write requests atomically.
|
||||
{
|
||||
nsAutoLock lock(mLock);
|
||||
writeReq = mWriteRequest;
|
||||
readReq = mReadRequest;
|
||||
}
|
||||
|
||||
// cancel the requests... this will cause OnStopRequest to be fired
|
||||
if (mWriteRequest) {
|
||||
mWriteRequest->Cancel(status);
|
||||
mWriteRequest = 0;
|
||||
}
|
||||
if (mReadRequest) {
|
||||
mReadRequest->Cancel(status);
|
||||
mReadRequest = 0;
|
||||
}
|
||||
|
||||
// break the cycle between the socket transport and this
|
||||
if (mSocketTransport)
|
||||
mSocketTransport->SetNotificationCallbacks(nsnull, 0);
|
||||
|
||||
if (!mKeepAlive || NS_FAILED(status)) {
|
||||
// if we're not going to be keeping this connection alive...
|
||||
mSocketTransport->SetReuseConnection(PR_FALSE);
|
||||
}
|
||||
if (writeReq)
|
||||
writeReq->Cancel(status);
|
||||
if (readReq)
|
||||
readReq->Cancel(status);
|
||||
|
||||
return NS_OK;
|
||||
}
|
||||
@ -319,23 +325,65 @@ nsHttpConnection::ActivateConnection()
|
||||
SetNotificationCallbacks(this, nsITransport::DONT_PROXY_PROGRESS);
|
||||
if (NS_FAILED(rv)) return rv;
|
||||
|
||||
// note: we pass a reference to ourselves as the context so we can
|
||||
// differentiate the OnStopRequest events.
|
||||
rv = mSocketTransport->AsyncWrite(this, (nsIStreamListener*) this,
|
||||
0, PRUint32(-1),
|
||||
nsITransport::DONT_PROXY_OBSERVER |
|
||||
nsITransport::DONT_PROXY_PROVIDER,
|
||||
getter_AddRefs(mWriteRequest));
|
||||
if (NS_FAILED(rv)) return rv;
|
||||
nsCOMPtr<nsIRequest> writeReq, readReq;
|
||||
PRBool mustCancelWrite = PR_FALSE;
|
||||
PRBool mustCancelRead = PR_FALSE;
|
||||
|
||||
mWriteDone = PR_FALSE;
|
||||
mReadDone = PR_FALSE;
|
||||
|
||||
// because AsyncRead can result in listener callbacks on the socket
|
||||
// transport thread before it even runs, we have to addref this in
|
||||
// order to ensure that we stay around long enough to complete this
|
||||
// call.
|
||||
NS_ADDREF_THIS();
|
||||
|
||||
// fire off the read first so that we'll often detect premature EOF before
|
||||
// writing to the socket, though this is not necessary.
|
||||
rv = mSocketTransport->AsyncRead(this, nsnull,
|
||||
0, PRUint32(-1),
|
||||
nsITransport::DONT_PROXY_OBSERVER |
|
||||
nsITransport::DONT_PROXY_LISTENER,
|
||||
getter_AddRefs(mReadRequest));
|
||||
if (NS_FAILED(rv)) return rv;
|
||||
getter_AddRefs(readReq));
|
||||
if (NS_FAILED(rv)) goto end;
|
||||
|
||||
return NS_OK;
|
||||
// we pass a reference to ourselves as the context so we can
|
||||
// differentiate the stream listener events.
|
||||
rv = mSocketTransport->AsyncWrite(this, (nsIStreamListener*) this,
|
||||
0, PRUint32(-1),
|
||||
nsITransport::DONT_PROXY_OBSERVER |
|
||||
nsITransport::DONT_PROXY_PROVIDER,
|
||||
getter_AddRefs(writeReq));
|
||||
if (NS_FAILED(rv)) goto end;
|
||||
|
||||
// grab pointers to the read/write requests provided they have not
|
||||
// already finished. check for early cancelation.
|
||||
{
|
||||
nsAutoLock lock(mLock);
|
||||
|
||||
if (!mWriteDone) {
|
||||
mWriteRequest = writeReq;
|
||||
if (mTransaction->IsDone())
|
||||
mustCancelWrite = PR_TRUE;
|
||||
}
|
||||
if (!mReadDone) {
|
||||
mReadRequest = readReq;
|
||||
if (mTransaction->IsDone())
|
||||
mustCancelRead = PR_TRUE;
|
||||
}
|
||||
}
|
||||
|
||||
// handle the case of an overlapped cancelation (ie. the transaction
|
||||
// could have been canceled prior to mReadRequest/mWriteRequest being
|
||||
// assigned).
|
||||
if (mustCancelWrite)
|
||||
writeReq->Cancel(mTransaction->Status());
|
||||
if (mustCancelRead)
|
||||
readReq->Cancel(mTransaction->Status());
|
||||
|
||||
end:
|
||||
NS_RELEASE_THIS();
|
||||
return rv;
|
||||
}
|
||||
|
||||
nsresult
|
||||
@ -472,18 +520,22 @@ nsHttpConnection::OnStopRequest(nsIRequest *request, nsISupports *ctxt,
|
||||
LOG(("nsHttpConnection::OnStopRequest [this=%x ctxt=%x status=%x]\n",
|
||||
this, ctxt, status));
|
||||
|
||||
if (!mTransaction)
|
||||
return NS_OK;
|
||||
|
||||
if (ctxt == (nsISupports *) (nsIStreamListener *) this)
|
||||
if (ctxt == (nsISupports *) (nsIStreamListener *) this) {
|
||||
// Done writing, so mark the write request as complete.
|
||||
nsAutoLock lock(mLock);
|
||||
mWriteDone = PR_TRUE;
|
||||
mWriteRequest = 0;
|
||||
}
|
||||
else {
|
||||
// Done reading, so signal transaction complete...
|
||||
mReadRequest = 0;
|
||||
// Done reading, so mark the read request as complete.
|
||||
{
|
||||
nsAutoLock lock(mLock);
|
||||
mReadDone = PR_TRUE;
|
||||
mReadRequest = 0;
|
||||
}
|
||||
|
||||
// break the cycle between the socket transport and this
|
||||
if (mSocketTransport)
|
||||
mSocketTransport->SetNotificationCallbacks(nsnull, 0);
|
||||
mSocketTransport->SetNotificationCallbacks(nsnull, 0);
|
||||
|
||||
// don't need this anymore
|
||||
mProgressSink = 0;
|
||||
@ -536,7 +588,7 @@ nsHttpConnection::OnDataWritable(nsIRequest *request, nsISupports *context,
|
||||
return outputStream->WriteFrom(mSSLProxyConnectStream, n, &n);
|
||||
}
|
||||
|
||||
LOG(("done writing proxy connect stream\n"));
|
||||
LOG(("done writing proxy connect stream, waiting for response.\n"));
|
||||
return NS_BASE_STREAM_WOULD_BLOCK;
|
||||
}
|
||||
|
||||
|
||||
@ -37,6 +37,7 @@
|
||||
#include "plstr.h"
|
||||
#include "prclist.h"
|
||||
#include "nsCRT.h"
|
||||
#include "prlock.h"
|
||||
|
||||
class nsHttpHandler;
|
||||
class nsHttpConnectionInfo;
|
||||
@ -73,7 +74,7 @@ public:
|
||||
nsresult OnHeadersAvailable(nsHttpTransaction *, PRBool *reset);
|
||||
|
||||
// called by the transaction to inform the connection that it is done.
|
||||
nsresult OnTransactionComplete(nsresult status);
|
||||
nsresult OnTransactionComplete(nsHttpTransaction *, nsresult status);
|
||||
|
||||
// called by the transaction to resume a read-in-progress
|
||||
nsresult Resume();
|
||||
@ -101,14 +102,6 @@ public:
|
||||
nsIEventQueue *ConsumerEventQ() { return mConsumerEventQ; }
|
||||
|
||||
private:
|
||||
enum {
|
||||
IDLE,
|
||||
WAITING_FOR_WRITE,
|
||||
WRITING,
|
||||
WAITING_FOR_READ,
|
||||
READING
|
||||
};
|
||||
|
||||
nsresult ActivateConnection();
|
||||
nsresult CreateTransport();
|
||||
|
||||
@ -130,12 +123,16 @@ private:
|
||||
nsHttpTransaction *mTransaction; // hard ref
|
||||
nsHttpConnectionInfo *mConnectionInfo; // hard ref
|
||||
|
||||
PRLock *mLock;
|
||||
|
||||
PRUint32 mReuseCount;
|
||||
PRUint32 mMaxReuseCount; // value of keep-alive: max=
|
||||
PRUint32 mIdleTimeout; // value of keep-alive: timeout=
|
||||
PRUint32 mLastActiveTime;
|
||||
|
||||
PRPackedBool mKeepAlive;
|
||||
PRPackedBool mWriteDone;
|
||||
PRPackedBool mReadDone;
|
||||
};
|
||||
|
||||
//-----------------------------------------------------------------------------
|
||||
|
||||
@ -375,17 +375,17 @@ nsHttpHandler::ReclaimConnection(nsHttpConnection *conn)
|
||||
{
|
||||
NS_ENSURE_ARG_POINTER(conn);
|
||||
|
||||
PRBool reusable = conn->CanReuse();
|
||||
|
||||
LOG(("nsHttpHandler::ReclaimConnection [conn=%x keep-alive=%d]\n",
|
||||
conn, conn->CanReuse()));
|
||||
conn, reusable));
|
||||
|
||||
nsAutoLock lock(mConnectionLock);
|
||||
|
||||
// remove connection from the active connection list
|
||||
mActiveConnections.RemoveElement(conn);
|
||||
|
||||
LOG(("active connection count = %u\n", mActiveConnections.Count()));
|
||||
|
||||
if (conn->CanReuse()) {
|
||||
if (reusable) {
|
||||
// verify that we aren't already maxed out on the number of
|
||||
// keep-alives we can have for this server.
|
||||
PRUint32 count = CountIdleConnections(conn->ConnectionInfo());
|
||||
@ -407,6 +407,8 @@ nsHttpHandler::ReclaimConnection(nsHttpConnection *conn)
|
||||
NS_RELEASE(conn);
|
||||
}
|
||||
|
||||
LOG(("active connection count is now %u\n", mActiveConnections.Count()));
|
||||
|
||||
// process the pending transaction queue...
|
||||
if (mTransactionQ.Count() > 0)
|
||||
ProcessTransactionQ();
|
||||
@ -418,21 +420,32 @@ nsHttpHandler::ReclaimConnection(nsHttpConnection *conn)
|
||||
nsresult
|
||||
nsHttpHandler::CancelTransaction(nsHttpTransaction *trans, nsresult status)
|
||||
{
|
||||
nsHttpConnection *conn;
|
||||
|
||||
LOG(("nsHttpHandler::CancelTransaction [trans=%x status=%x]\n",
|
||||
trans, status));
|
||||
|
||||
NS_ENSURE_ARG_POINTER(trans);
|
||||
|
||||
nsAutoLock lock(mConnectionLock);
|
||||
|
||||
// we need to be inside the connection lock in order to know whether
|
||||
// or not this transaction has an associated connection. otherwise,
|
||||
// we'd have a race condition (see bug 85822).
|
||||
{
|
||||
nsAutoLock lock(mConnectionLock);
|
||||
|
||||
if (trans->Connection())
|
||||
trans->Connection()->OnTransactionComplete(status);
|
||||
conn = trans->Connection();
|
||||
if (conn)
|
||||
NS_ADDREF(conn); // make sure the connection stays around.
|
||||
else
|
||||
RemovePendingTransaction(trans);
|
||||
}
|
||||
|
||||
if (conn) {
|
||||
conn->OnTransactionComplete(trans, status);
|
||||
NS_RELEASE(conn);
|
||||
}
|
||||
else
|
||||
CancelPendingTransaction(trans, status);
|
||||
trans->OnStopTransaction(status);
|
||||
|
||||
return NS_OK;
|
||||
}
|
||||
@ -693,27 +706,39 @@ nsHttpHandler::InitiateTransaction_Locked(nsHttpTransaction *trans,
|
||||
NS_ADDREF(conn);
|
||||
|
||||
rv = conn->Init(ci);
|
||||
if (NS_FAILED(rv)) goto failed;
|
||||
if (NS_FAILED(rv)) {
|
||||
NS_RELEASE(conn);
|
||||
return rv;
|
||||
}
|
||||
}
|
||||
|
||||
rv = conn->SetTransaction(trans);
|
||||
if (NS_FAILED(rv)) goto failed;
|
||||
// assign the connection to the transaction.
|
||||
trans->SetConnection(conn);
|
||||
|
||||
// consider this connection active, even though it may fail.
|
||||
mActiveConnections.AppendElement(conn);
|
||||
return NS_OK;
|
||||
|
||||
// we must not hold the connection lock while making this call
|
||||
// as it could lead to deadlocks.
|
||||
PR_Unlock(mConnectionLock);
|
||||
rv = conn->SetTransaction(trans);
|
||||
PR_Lock(mConnectionLock);
|
||||
|
||||
if (NS_FAILED(rv)) {
|
||||
// the connection may already have been removed from the
|
||||
// active connection list.
|
||||
if (mActiveConnections.RemoveElement(conn))
|
||||
NS_RELEASE(conn);
|
||||
}
|
||||
|
||||
failed:
|
||||
NS_RELEASE(conn);
|
||||
return rv;
|
||||
}
|
||||
|
||||
// called with the connection lock held
|
||||
nsresult
|
||||
nsHttpHandler::CancelPendingTransaction(nsHttpTransaction *trans,
|
||||
nsresult status)
|
||||
nsHttpHandler::RemovePendingTransaction(nsHttpTransaction *trans)
|
||||
{
|
||||
LOG(("nsHttpHandler::CancelPendingTransaction [trans=%x status=%x]\n",
|
||||
trans, status));
|
||||
LOG(("nsHttpHandler::RemovePendingTransaction [trans=%x]\n", trans));
|
||||
|
||||
NS_ENSURE_ARG_POINTER(trans);
|
||||
|
||||
@ -723,16 +748,13 @@ nsHttpHandler::CancelPendingTransaction(nsHttpTransaction *trans,
|
||||
pt = (nsPendingTransaction *) mTransactionQ[i];
|
||||
|
||||
if (pt->Transaction() == trans) {
|
||||
trans->OnStopTransaction(status);
|
||||
|
||||
mTransactionQ.RemoveElementAt(i);
|
||||
delete pt;
|
||||
|
||||
return NS_OK;
|
||||
}
|
||||
}
|
||||
|
||||
LOG(("CancelPendingTransaction failed: transaction not in pending queue\n"));
|
||||
NS_WARNING("transaction not in pending queue");
|
||||
return NS_ERROR_NOT_AVAILABLE;
|
||||
}
|
||||
|
||||
|
||||
@ -168,7 +168,7 @@ private:
|
||||
nsHttpConnectionInfo *,
|
||||
PRBool failIfBusy = PR_FALSE);
|
||||
|
||||
nsresult CancelPendingTransaction(nsHttpTransaction *, nsresult status);
|
||||
nsresult RemovePendingTransaction(nsHttpTransaction *);
|
||||
|
||||
PRUint32 CountActiveConnections(nsHttpConnectionInfo *);
|
||||
PRUint32 CountIdleConnections(nsHttpConnectionInfo *);
|
||||
|
||||
@ -29,6 +29,7 @@
|
||||
#include "nsHttpResponseHead.h"
|
||||
#include "nsHttpChunkedDecoder.h"
|
||||
#include "nsIStringStream.h"
|
||||
#include "nsIFileStream.h"
|
||||
#include "pratom.h"
|
||||
#include "plevent.h"
|
||||
|
||||
@ -182,6 +183,14 @@ nsHttpTransaction::OnDataReadable(nsIInputStream *is)
|
||||
|
||||
LOG(("restarting transaction @%x\n", this));
|
||||
|
||||
// rewind streams in case we already wrote out the request
|
||||
nsCOMPtr<nsIRandomAccessStore> ras = do_QueryInterface(mReqHeaderStream);
|
||||
if (ras)
|
||||
ras->Seek(PR_SEEK_SET, 0);
|
||||
ras = do_QueryInterface(mReqUploadStream);
|
||||
if (ras)
|
||||
ras->Seek(PR_SEEK_SET, 0);
|
||||
|
||||
// just in case the connection is holding the last reference to us...
|
||||
NS_ADDREF_THIS();
|
||||
|
||||
@ -206,7 +215,7 @@ nsHttpTransaction::OnDataReadable(nsIInputStream *is)
|
||||
return rv;
|
||||
}
|
||||
|
||||
// called on the socket transport thread
|
||||
// called on any thread
|
||||
nsresult
|
||||
nsHttpTransaction::OnStopTransaction(nsresult status)
|
||||
{
|
||||
@ -494,7 +503,7 @@ nsHttpTransaction::HandleContent(char *buf,
|
||||
if (priorVal == 0) {
|
||||
// let the connection know that we are done with it; this should
|
||||
// result in OnStopTransaction being fired.
|
||||
return mConnection->OnTransactionComplete(NS_OK);
|
||||
return mConnection->OnTransactionComplete(this, NS_OK);
|
||||
}
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
@ -73,6 +73,8 @@ public:
|
||||
nsIInterfaceRequestor *Callbacks() { return mCallbacks; }
|
||||
nsIEventQueue *ConsumerEventQ() { return mConsumerEventQ; }
|
||||
nsISupports *SecurityInfo() { return mSecurityInfo; }
|
||||
PRBool IsDone() { return mTransactionDone; }
|
||||
nsresult Status() { return mStatus; }
|
||||
|
||||
// Called to take ownership of the response headers; the transaction
|
||||
// will drop any reference to the response headers after this call.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user