Bug 343332 - thread-safety assertion and crash with nsWeakRefPtr and nsLDAPConnection; r=Standard8,Neil
authorjhorak@redhat.com
Mon, 07 Feb 2011 22:12:38 +0000
changeset 7102 ee3e25817685e52122abddea30eddd519a6e1ce7
parent 7101 65aed01a5b8d2143d373325e362f4b1ecd9014b0
child 7103 b13f071ca3478087d8cdbe7a51b56721885b0495
push idunknown
push userunknown
push dateunknown
reviewersStandard8, Neil
bugs343332
Bug 343332 - thread-safety assertion and crash with nsWeakRefPtr and nsLDAPConnection; r=Standard8,Neil
ldap/xpcom/src/nsLDAPConnection.cpp
ldap/xpcom/src/nsLDAPConnection.h
ldap/xpcom/src/nsLDAPMessage.h
ldap/xpcom/src/nsLDAPOperation.cpp
ldap/xpcom/src/nsLDAPOperation.h
--- a/ldap/xpcom/src/nsLDAPConnection.cpp
+++ b/ldap/xpcom/src/nsLDAPConnection.cpp
@@ -56,102 +56,60 @@
 #include "nsIDNSRecord.h"
 #include "nsIRequestObserver.h"
 #include "nsIProxyObjectManager.h"
 #include "nsNetError.h"
 #include "nsLDAPOperation.h"
 #include "nsILDAPErrors.h"
 #include "nsIClassInfoImpl.h"
 #include "nsILDAPURL.h"
+#include "nsIObserverService.h"
+#include "mozilla/Services.h"
+#include "nsCRT.h"
 
 const char kConsoleServiceContractId[] = "@mozilla.org/consoleservice;1";
 const char kDNSServiceContractId[] = "@mozilla.org/network/dns-service;1";
 
 // constructor
 //
 nsLDAPConnection::nsLDAPConnection()
     : mConnectionHandle(0),
-      mPendingOperations(0),
-      mRunnable(0),
       mSSL(PR_FALSE),
       mVersion(nsILDAPConnection::VERSION3),
       mDNSRequest(0)
 {
+  // We have to abort all LDAP pending operation before shutdown.
+  nsCOMPtr<nsIObserverService> obsServ =
+      mozilla::services::GetObserverService();
+  obsServ->AddObserver(this, "profile-change-net-teardown", PR_FALSE);
 }
 
 // destructor
 //
 nsLDAPConnection::~nsLDAPConnection()
 {
   Close();
-  // Release the reference to the runnable object.
-  //
-  NS_IF_RELEASE(mRunnable);
 }
 
-// We need our own Release() here, so that we can lock around the delete.
-// This is needed to avoid a race condition with the weak reference to us,
-// which is used in nsLDAPConnectionLoop. A problem could occur if the
-// nsLDAPConnection gets destroyed while do_QueryReferent() is called,
-// since converting to the strong reference isn't MT safe.
-//
 NS_IMPL_THREADSAFE_ADDREF(nsLDAPConnection)
-
+NS_IMPL_THREADSAFE_RELEASE(nsLDAPConnection)
 NS_IMPL_CLASSINFO(nsLDAPConnection, NULL, nsIClassInfo::THREADSAFE,
                   NS_LDAPCONNECTION_CID)
 
 NS_INTERFACE_MAP_BEGIN(nsLDAPConnection)
   NS_INTERFACE_MAP_ENTRY(nsILDAPConnection)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
   NS_INTERFACE_MAP_ENTRY(nsIDNSListener)
+  NS_INTERFACE_MAP_ENTRY(nsIObserver)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsILDAPConnection)
   NS_IMPL_QUERY_CLASSINFO(nsLDAPConnection)
 NS_INTERFACE_MAP_END_THREADSAFE
-NS_IMPL_CI_INTERFACE_GETTER3(nsLDAPConnection, nsILDAPConnection, 
-                             nsISupportsWeakReference, nsIDNSListener)
-
-nsrefcnt
-nsLDAPConnection::Release(void)
-{
-    nsrefcnt count;
-
-    NS_PRECONDITION(0 != mRefCnt, "dup release");
-    count = PR_AtomicDecrement((PRInt32 *)&mRefCnt);
-    NS_LOG_RELEASE(this, count, "nsLDAPConnection");
-    if (0 == count) {
-        // As commented by danm: In the object's destructor, if by some
-        // convoluted, indirect means it happens to run into some code
-        // that temporarily references it (addref/release), then if the
-        // refcount had been left at 0 the unexpected release would
-        // attempt to reenter the object's destructor.
-        //
-        mRefCnt = 1; /* stabilize */
-
-        // If we have a mRunnable object, we need to make sure to lock it's
-        // mLock before we try to DELETE. This is to avoid a race condition.
-        // We also make sure to keep a strong reference to the runnable
-        // object, to make sure it doesn't get GCed from underneath us,
-        // while we are still holding a lock for instance.
-        //
-        if (mRunnable && mRunnable->mLock) {
-            nsLDAPConnectionLoop *runnable  = mRunnable;
-
-            NS_ADDREF(runnable);
-            PR_Lock(runnable->mLock);
-            delete this;
-            PR_Unlock(runnable->mLock);
-            NS_RELEASE(runnable);
-        } else {
-            delete this;
-        }
-
-        return 0;
-    }
-    return count;
-}
+NS_IMPL_CI_INTERFACE_GETTER4(nsLDAPConnection, nsILDAPConnection,
+                             nsISupportsWeakReference, nsIDNSListener,
+                             nsIObserver)
 
 NS_IMETHODIMP
 nsLDAPConnection::Init(nsILDAPURL *aUrl, const nsACString &aBindName,
                        nsILDAPMessageListener *aMessageListener,
                        nsISupports *aClosure, PRUint32 aVersion)
 {
   NS_ENSURE_ARG_POINTER(aUrl);
   NS_ENSURE_ARG_POINTER(aMessageListener);
@@ -162,17 +120,17 @@ nsLDAPConnection::Init(nsILDAPURL *aUrl,
   mInitListener = aMessageListener;
 
   // Make sure we haven't called Init earlier, i.e. there's a DNS
   // request pending.
   NS_ASSERTION(!mDNSRequest, "nsLDAPConnection::Init() "
                "Connection was already initialized\n");
 
   // Check and save the version number
-  if (aVersion != nsILDAPConnection::VERSION2 && 
+  if (aVersion != nsILDAPConnection::VERSION2 &&
       aVersion != nsILDAPConnection::VERSION3) {
     NS_ERROR("nsLDAPConnection::Init(): illegal version");
     return NS_ERROR_ILLEGAL_VALUE;
   }
   mVersion = aVersion;
 
   nsresult rv;
 
@@ -182,61 +140,59 @@ nsLDAPConnection::Init(nsILDAPURL *aUrl,
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRUint32 options;
   rv = aUrl->GetOptions(&options);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mSSL = options & nsILDAPURL::OPT_SECURE;
 
-  // allocate a hashtable to keep track of pending operations.
-  // 10 buckets seems like a reasonable size, and we do want it to 
-  // be threadsafe
-  mPendingOperations = new nsSupportsHashtable(10, PR_TRUE);
-  if (!mPendingOperations) {
-    NS_ERROR("failure initializing mPendingOperations hashtable");
-    return NS_ERROR_OUT_OF_MEMORY;
+  // Initialise the hashtable to keep track of pending operations.
+  // 10 buckets seems like a reasonable size.
+  if (!mPendingOperations.Init(10)) { //OOM
+    NS_ERROR("nsLDAPConnection::Init(): out of memory for mPendingOperations");
+    return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIThread> curThread = do_GetCurrentThread();
   if (!curThread) {
     NS_ERROR("nsLDAPConnection::Init(): couldn't get current thread");
     return NS_ERROR_FAILURE;
   }
 
   // Do the pre-resolve of the hostname, using the DNS service. This
   // will also initialize the LDAP connection properly, once we have
   // the IPs resolved for the hostname. This includes creating the
   // new thread for this connection.
   //
   // XXX - What return codes can we expect from the DNS service?
   //
-  nsCOMPtr<nsIDNSService> 
+  nsCOMPtr<nsIDNSService>
     pDNSService(do_GetService(kDNSServiceContractId, &rv));
 
   if (NS_FAILED(rv)) {
     NS_ERROR("nsLDAPConnection::Init(): couldn't create the DNS Service object");
     return NS_ERROR_FAILURE;
   }
 
   rv = aUrl->GetAsciiHost(mDNSHost);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // if the caller has passed in a space-delimited set of hosts, as the 
+  // if the caller has passed in a space-delimited set of hosts, as the
   // ldap c-sdk allows, strip off the trailing hosts for now.
   // Soon, we'd like to make multiple hosts work, but now make
   // at least the first one work.
   mDNSHost.CompressWhitespace(PR_TRUE, PR_TRUE);
 
   PRInt32 spacePos = mDNSHost.FindChar(' ');
   // trim off trailing host(s)
   if (spacePos != kNotFound)
     mDNSHost.Truncate(spacePos);
 
-  rv = pDNSService->AsyncResolve(mDNSHost, 0, this, curThread, 
+  rv = pDNSService->AsyncResolve(mDNSHost, 0, this, curThread,
                                  getter_AddRefs(mDNSRequest));
 
   if (NS_FAILED(rv)) {
     switch (rv) {
     case NS_ERROR_OUT_OF_MEMORY:
     case NS_ERROR_UNKNOWN_HOST:
     case NS_ERROR_FAILURE:
     case NS_ERROR_OFFLINE:
@@ -251,52 +207,85 @@ nsLDAPConnection::Init(nsILDAPURL *aUrl,
 }
 
 // this might get exposed to clients, so we've broken it
 // out of the destructor.
 void
 nsLDAPConnection::Close()
 {
   int rc;
-
   PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, ("unbinding\n"));
 
   if (mConnectionHandle) {
       // note that the ldap_unbind() call in the 5.0 version of the LDAP C SDK
       // appears to be exactly identical to ldap_unbind_s(), so it may in fact
       // still be synchronous
       //
       rc = ldap_unbind(mConnectionHandle);
 #ifdef PR_LOGGING
       if (rc != LDAP_SUCCESS) {
-          PR_LOG(gLDAPLogModule, PR_LOG_WARNING, 
-                 ("nsLDAPConnection::Close(): %s\n", 
+          PR_LOG(gLDAPLogModule, PR_LOG_WARNING,
+                 ("nsLDAPConnection::Close(): %s\n",
                   ldap_err2string(rc)));
       }
 #endif
       mConnectionHandle = nsnull;
   }
 
   PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, ("unbound\n"));
 
-  if (mPendingOperations) {
-      delete mPendingOperations;
-      mPendingOperations = nsnull;
-  }
+  NS_ASSERTION(NS_SUCCEEDED(mThread->Shutdown()),
+               "Failed to shutdown thread cleanly");
 
   // Cancel the DNS lookup if needed, and also drop the reference to the
   // Init listener (if still there).
   //
   if (mDNSRequest) {
       mDNSRequest->Cancel(NS_ERROR_ABORT);
       mDNSRequest = 0;
   }
   mInitListener = 0;
 
 }
+/** Get list of pending operation and store pointers to array
+  * \param userArg pointer to nsTArray<nsILDAPOperation*>
+  */
+PLDHashOperator
+GetListOfPendingOperations(const PRUint32 &key, nsILDAPOperation *op, void *userArg)
+{
+  nsTArray<nsILDAPOperation*>* pending_operations = static_cast<nsTArray<nsILDAPOperation*>* >(userArg);
+  pending_operations->AppendElement(op);
+  return PL_DHASH_NEXT;
+}
+
+NS_IMETHODIMP
+nsLDAPConnection::Observe(nsISupports *aSubject, const char *aTopic,
+                          const PRUnichar *aData)
+{
+  if (!nsCRT::strcmp(aTopic, "profile-change-net-teardown")) {
+    // Abort all ldap requests.
+    if (mPendingOperations.Count() > 0) {
+      /* We cannot use enumerate function to abort operations because
+       * nsILDAPOperation::AbandonExt() is modifying list of operations
+       * and this leads to starvation.
+       * We have to do a copy of pending operations.
+       */
+      nsTArray<nsILDAPOperation*> pending_operations;
+      mPendingOperations.EnumerateRead(GetListOfPendingOperations, (void *) (&pending_operations));
+      for (PRUint32 i = 0; i < pending_operations.Length(); i++) {
+        pending_operations[i]->AbandonExt();
+      }
+    }
+    Close();
+  } else {
+    NS_NOTREACHED("unexpected topic");
+    return NS_ERROR_UNEXPECTED;
+  }
+  return NS_OK;
+}
 
 NS_IMETHODIMP
 nsLDAPConnection::GetClosure(nsISupports **_retval)
 {
     if (!_retval) {
         return NS_ERROR_ILLEGAL_VALUE;
     }
     NS_IF_ADDREF(*_retval = mClosure);
@@ -320,17 +309,17 @@ nsLDAPConnection::GetBindName(nsACString
     _retval.Assign(mBindName);
     return NS_OK;
 }
 
 // wrapper for ldap_get_lderrno
 // XXX should copy before returning
 //
 NS_IMETHODIMP
-nsLDAPConnection::GetLdErrno(nsACString& matched, nsACString& errString, 
+nsLDAPConnection::GetLdErrno(nsACString& matched, nsACString& errString,
                              PRInt32 *_retval)
 {
     char *match, *err;
 
     NS_ENSURE_ARG_POINTER(_retval);
 
     *_retval = ldap_get_lderrno(mConnectionHandle, &match, &err);
     matched.Assign(match);
@@ -359,546 +348,135 @@ nsLDAPConnection::GetErrorString(PRUnich
     //
     *_retval = UTF8ToNewUnicode(nsDependentCString(rv));
     if (!*_retval) {
         return NS_ERROR_OUT_OF_MEMORY;
     }
     return NS_OK;
 }
 
-/** 
+/**
  * Add an nsILDAPOperation to the list of operations pending on
  * this connection.  This is also mainly intended for use by the
  * nsLDAPOperation code.
  */
 nsresult
-nsLDAPConnection::AddPendingOperation(nsILDAPOperation *aOperation)
+nsLDAPConnection::AddPendingOperation(PRUint32 aOperationID, nsILDAPOperation *aOperation)
 {
-    NS_ENSURE_TRUE(mPendingOperations, NS_ERROR_FAILURE);
-    NS_ENSURE_ARG_POINTER(aOperation);
+  NS_ENSURE_ARG_POINTER(aOperation);
 
-    if (!aOperation) {
-        return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // find the message id
-    PRInt32 msgID;
-    aOperation->GetMessageID(&msgID);
+  nsIRunnable* runnable = new nsLDAPConnectionRunnable(aOperationID, aOperation,
+                                                       this);
+  mPendingOperations.Put((PRUint32)aOperationID, aOperation);
 
-    // turn it into an nsVoidKey.  note that this is another spot that
-    // assumes that sizeof(void*) >= sizeof(PRInt32).  
-    //
-    // XXXdmose  should really create an nsPRInt32Key.
-    //
-    nsVoidKey *key = new nsVoidKey(reinterpret_cast<void *>(msgID));
-    if (!key) {
-        return NS_ERROR_OUT_OF_MEMORY;
-    }
+  nsresult rv;
+  if (!mThread)
+  {
+    rv = NS_NewThread(getter_AddRefs(mThread), runnable);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  else
+  {
+    rv = mThread->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
-    // actually add it to the queue.  if Put indicates that an item in 
-    // the hashtable was actually overwritten, something is really wrong.
-    //
-    if (mPendingOperations->Put(key, aOperation)) {
-        NS_ERROR("nsLDAPConnection::AddPendingOperation() "
-                 "mPendingOperations->Put() overwrote an item.  msgId "
-                 "is supposed to be unique\n");
-        delete key;
-        return NS_ERROR_UNEXPECTED;
-    }
+  PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
+         ("pending operation added; total pending operations now = %d\n",
+          mPendingOperations.Count()));
 
-    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-           ("pending operation added; total pending operations now = %d\n", 
-            mPendingOperations->Count()));
-
-    delete key;
-    return NS_OK;
+  return NS_OK;
 }
 
 /**
  * Remove an nsILDAPOperation from the list of operations pending on this
  * connection.  Mainly intended for use by the nsLDAPOperation code.
  *
  * @param aOperation    operation to add
  * @exception NS_ERROR_INVALID_POINTER  aOperation was NULL
  * @exception NS_ERROR_OUT_OF_MEMORY    out of memory
- * @exception NS_ERROR_FAILURE      could not delete the operation 
+ * @exception NS_ERROR_FAILURE      could not delete the operation
  *
  * void removePendingOperation(in nsILDAPOperation aOperation);
  */
 nsresult
-nsLDAPConnection::RemovePendingOperation(nsILDAPOperation *aOperation)
+nsLDAPConnection::RemovePendingOperation(PRUint32 aOperationID)
 {
-    nsresult rv;
-    PRInt32 msgID;
+  NS_ENSURE_TRUE(aOperationID > 0, NS_ERROR_UNEXPECTED);
 
-    NS_ENSURE_TRUE(mPendingOperations, NS_OK);
-    NS_ENSURE_ARG_POINTER(aOperation);
-
-    // find the message id
-    //
-    rv = aOperation->GetMessageID(&msgID);
-    NS_ENSURE_SUCCESS(rv, rv);
+  PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
+         ("nsLDAPConnection::RemovePendingOperation(): operation removed\n"));
 
-    // turn it into an nsVoidKey.  note that this is another spot that
-    // assumes that sizeof(void*) >= sizeof(PRInt32).  
-    //
-    // XXXdmose  should really create an nsPRInt32Key.
-    //
-    nsVoidKey *key = new nsVoidKey(reinterpret_cast<void *>(msgID));
-    if (!key) {
-        return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    // remove the operation if it's still there.  
-    //
-    if (!mPendingOperations->Remove(key)) {
+  mPendingOperations.Remove(aOperationID);
+  PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
+         ("nsLDAPConnection::RemovePendingOperation(): operation "
+          "removed; total pending operations now = %d\n",
+          mPendingOperations.Count()));
 
-        PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-               ("nsLDAPConnection::RemovePendingOperation(): could not remove "
-                "operation; perhaps it already completed."));
-    } else {
-
-        PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-               ("nsLDAPConnection::RemovePendingOperation(): operation "
-                "removed; total pending operations now = %d\n", 
-                mPendingOperations->Count()));
-    }
-
-    delete key;
-    return NS_OK;
+  return NS_OK;
 }
 
 nsresult
-nsLDAPConnection::InvokeMessageCallback(LDAPMessage *aMsgHandle, 
+nsLDAPConnection::InvokeMessageCallback(LDAPMessage *aMsgHandle,
                                         nsILDAPMessage *aMsg,
+                                        PRInt32 aOperation,
                                         PRBool aRemoveOpFromConnQ)
 {
-    PRInt32 msgId;
-    nsresult rv;
-    nsCOMPtr<nsILDAPOperation> operation;
-    nsCOMPtr<nsILDAPMessageListener> listener;
-
 #if defined(DEBUG)
-    // We only want this being logged for debug builds so as not to affect performance too much.
-    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, ("InvokeMessageCallback entered\n"));
+  // We only want this being logged for debug builds so as not to affect performance too much.
+  PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, ("InvokeMessageCallback entered\n"));
 #endif
 
-    // get the message id corresponding to this operation
-    //
-    msgId = ldap_msgid(aMsgHandle);
-    if (msgId == -1) {
-        NS_ERROR("nsLDAPConnection::GetCallbackByMessage(): "
-                 "ldap_msgid() failed\n");
-        return NS_ERROR_FAILURE;
-    }
-
-    // get this in key form.  note that using nsVoidKey in this way assumes
-    // that sizeof(void *) >= sizeof PRInt32
-    //
-    nsVoidKey *key = new nsVoidKey(reinterpret_cast<void *>(msgId));
-    if (!key)
-        return NS_ERROR_OUT_OF_MEMORY;
-
-    // find the operation in question
-    operation = getter_AddRefs(static_cast<nsILDAPOperation *>(mPendingOperations->Get(key)));
-    if (!operation) {
-
-        PR_LOG(gLDAPLogModule, PR_LOG_WARNING, 
-               ("Warning: InvokeMessageCallback(): couldn't find "
-                "nsILDAPOperation corresponding to this message id\n"));
-        delete key;
-
-        // this may well be ok, since it could just mean that the operation
-        // was aborted while some number of messages were already in transit.
-        //
-        return NS_OK;
-    }
-
-
-    // Make sure the mOperation member is set to this operation before
-    // we call the callback.
-    //
-    static_cast<nsLDAPMessage *>(aMsg)->mOperation = operation;
-
-    // get the message listener object (this may be a proxy for a
-    // callback which should happen on another thread)
-    //
-    rv = operation->GetMessageListener(getter_AddRefs(listener));
-    if (NS_FAILED(rv)) {
-        NS_ERROR("nsLDAPConnection::InvokeMessageCallback(): probable "
-                 "memory corruption: GetMessageListener() returned error");
-        delete key;
-        return NS_ERROR_UNEXPECTED;
-    }
+  nsresult rv;
+  // Get the operation.
+  nsCOMPtr<nsILDAPOperation> operation;
+  mPendingOperations.Get((PRUint32)aOperation, getter_AddRefs(operation));
 
-    // invoke the callback 
-    //
-    if (listener) {
-      listener->OnLDAPMessage(aMsg);
-    }
-    // if requested (ie the operation is done), remove the operation
-    // from the connection queue.
-    //
-    if (aRemoveOpFromConnQ) {
-        nsCOMPtr <nsLDAPOperation> operation = 
-          getter_AddRefs(static_cast<nsLDAPOperation *>
-                                    (mPendingOperations->Get(key)));
-        // try to break cycles
-        if (operation)
-          operation->Clear();
-        rv = mPendingOperations->Remove(key);
-        if (NS_FAILED(rv)) {
-            NS_ERROR("nsLDAPConnection::InvokeMessageCallback: unable to "
-                     "remove operation from the connection queue\n");
-            delete key;
-            return NS_ERROR_UNEXPECTED;
-        }
+  NS_ENSURE_TRUE(operation, NS_ERROR_NULL_POINTER);
 
-        PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-               ("pending operation removed; total pending operations now ="
-                " %d\n", mPendingOperations->Count()));
-    }
-
-    delete key;
-    return NS_OK;
-}
-
-// constructor
-//
-nsLDAPConnectionLoop::nsLDAPConnectionLoop()
-    : mWeakConn(0),
-      mLock(0)
-{
-}
-
-// destructor
-//
-nsLDAPConnectionLoop::~nsLDAPConnectionLoop()
-{
-    // Delete the lock object
-    if (mLock)
-        PR_DestroyLock(mLock);
-}
+  static_cast<nsLDAPMessage *>(aMsg)->mOperation = operation;
 
-NS_IMPL_THREADSAFE_ISUPPORTS1(nsLDAPConnectionLoop, nsIRunnable)
-
-NS_IMETHODIMP
-nsLDAPConnectionLoop::Init()
-{
-    if (!mLock) {
-        mLock = PR_NewLock();
-        if (!mLock) {
-            NS_ERROR("nsLDAPConnectionLoop::Init: out of memory ");
-            return NS_ERROR_OUT_OF_MEMORY;
-        }
-    }
-
-    return NS_OK;
-}
-
-// typedef PRBool
-// (* nsHashtableEnumFunc)
-//      (nsHashKey *aKey, void *aData, void* aClosure);
-PRBool
-CheckLDAPOperationResult(nsHashKey *aKey, void *aData, void* aClosure)
-{
-    int lderrno;
-    nsresult rv;
-    PRInt32 returnCode;
-    LDAPMessage *msgHandle;
-    nsCOMPtr<nsILDAPMessage> msg;
-    PRBool operationFinished = PR_TRUE;
-    struct timeval timeout = { 0, 0 }; 
-    PRIntervalTime sleepTime = PR_MillisecondsToInterval(40);
-
-    // we need to access some of the connection loop's objects
-    //
-    nsLDAPConnectionLoop *loop = 
-        static_cast<nsLDAPConnectionLoop *>(aClosure);
-
-    // get the console service so we can log messages
-    //
-    nsCOMPtr<nsIConsoleService> consoleSvc = 
-        do_GetService(kConsoleServiceContractId, &rv);
-    if (NS_FAILED(rv)) {
-        NS_ERROR("CheckLDAPOperationResult() couldn't get console service");
-        return NS_ERROR_FAILURE;
-    }
-
-    returnCode = ldap_result(loop->mRawConn->mConnectionHandle,
-                             aKey->HashCode(), LDAP_MSG_ONE,
-                                 &timeout, &msgHandle);
-
-        // if we didn't error or timeout, create an nsILDAPMessage
-        //      
-        switch (returnCode) {
-
-        case 0: // timeout
-
-            // the connection may not exist yet.  sleep for a while
-            // to avoid a problem where the LDAP connection/thread isn't 
-            // ready quite yet, and we want to avoid a very busy loop.
-            //
-            PR_Sleep(sleepTime);
-            return PR_TRUE;
-
-        case -1: // something went wrong 
-
-        lderrno = ldap_get_lderrno(loop->mRawConn->mConnectionHandle, 0, 0);
-
-            // Sleep briefly, to avoid a very busy loop again.
-            //
-            PR_Sleep(sleepTime);
-
-            switch (lderrno) {
-
-            case LDAP_SERVER_DOWN:
-                // We might want to shutdown the thread here, but it has
-                // implications to the user of the nsLDAPConnection, so
-                // for now we just ignore it. It's up to the owner of
-                // the nsLDAPConnection to detect the error, and then
-                // create a new connection.
-                //
-                PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-                       ("CheckLDAPOperationResult(): ldap_result returned" 
-                        " LDAP_SERVER_DOWN"));
-                break;
-
-            case LDAP_DECODING_ERROR:
-                consoleSvc->LogStringMessage(
-                    NS_LITERAL_STRING("LDAP: WARNING: decoding error; possible corrupt data received").get());
-                NS_WARNING("CheckLDAPOperationResult(): ldaperrno = "
-                           "LDAP_DECODING_ERROR after ldap_result()");
-                break;
-
-            case LDAP_NO_MEMORY:
-                NS_ERROR("CheckLDAPOperationResult(): Couldn't allocate memory"
-                         " while getting async operation result");
-                // punt and hope things work out better next time around
-                break;
+  // get the message listener object (this may be a proxy for a
+  // callback which should happen on another thread)
+  nsCOMPtr<nsILDAPMessageListener> listener;
+  rv = operation->GetMessageListener(getter_AddRefs(listener));
+  if (NS_FAILED(rv))
+  {
+    NS_ERROR("nsLDAPConnection::InvokeMessageCallback(): probable "
+             "memory corruption: GetMessageListener() returned error");
+    return NS_ERROR_UNEXPECTED;
+  }
 
-            case LDAP_PARAM_ERROR:
-                // I think it's possible to hit a race condition where we're
-                // continuing to poll for a result after the C SDK connection
-                // has removed the operation because the connection has gone
-                // dead.  In theory we should fix this.  Practically, it's
-                // unclear to me whether it matters.
-                //
-                NS_WARNING("CheckLDAPOperationResult(): ldap_result returned"
-                           " LDAP_PARAM_ERROR");
-                break;
-
-            default:
-                NS_ERROR("CheckLDAPOperationResult(): lderrno set to "
-                           "unexpected value after ldap_result() "
-                           "call in nsLDAPConnection::Run()");
-                PR_LOG(gLDAPLogModule, PR_LOG_ERROR, 
-                       ("lderrno = 0x%x", lderrno));
-                break;
-            }
-            break;
-
-        case LDAP_RES_SEARCH_ENTRY:
-        case LDAP_RES_SEARCH_REFERENCE:
-            // XXX what should we do with LDAP_RES_SEARCH_EXTENDED?
-
-            // not done yet, so we shouldn't remove the op from the conn q
-            operationFinished = PR_FALSE;
-
-            // fall through to default case
-
-        default: // initialize the message and call the callback
-
-            // we want nsLDAPMessage specifically, not a compatible, since
-            // we're sharing native objects used by the LDAP C SDK
-            //
-            nsLDAPMessage *rawMsg = new nsLDAPMessage();
-
-            if (!rawMsg) {
-            NS_ERROR("CheckLDAPOperationResult(): couldn't allocate memory"
-                     " for new LDAP message; search entry dropped");
-                // punt and hope things work out better next time around
-                break;
-            }
-
-            // initialize the message, using a protected method not available
-            // through nsILDAPMessage (which is why we need the raw pointer)
-            //
-            rv = rawMsg->Init(loop->mRawConn, msgHandle);
-
-            // now let the scoping mechanisms provided by nsCOMPtr manage
-            // the reference for us.
-            //
-            msg = rawMsg;
-            
-            switch (rv) {
-
-            case NS_OK: {
-                PRInt32 errorCode;
-                rawMsg->GetErrorCode(&errorCode);
-                // maybe a version error, e.g., using v3 on a v2 server.
-                // if we're using v3, try v2.
-                //
-                if (errorCode == LDAP_PROTOCOL_ERROR && 
-                   loop->mRawConn->mVersion == nsILDAPConnection::VERSION3) {
-                    nsCAutoString password;
-                    loop->mRawConn->mVersion = nsILDAPConnection::VERSION2;
-                    ldap_set_option(loop->mRawConn->mConnectionHandle,
-                          LDAP_OPT_PROTOCOL_VERSION, &loop->mRawConn->mVersion);
-                    nsCOMPtr <nsILDAPOperation> operation = 
-                      static_cast<nsILDAPOperation *>(static_cast<nsISupports *>(aData));
-                    // we pass in an empty password to tell the operation that 
-                    // it should use the cached password.
-                    //
-                    rv = operation->SimpleBind(password);
-                    if (NS_SUCCEEDED(rv)) {
-                        operationFinished = PR_FALSE;
-                        // we don't want to notify callers that we're done...
-                        return PR_TRUE;
-                    }
-                }
-                
-                // If we're midway through a SASL Bind, we need to continue
-                // without letting our caller know what we're up to!
-                //
-                if (errorCode == LDAP_SASL_BIND_IN_PROGRESS) {
-                    struct berval *creds;
-                    ldap_parse_sasl_bind_result(
-                      loop->mRawConn->mConnectionHandle, msgHandle, 
-                      &creds, 0);
-
-                    nsCOMPtr <nsILDAPOperation> operation =
-                      static_cast<nsILDAPOperation *>
-                      (static_cast<nsISupports *>(aData));
-
-                    rv = operation->SaslStep(creds->bv_val, creds->bv_len);
-                    if (NS_SUCCEEDED(rv)) {
-                        return PR_TRUE;
-                    }
-                }
-            }
-            break;
+  // invoke the callback
+  if (listener)
+    listener->OnLDAPMessage(aMsg);
 
-            case NS_ERROR_LDAP_DECODING_ERROR:
-                consoleSvc->LogStringMessage(
-                    NS_LITERAL_STRING("LDAP: WARNING: decoding error; possible corrupt data received").get());
-            NS_WARNING("CheckLDAPOperationResult(): ldaperrno = "
-                           "LDAP_DECODING_ERROR after ldap_result()");
-            return PR_TRUE;
-
-            case NS_ERROR_OUT_OF_MEMORY:
-                // punt and hope things work out better next time around
-            return PR_TRUE;
-
-            case NS_ERROR_ILLEGAL_VALUE:
-            case NS_ERROR_UNEXPECTED:
-            default:
-                // shouldn't happen; internal error
-                //
-            NS_ERROR("CheckLDAPOperationResult(): nsLDAPMessage::Init() "
-                           "returned unexpected value.");
-
-                // punt and hope things work out better next time around
-            return PR_TRUE;
-            }
-
-            // invoke the callback on the nsILDAPOperation corresponding to 
-            // this message
-            //
-        rv = loop->mRawConn->InvokeMessageCallback(msgHandle, msg, 
-                                                    operationFinished);
-            if (NS_FAILED(rv)) {
-            NS_ERROR("CheckLDAPOperationResult(): error invoking message"
-                     " callback");
-                // punt and hope things work out better next time around
-            return PR_TRUE;
-            }
-
-            break;
-        }       
-
-    return PR_TRUE;
-}
-
-// for nsIRunnable.  this thread spins in ldap_result() awaiting the next
-// message.  once one arrives, it dispatches it to the nsILDAPMessageListener 
-// on the main thread.
-//
-// XXX do all returns from this function need to do thread cleanup?
-//
-NS_IMETHODIMP
-nsLDAPConnectionLoop::Run(void)
-{
-    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
-           ("nsLDAPConnection::Run() entered\n"));
+  // if requested (ie the operation is done), remove the operation
+  // from the connection queue.
+  if (aRemoveOpFromConnQ)
+  {
+    // try to break cycles
+    nsLDAPOperation* nsoperation = static_cast<nsLDAPOperation *>(operation.get());
+    if (nsoperation)
+      nsoperation->Clear();
+    mPendingOperations.Remove(aOperation);
 
-    // wait for results
-    //
-    while(1) {
-
-        // Exit this thread if we no longer have an nsLDAPConnection
-        // associated with it. We also aquire a lock here, to make sure
-        // to avoid a possible race condition when the nsLDAPConnection
-        // is destructed during the call to do_QueryReferent() (since that
-        // function isn't MT safe).
-        //
-        nsresult rv;
-
-        PR_Lock(mLock);
-        nsCOMPtr<nsILDAPConnection> strongConn = 
-            do_QueryReferent(mWeakConn, &rv);
-        PR_Unlock(mLock);
-
-        if (NS_FAILED(rv)) {
-            mWeakConn = 0;
-            return NS_OK;
-        }
-        // we use a raw connection because we need to call non-interface
-        // methods
-        mRawConn = static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>(strongConn.get()));
+    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
+           ("pending operation removed; total pending operations now ="
+            " %d\n", mPendingOperations.Count()));
+  }
 
-        // XXX deal with timeouts better
-        //
-        NS_ASSERTION(mRawConn->mConnectionHandle, "nsLDAPConnection::Run(): "
-                     "no connection created.\n");
-
-        // We can't enumerate over mPendingOperations itself, because the
-        // callback needs to modify mPendingOperations.  So we clone it first,
-        // and enumerate over the clone.  It kinda sucks that we need to do
-        // this everytime we poll, but the hashtable will pretty much always
-        // be small.
-        //
-        // only clone if the number of pending operations is non-zero
-        // otherwise, put the LDAP connection thread to sleep (briefly)
-        // until there is pending operations..
-        if (mRawConn->mPendingOperations->Count()) {
-          nsHashtable *hashtableCopy = mRawConn->mPendingOperations->Clone();
-          if (hashtableCopy) {
-            hashtableCopy->Enumerate(CheckLDAPOperationResult, this);
-            delete hashtableCopy;
-          } else {
-            // punt and hope it works next time around
-            NS_ERROR("nsLDAPConnectionLoop::Run() error cloning hashtable");
-          }
-        }
-        else {
-          PR_Sleep(PR_MillisecondsToInterval(40));
-        }
-    }
-
-    // This will never happen, but here just in case.
-    //
-    return NS_OK;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLDAPConnection::OnLookupComplete(nsICancelable *aRequest,
                                    nsIDNSRecord  *aRecord,
                                    nsresult       aStatus)
-{    
+{
     nsresult rv = NS_OK;
 
     if (aRecord) {
         // Build mResolvedIP list
         //
         mResolvedIP.Truncate();
 
         PRInt32 index = 0;
@@ -914,17 +492,17 @@ nsLDAPConnection::OnLookupComplete(nsICa
             if (addr.raw.family == PR_AF_INET || v4mapped) {
                 // If there are more IPs in the list, we separate them with
                 // a space, as supported/used by the LDAP C-SDK.
                 //
                 if (index++)
                     mResolvedIP.Append(' ');
 
                 // Convert the IPv4 address to a string, and append it to our
-                // list of IPs.  Strip leading '::FFFF:' (the IPv4-mapped-IPv6 
+                // list of IPs.  Strip leading '::FFFF:' (the IPv4-mapped-IPv6
                 // indicator) if present.
                 //
                 PR_NetAddrToString(&addr, addrbuf, sizeof(addrbuf));
                 if ((addrbuf[0] == ':') && (strlen(addrbuf) > 7))
                     mResolvedIP.Append(addrbuf+7);
                 else
                     mResolvedIP.Append(addrbuf);
             }
@@ -948,17 +526,17 @@ nsLDAPConnection::OnLookupComplete(nsICa
             break;
         }
     } else if (!mResolvedIP.Length()) {
         // We have no host resolved, that is very bad, and should most
         // likely have been caught earlier.
         //
         NS_ERROR("nsLDAPConnection::OnStopLookup(): the resolved IP "
                  "string is empty.\n");
-        
+
         rv = NS_ERROR_UNKNOWN_HOST;
     } else {
         // We've got the IP(s) for the hostname, now lets setup the
         // LDAP connection using this information. Note that if the
         // LDAP server returns a referral, the C-SDK will perform a
         // new, synchronous DNS lookup, which might hang (but hopefully
         // if we've come this far, DNS is working properly).
         //
@@ -971,26 +549,26 @@ nsLDAPConnection::OnLookupComplete(nsICa
         if ( !mConnectionHandle ) {
             rv = NS_ERROR_FAILURE;  // LDAP C SDK API gives no useful error
         } else {
 #if defined(DEBUG_dmose) || defined(DEBUG_bienvenu)
             const int lDebug = 0;
             ldap_set_option(mConnectionHandle, LDAP_OPT_DEBUG_LEVEL, &lDebug);
 #endif
 
-            // the C SDK currently defaults to v2.  if we're to use v3, 
+            // the C SDK currently defaults to v2.  if we're to use v3,
             // tell it so.
             //
             int version;
             switch (mVersion) {
             case 2:
                 break;
             case 3:
                 version = LDAP_VERSION3;
-                ldap_set_option(mConnectionHandle, LDAP_OPT_PROTOCOL_VERSION, 
+                ldap_set_option(mConnectionHandle, LDAP_OPT_PROTOCOL_VERSION,
                                 &version);
 		break;
             default:
                 NS_ERROR("nsLDAPConnection::OnLookupComplete(): mVersion"
                          " invalid");
             }
 
 #ifdef MOZ_PSM
@@ -1012,56 +590,134 @@ nsLDAPConnection::OnLookupComplete(nsICa
                 if (NS_FAILED(rv)) {
                     NS_ERROR("nsLDAPConnection::OnStopLookup(): Error"
                              " installing secure LDAP routines for"
                              " connection");
                 }
             }
 #endif
         }
-
-        // Create a new runnable object, and increment the refcnt. The
-        // thread will also hold a strong ref to the runnable, but we need
-        // to make sure it doesn't get destructed until we are done with
-        // all locking etc. in nsLDAPConnection::Release().
-        //
-        mRunnable = new nsLDAPConnectionLoop();
-        NS_IF_ADDREF(mRunnable);
-        if (!mRunnable || NS_FAILED(mRunnable->Init())) {
-            rv = NS_ERROR_OUT_OF_MEMORY;
-        } else {
-            // Here we keep a weak reference in the runnable object to the
-            // nsLDAPConnection ("this"). This avoids the problem where a
-            // connection can't get destructed because of the new thread
-            // keeping a strong reference to it. It also helps us know when
-            // we need to exit the new thread: when we can't convert the weak
-            // reference to a strong ref, we know that the nsLDAPConnection
-            // object is gone, and we need to stop the thread running.
-            //
-            nsCOMPtr<nsILDAPConnection> conn =
-                static_cast<nsILDAPConnection *>(this);
-
-            mRunnable->mWeakConn = do_GetWeakReference(conn);
-
-            // kick off a thread for result listening and marshalling
-            //
-            rv = NS_NewThread(getter_AddRefs(mThread), mRunnable);
-            if (NS_FAILED(rv)) {
-                rv = NS_ERROR_NOT_AVAILABLE;
-            }
-            // XXX(darin): We need to shutdown this thread at some point.
-            //             Otherwise, it will stick around until shutdown.
-        }
     }
 
     // Drop the DNS request object, we no longer need it, and set the flag
     // indicating that DNS has finished.
     //
     mDNSRequest = 0;
     mDNSHost.Truncate();
 
     // Call the listener, and then we can release our reference to it.
     //
     mInitListener->OnLDAPInit(this, rv);
     mInitListener = 0;
 
     return rv;
 }
+
+nsLDAPConnectionRunnable::nsLDAPConnectionRunnable(PRInt32 aOperationID,
+                                                   nsILDAPOperation *aOperation,
+                                                   nsLDAPConnection *aConnection)
+  : mOperationID(aOperationID),  mConnection(aConnection)
+{
+}
+
+nsLDAPConnectionRunnable::~nsLDAPConnectionRunnable()
+{
+}
+
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsLDAPConnectionRunnable, nsIRunnable)
+
+NS_IMETHODIMP nsLDAPConnectionRunnable::Run()
+{
+  if (!mOperationID) {
+    NS_ERROR("mOperationID is null");
+    return NS_ERROR_NULL_POINTER;
+  }
+
+  LDAPMessage *msgHandle;
+  PRBool operationFinished = PR_TRUE;
+  nsRefPtr<nsLDAPMessage> msg;
+
+  struct timeval timeout = { 0, 0 };
+
+  nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
+  PRInt32 returnCode = ldap_result(mConnection->mConnectionHandle, mOperationID, LDAP_MSG_ONE, &timeout, &msgHandle);
+  switch (returnCode)
+  {
+    // timeout
+    case 0:
+      // XXX do we need a timer?
+      return thread->Dispatch(this, nsIEventTarget::DISPATCH_NORMAL);
+    case -1:
+      NS_ERROR("We don't know what went wrong with the ldap operation");
+      return NS_ERROR_FAILURE;
+
+    case LDAP_RES_SEARCH_ENTRY:
+    case LDAP_RES_SEARCH_REFERENCE:
+      // XXX what should we do with LDAP_RES_SEARCH_EXTENDED
+      operationFinished = PR_FALSE;
+    default:
+    {
+      msg = new nsLDAPMessage;
+      if (!msg)
+        return NS_ERROR_NULL_POINTER;
+
+      // initialize the message, using a protected method not available
+      // through nsILDAPMessage (which is why we need the raw pointer)
+      nsresult rv = msg->Init(mConnection, msgHandle);
+
+      switch (rv)
+      {
+        case NS_OK:
+        {
+          PRInt32 errorCode;
+          msg->GetErrorCode(&errorCode);
+
+          // maybe a version error, e.g., using v3 on a v2 server.
+          // if we're using v3, try v2.
+          if (errorCode == LDAP_PROTOCOL_ERROR &&
+              mConnection->mVersion == nsILDAPConnection::VERSION3)
+          {
+            nsCAutoString password;
+            mConnection->mVersion = nsILDAPConnection::VERSION2;
+            ldap_set_option(mConnection->mConnectionHandle,
+                            LDAP_OPT_PROTOCOL_VERSION, &mConnection->mVersion);
+
+            if (NS_SUCCEEDED(rv))
+            {
+              // We don't want to notify callers that we are done, so
+              // redispatch the runnable.
+              // XXX do we need a timer?
+              rv = thread->Dispatch(this, nsIEventTarget::DISPATCH_NORMAL);
+              NS_ENSURE_SUCCESS(rv, rv);
+              return NS_OK;
+            }
+          }
+          break;
+        }
+          // Error code handling in here
+        default:
+          return NS_OK;
+      }
+
+      // invoke the callback on the nsILDAPOperation corresponding to
+      // this message
+      rv = mConnection->InvokeMessageCallback(msgHandle, msg, mOperationID,
+                                              operationFinished);
+      if (NS_FAILED(rv))
+      {
+        NS_ERROR("CheckLDAPOperationResult(): error invoking message"
+                 " callback");
+        // punt and hope things work out better next time around
+        return NS_OK;
+      }
+
+      if (!operationFinished)
+      {
+        // XXX do we need a timer?
+        rv = thread->Dispatch(this, nsIEventTarget::DISPATCH_NORMAL);
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      break;
+    }
+  }
+  return NS_OK;
+}
--- a/ldap/xpcom/src/nsLDAPConnection.h
+++ b/ldap/xpcom/src/nsLDAPConnection.h
@@ -1,10 +1,10 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * 
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -43,124 +43,116 @@
 
 #include "nsILDAPConnection.h"
 #include "ldap.h"
 #include "nsString.h"
 #include "nsIThread.h"
 #include "nsIRunnable.h"
 #include "nsCOMPtr.h"
 #include "nsILDAPMessageListener.h"
-#include "nsHashtable.h"
+#include "nsInterfaceHashtable.h"
 #include "nspr.h"
 #include "nsWeakReference.h"
 #include "nsWeakPtr.h"
 #include "nsIDNSListener.h"
 #include "nsICancelable.h"
 #include "nsIRequest.h"
+#include "nsCOMArray.h"
+#include "nsIObserver.h"
+#include "nsAutoPtr.h"
 
 // 0d871e30-1dd2-11b2-8ea9-831778c78e93
 //
 #define NS_LDAPCONNECTION_CID \
 { 0x0d871e30, 0x1dd2, 0x11b2, \
  { 0x8e, 0xa9, 0x83, 0x17, 0x78, 0xc7, 0x8e, 0x93 }}
 
-class nsLDAPConnectionLoop;
-
 class nsLDAPConnection : public nsILDAPConnection,
                          public nsSupportsWeakReference,
-                         public nsIDNSListener
+                         public nsIDNSListener,
+                         public nsIObserver
 
 {
     friend class nsLDAPOperation;
     friend class nsLDAPMessage;
-    friend class nsLDAPConnectionLoop;
-    friend PRBool CheckLDAPOperationResult(nsHashKey *aKey,
-                                           void *aData,
-                                           void* aClosure);
+    friend class nsLDAPConnectionRunnable;
 
   public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSILDAPCONNECTION
     NS_DECL_NSIDNSLISTENER
+    NS_DECL_NSIOBSERVER
 
     // constructor & destructor
     //
     nsLDAPConnection();
     virtual ~nsLDAPConnection();
 
   protected:
-    // invoke the callback associated with a given message, and possibly 
+    // invoke the callback associated with a given message, and possibly
     // delete it from the connection queue
     //
-    nsresult InvokeMessageCallback(LDAPMessage *aMsgHandle, 
+    nsresult InvokeMessageCallback(LDAPMessage *aMsgHandle,
                                    nsILDAPMessage *aMsg,
+                                   PRInt32 aOperation,
                                    PRBool aRemoveOpFromConnQ);
-    /** 
+    /**
      * Add an nsILDAPOperation to the list of operations pending on
      * this connection.  This is mainly intended for use by the
      * nsLDAPOperation code.  Used so that the thread waiting on messages
      * for this connection has an operation to callback to.
      *
      * @param aOperation                    operation to add
      * @exception NS_ERROR_ILLEGAL_VALUE    aOperation was NULL
      * @exception NS_ERROR_UNEXPECTED       this operation's msgId was not
      *                                      unique to this connection
      * @exception NS_ERROR_OUT_OF_MEMORY    out of memory
      */
-    nsresult AddPendingOperation(nsILDAPOperation *aOperation);
+  nsresult AddPendingOperation(PRUint32 aOperationID, nsILDAPOperation *aOperation);
 
     /**
      * Remove an nsILDAPOperation from the list of operations pending on this
      * connection.  Mainly intended for use by the nsLDAPOperation code.
      *
      * @param aOperation        operation to add
      * @exception NS_ERROR_INVALID_POINTER  aOperation was NULL
      * @exception NS_ERROR_OUT_OF_MEMORY    out of memory
-     * @exception NS_ERROR_FAILURE          could not delete the operation 
+     * @exception NS_ERROR_FAILURE          could not delete the operation
      */
-    nsresult RemovePendingOperation(nsILDAPOperation *aOperation);
+    nsresult RemovePendingOperation(PRUint32 aOperationID);
 
     void Close();                       // close the connection
     LDAP *mConnectionHandle;            // the LDAP C SDK's connection object
     nsCString mBindName;                // who to bind as
     nsCOMPtr<nsIThread> mThread;        // thread which marshals results
 
-    nsSupportsHashtable *mPendingOperations; // keep these around for callbacks
-    nsLDAPConnectionLoop *mRunnable;    // nsIRunnable object
+    nsInterfaceHashtableMT<nsUint32HashKey, nsILDAPOperation> mPendingOperations;
 
     PRInt32 mPort;                      // The LDAP port we're binding to
     PRBool mSSL;                        // the options
     PRUint32 mVersion;                  // LDAP protocol version
 
     nsCString mResolvedIP;              // Preresolved list of host IPs
     nsCOMPtr<nsILDAPMessageListener> mInitListener; // Init callback
     nsCOMPtr<nsICancelable> mDNSRequest;   // The "active" DNS request
     nsCString               mDNSHost;   // The hostname being resolved
     nsCOMPtr<nsISupports> mClosure;     // private parameter (anything caller desires)
 };
 
-// This class implements the nsIRunnable interface, in this case just a
-// Run() method. This is to be used within the nsLDAPConnection only, when
-// creating a new thread.
-//
-class nsLDAPConnectionLoop : public nsIRunnable
+class nsLDAPConnectionRunnable : public nsIRunnable
 {
-    friend class nsLDAPConnection;
-    friend class nsLDAPMessage;
+  friend class nsLDAPConnection;
+  friend class nsLDAPMessage;
 
-  public:
-    NS_DECL_ISUPPORTS
-    NS_DECL_NSIRUNNABLE
+public:
+  nsLDAPConnectionRunnable(PRInt32 aOperationID,
+                           nsILDAPOperation *aOperation,
+                           nsLDAPConnection *aConnection);
+  virtual ~nsLDAPConnectionRunnable();
 
-    // constructor & destructor
-    //
-    nsLDAPConnectionLoop();
-    virtual ~nsLDAPConnectionLoop();
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIRUNNABLE
 
-    NS_IMETHOD Init();
-
-    nsWeakPtr mWeakConn;        // the connection object, a weak reference
-    nsLDAPConnection *mRawConn; // raw version of the connection object ptr
-    PRLock *mLock;              // Lock mechanism, since weak references
-                                // aren't thread safe
+  PRInt32 mOperationID;
+  nsRefPtr<nsLDAPConnection> mConnection;
 };
 
-#endif // _nsLDAPConnection_h_ 
+#endif // _nsLDAPConnection_h_
--- a/ldap/xpcom/src/nsLDAPMessage.h
+++ b/ldap/xpcom/src/nsLDAPMessage.h
@@ -1,10 +1,10 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * 
+ *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -51,48 +51,45 @@
 #define NS_LDAPMESSAGE_CID \
 { 0x76e061ad, 0xa59f, 0x43b6, \
   { 0xb8, 0x12, 0xee, 0x6e, 0x8e, 0x69, 0x42, 0x3f }}
 
 class nsLDAPMessage : public nsILDAPMessage
 {
     friend class nsLDAPOperation;
     friend class nsLDAPConnection;
-    friend class nsLDAPConnectionLoop;
-    friend PRBool CheckLDAPOperationResult(nsHashKey *aKey,
-                                           void *aData,
-                                           void* aClosure);
+    friend class nsLDAPConnectionRunnable;
 
-  public:       
+  public:
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSILDAPMESSAGE
 
     // constructor & destructor
     //
     nsLDAPMessage();
     virtual ~nsLDAPMessage();
 
   protected:
-    nsresult IterateAttrErrHandler(PRInt32 aLderrno, PRUint32 *aAttrCount, 
+    nsresult IterateAttrErrHandler(PRInt32 aLderrno, PRUint32 *aAttrCount,
                             char** *aAttributes, BerElement *position);
-    nsresult IterateAttributes(PRUint32 *aAttrCount, char** *aAttributes, 
+    nsresult IterateAttributes(PRUint32 *aAttrCount, char** *aAttributes,
                               PRBool getP);
-    nsresult Init(nsILDAPConnection *aConnection, 
+    nsresult Init(nsILDAPConnection *aConnection,
                   LDAPMessage *aMsgHandle);
     LDAPMessage *mMsgHandle; // the message we're wrapping
     nsCOMPtr<nsILDAPOperation> mOperation;  // operation this msg relates to
 
     LDAP *mConnectionHandle; // cached connection this op is on
 
-    // since we're caching the connection handle (above), we need to 
+    // since we're caching the connection handle (above), we need to
     // hold an owning ref to the relevant nsLDAPConnection object as long
     // as we're around
     //
-    nsCOMPtr<nsILDAPConnection> mConnection; 
+    nsCOMPtr<nsILDAPConnection> mConnection;
 
     // the next five member vars are returned by ldap_parse_result()
     //
     int mErrorCode;
     char *mMatchedDn;
     char *mErrorMessage;
     char **mReferrals;
     LDAPControl **mServerControls;
--- a/ldap/xpcom/src/nsLDAPOperation.cpp
+++ b/ldap/xpcom/src/nsLDAPOperation.cpp
@@ -1,10 +1,10 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
- * 
+ *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -36,17 +36,16 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsLDAPInternal.h"
 #include "nsLDAPOperation.h"
 #include "nsLDAPBERValue.h"
-#include "nsLDAPConnection.h"
 #include "nsILDAPMessage.h"
 #include "nsILDAPModification.h"
 #include "nsIComponentManager.h"
 #include "nsReadableUtils.h"
 #include "nspr.h"
 #include "nsISimpleEnumerator.h"
 #include "nsLDAPControl.h"
 #include "nsILDAPErrors.h"
@@ -109,20 +108,20 @@ NS_IMPL_THREADSAFE_RELEASE(nsLDAPOperati
 NS_INTERFACE_MAP_BEGIN(nsLDAPOperation)
   NS_INTERFACE_MAP_ENTRY(nsILDAPOperation)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsILDAPOperation)
   NS_IMPL_QUERY_CLASSINFO(nsLDAPOperation)
 NS_INTERFACE_MAP_END_THREADSAFE
 NS_IMPL_CI_INTERFACE_GETTER1(nsLDAPOperation, nsILDAPOperation)
 
 /**
- * Initializes this operation.  Must be called prior to use. 
+ * Initializes this operation.  Must be called prior to use.
  *
  * @param aConnection connection this operation should use
- * @param aMessageListener where are the results are called back to. 
+ * @param aMessageListener where are the results are called back to.
  */
 NS_IMETHODIMP
 nsLDAPOperation::Init(nsILDAPConnection *aConnection,
                       nsILDAPMessageListener *aMessageListener,
                       nsISupports *aClosure)
 {
     if (!aConnection) {
         return NS_ERROR_ILLEGAL_VALUE;
@@ -130,24 +129,24 @@ nsLDAPOperation::Init(nsILDAPConnection 
 
     // so we know that the operation is not yet running (and therefore don't
     // try and call ldap_abandon_ext() on it) or remove it from the queue.
     //
     mMsgID = 0;
 
     // set the member vars
     //
-    mConnection = aConnection;
+    mConnection = static_cast<nsLDAPConnection*>(aConnection);
     mMessageListener = aMessageListener;
     mClosure = aClosure;
 
     // cache the connection handle
     //
-    mConnectionHandle = 
-        static_cast<nsLDAPConnection *>(aConnection)->mConnectionHandle;
+    mConnectionHandle =
+        mConnection->mConnectionHandle;
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLDAPOperation::GetClosure(nsISupports **_retval)
 {
     if (!_retval) {
@@ -194,159 +193,153 @@ nsLDAPOperation::GetMessageListener(nsIL
 
     *aMessageListener = mMessageListener;
     NS_IF_ADDREF(*aMessageListener);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsLDAPOperation::SaslBind(const nsACString &service, 
-                          const nsACString &mechanism, 
+nsLDAPOperation::SaslBind(const nsACString &service,
+                          const nsACString &mechanism,
                           nsIAuthModule *authModule)
 {
   nsresult rv;
   nsCAutoString bindName;
   struct berval creds;
   unsigned int credlen;
 
   mAuthModule = authModule;
   mMechanism.Assign(mechanism);
 
   rv = mConnection->GetBindName(bindName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   creds.bv_val = NULL;
-  mAuthModule->Init(PromiseFlatCString(service).get(), 
-                    nsIAuthModule::REQ_DEFAULT, nsnull, 
+  mAuthModule->Init(PromiseFlatCString(service).get(),
+                    nsIAuthModule::REQ_DEFAULT, nsnull,
                     NS_ConvertUTF8toUTF16(bindName).get(), nsnull);
 
-  rv = mAuthModule->GetNextToken(nsnull, 0, (void **)&creds.bv_val, 
+  rv = mAuthModule->GetNextToken(nsnull, 0, (void **)&creds.bv_val,
                                  &credlen);
   if (NS_FAILED(rv) || !creds.bv_val)
     return rv;
 
   creds.bv_len = credlen;
   const int lderrno = ldap_sasl_bind(mConnectionHandle, bindName.get(),
                                      mMechanism.get(), &creds, NULL, NULL,
                                      &mMsgID);
   nsMemory::Free(creds.bv_val);
 
   if (lderrno != LDAP_SUCCESS)
     return TranslateLDAPErrorToNSError(lderrno);
-  
+
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>
-       (static_cast<nsILDAPConnection *>(mConnection.get()))
-       ->AddPendingOperation(this);
+  rv = mConnection->AddPendingOperation(mMsgID, this);
 
   if (NS_FAILED(rv))
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsLDAPOperation::SaslStep(const char *token, PRUint32 tokenLen)
 {
   nsresult rv;
   nsCAutoString bindName;
   struct berval clientCreds;
   struct berval serverCreds;
   unsigned int credlen;
 
-  rv = static_cast<nsLDAPConnection *>
-       (static_cast<nsILDAPConnection *>(mConnection.get()))
-       ->RemovePendingOperation(this);
+  rv = mConnection->RemovePendingOperation(mMsgID);
   NS_ENSURE_SUCCESS(rv, rv);
 
   serverCreds.bv_val = (char *) token;
   serverCreds.bv_len = tokenLen;
 
   rv = mConnection->GetBindName(bindName);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = mAuthModule->GetNextToken(serverCreds.bv_val, serverCreds.bv_len, 
+  rv = mAuthModule->GetNextToken(serverCreds.bv_val, serverCreds.bv_len,
                                  (void **) &clientCreds.bv_val, &credlen);
   NS_ENSURE_SUCCESS(rv, rv);
 
   clientCreds.bv_len = credlen;
 
-  const int lderrno = ldap_sasl_bind(mConnectionHandle, bindName.get(), 
-                                     mMechanism.get(), &clientCreds, NULL, 
+  const int lderrno = ldap_sasl_bind(mConnectionHandle, bindName.get(),
+                                     mMechanism.get(), &clientCreds, NULL,
                                      NULL, &mMsgID);
 
   nsMemory::Free(clientCreds.bv_val);
 
   if (lderrno != LDAP_SUCCESS)
     return TranslateLDAPErrorToNSError(lderrno);
 
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>
-       (static_cast<nsILDAPConnection *>(mConnection.get()))
-       ->AddPendingOperation(this);
-  if (NS_FAILED(rv)) 
+  rv = mConnection->AddPendingOperation(mMsgID, this);
+  if (NS_FAILED(rv))
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
 
   return rv;
 }
 
 
 // wrapper for ldap_simple_bind()
 //
 NS_IMETHODIMP
 nsLDAPOperation::SimpleBind(const nsACString& passwd)
 {
     nsresult rv;
     nsCAutoString bindName;
     PRBool originalMsgID = mMsgID;
     // Ugly hack alert:
     // the first time we get called with a passwd, remember it.
-    // Then, if we get called again w/o a password, use the 
+    // Then, if we get called again w/o a password, use the
     // saved one. Getting called again means we're trying to
     // fall back to VERSION2.
     // Since LDAP operations are thrown away when done, it won't stay
     // around in memory.
     if (!passwd.IsEmpty())
       mSavePassword = passwd;
 
     NS_PRECONDITION(mMessageListener != 0, "MessageListener not set");
 
     rv = mConnection->GetBindName(bindName);
     if (NS_FAILED(rv))
         return rv;
 
-    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
+    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::SimpleBind(): called; bindName = '%s'; ",
             bindName.get()));
 
     // If this is a second try at binding, remove the operation from pending ops
     // because msg id has changed...
     if (originalMsgID)
-      static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>(mConnection.get()))->RemovePendingOperation(this);
+      mConnection->RemovePendingOperation(originalMsgID);
 
     mMsgID = ldap_simple_bind(mConnectionHandle, bindName.get(),
                               PromiseFlatCString(mSavePassword).get());
 
     if (mMsgID == -1) {
       // XXX Should NS_ERROR_LDAP_SERVER_DOWN cause a rebind here?
       return TranslateLDAPErrorToNSError(ldap_get_lderrno(mConnectionHandle,
                                                           0, 0));
-    } 
-  
+    }
+
     // make sure the connection knows where to call back once the messages
     // for this operation start coming in
-    rv = static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>(mConnection.get()))->AddPendingOperation(this);
+    rv = mConnection->AddPendingOperation(mMsgID, this);
     switch (rv) {
     case NS_OK:
         break;
 
-        // note that the return value of ldap_abandon_ext() is ignored, as 
+        // note that the return value of ldap_abandon_ext() is ignored, as
         // there's nothing useful to do with it
 
     case NS_ERROR_OUT_OF_MEMORY:
         (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
         return NS_ERROR_OUT_OF_MEMORY;
         break;
 
     case NS_ERROR_UNEXPECTED:
@@ -376,17 +369,17 @@ convertControlArray(nsIArray *aXpcomArra
         *aArray = 0;
         return NS_OK;
     }
 
     // allocate a local array of the form understood by the C-SDK;
     // +1 is to account for the final null terminator.  PR_Calloc is
     // is used so that ldap_controls_free will work anywhere during the
     // iteration
-    LDAPControl **controls = 
+    LDAPControl **controls =
         static_cast<LDAPControl **>
                    (PR_Calloc(length+1, sizeof(LDAPControl)));
 
     // prepare to enumerate the array
     nsCOMPtr<nsISimpleEnumerator> enumerator;
     rv = aXpcomArray->Enumerate(getter_AddRefs(enumerator));
     NS_ENSURE_SUCCESS(rv, rv);
 
@@ -429,30 +422,30 @@ convertControlArray(nsIArray *aXpcomArra
         ++i;
     }
 
     *aArray = controls;
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsLDAPOperation::SearchExt(const nsACString& aBaseDn, PRInt32 aScope, 
-                           const nsACString& aFilter, 
+nsLDAPOperation::SearchExt(const nsACString& aBaseDn, PRInt32 aScope,
+                           const nsACString& aFilter,
                            PRUint32 aAttrCount, const char **aAttributes,
-                           PRIntervalTime aTimeOut, PRInt32 aSizeLimit) 
+                           PRIntervalTime aTimeOut, PRInt32 aSizeLimit)
 {
     if (!mMessageListener) {
         NS_ERROR("nsLDAPOperation::SearchExt(): mMessageListener not set");
         return NS_ERROR_NOT_INITIALIZED;
     }
 
     // XXX add control logging
-    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
+    PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::SearchExt(): called with aBaseDn = '%s'; "
-            "aFilter = '%s', aAttrCounts = %u, aSizeLimit = %d", 
+            "aFilter = '%s', aAttrCounts = %u, aSizeLimit = %d",
             PromiseFlatCString(aBaseDn).get(),
             PromiseFlatCString(aFilter).get(),
             aAttrCount, aSizeLimit));
 
     char **attrs = 0;
 
     // Convert our XPCOM style C-Array to one that the C-SDK will like, i.e.
     // add a last NULL element.
@@ -496,43 +489,43 @@ nsLDAPOperation::SearchExt(const nsACStr
                 nsMemory::Free(attrs);
             }
             ldap_controls_free(serverctls);
             return rv;
         }
     }
 
     // XXX deal with timeout here
-    int retVal = ldap_search_ext(mConnectionHandle, 
+    int retVal = ldap_search_ext(mConnectionHandle,
                                  PromiseFlatCString(aBaseDn).get(),
-                                 aScope, PromiseFlatCString(aFilter).get(), 
+                                 aScope, PromiseFlatCString(aFilter).get(),
                                  attrs, 0, serverctls, clientctls, 0,
                                  aSizeLimit, &mMsgID);
 
     // clean up
-    ldap_controls_free(serverctls);        
+    ldap_controls_free(serverctls);
     ldap_controls_free(clientctls);
     if (attrs) {
         nsMemory::Free(attrs);
     }
 
     rv = TranslateLDAPErrorToNSError(retVal);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // make sure the connection knows where to call back once the messages
     // for this operation start coming in
     //
-    rv = static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>(mConnection.get()))->AddPendingOperation(this);
+    rv = mConnection->AddPendingOperation(mMsgID, this);
     if (NS_FAILED(rv)) {
         switch (rv) {
-        case NS_ERROR_OUT_OF_MEMORY: 
+        case NS_ERROR_OUT_OF_MEMORY:
             (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
             return NS_ERROR_OUT_OF_MEMORY;
 
-        default: 
+        default:
             (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
             NS_ERROR("nsLDAPOperation::SearchExt(): unexpected error in "
                      "mConnection->AddPendingOperation");
             return NS_ERROR_UNEXPECTED;
         }
     }
 
     return NS_OK;
@@ -541,17 +534,17 @@ nsLDAPOperation::SearchExt(const nsACStr
 NS_IMETHODIMP
 nsLDAPOperation::GetMessageID(PRInt32 *aMsgID)
 {
     if (!aMsgID) {
         return NS_ERROR_ILLEGAL_VALUE;
     }
 
     *aMsgID = mMsgID;
-   
+
     return NS_OK;
 }
 
 // as far as I can tell from reading the LDAP C SDK code, abandoning something
 // that has already been abandoned does not return an error
 //
 NS_IMETHODIMP
 nsLDAPOperation::AbandonExt()
@@ -571,31 +564,31 @@ nsLDAPOperation::AbandonExt()
     }
 
     rv = TranslateLDAPErrorToNSError(ldap_abandon_ext(mConnectionHandle,
                                                       mMsgID, 0, 0));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // try to remove it from the pendingOperations queue, if it's there.
     // even if something goes wrong here, the abandon() has already succeeded
-    // succeeded (and there's nothing else the caller can reasonably do), 
+    // succeeded (and there's nothing else the caller can reasonably do),
     // so we only pay attention to this in debug builds.
     //
-    // check mConnection in case we're getting bit by 
-    // http://bugzilla.mozilla.org/show_bug.cgi?id=239729, wherein we 
+    // check mConnection in case we're getting bit by
+    // http://bugzilla.mozilla.org/show_bug.cgi?id=239729, wherein we
     // theorize that ::Clearing the operation is nulling out the mConnection
     // from another thread.
     if (mConnection)
     {
-      rv = static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>(mConnection.get()))->RemovePendingOperation(this);
+      rv = mConnection->RemovePendingOperation(mMsgID);
 
       if (NS_FAILED(rv)) {
-          // XXXdmose should we keep AbandonExt from happening on multiple 
-          // threads at the same time?  that's when this condition is most 
-          // likely to occur.  i _think_ the LDAP C SDK is ok with this; need 
+          // XXXdmose should we keep AbandonExt from happening on multiple
+          // threads at the same time?  that's when this condition is most
+          // likely to occur.  i _think_ the LDAP C SDK is ok with this; need
           // to verify.
           //
           NS_WARNING("nsLDAPOperation::AbandonExt: "
                      "mConnection->RemovePendingOperation(this) failed.");
       }
     }
 
     return retStatus;
@@ -726,18 +719,17 @@ nsLDAPOperation::AddExt(const nsACString
           PromiseFlatCString(aBaseDn).get()));
 
   nsresult rv = AddExt(PromiseFlatCString(aBaseDn).get(), aMods, 0, 0);
   if (NS_FAILED(rv))
     return rv;
 
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>(static_cast<nsILDAPConnection *>
-                              (mConnection.get()))->AddPendingOperation(this);
+  rv = mConnection->AddPendingOperation(mMsgID, this);
 
   if (NS_FAILED(rv)) {
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::AddExt(): abandoned due to rv %x",
             rv));
   }
   return rv;
@@ -777,19 +769,17 @@ nsLDAPOperation::DeleteExt(const nsACStr
           PromiseFlatCString(aBaseDn).get()));
 
   nsresult rv = DeleteExt(PromiseFlatCString(aBaseDn).get(), 0, 0);
   if (NS_FAILED(rv))
     return rv;
 
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>
-                  (static_cast<nsILDAPConnection *>
-                              (mConnection.get()))->AddPendingOperation(this);
+  rv = mConnection->AddPendingOperation(mMsgID, this);
 
   if (NS_FAILED(rv)) {
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::AddExt(): abandoned due to rv %x",
             rv));
   }
   return rv;
@@ -826,22 +816,22 @@ nsLDAPOperation::ModifyExt(const char *b
     for (index = 0; index < modCount && NS_SUCCEEDED(rv); ++index) {
       attrs[index] = new LDAPMod();
       if (!attrs[index])
         return NS_ERROR_OUT_OF_MEMORY;
 
       nsCOMPtr<nsILDAPModification> modif(do_QueryElementAt(mods, index, &rv));
       if (NS_FAILED(rv))
         break;
-      
+
       PRInt32 operation;
       nsresult rv = modif->GetOperation(&operation);
       if (NS_FAILED(rv))
         break;
-      
+
       attrs[index]->mod_op = operation | LDAP_MOD_BVALUES;
 
       rv = modif->GetType(type);
       if (NS_FAILED(rv))
         break;
 
       attrs[index]->mod_type = ToNewCString(type);
 
@@ -892,19 +882,17 @@ nsLDAPOperation::ModifyExt(const nsACStr
 
   nsresult rv = ModifyExt(PromiseFlatCString(aBaseDn).get(),
                           aMods, 0, 0);
   if (NS_FAILED(rv))
     return rv;
 
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>
-                  (static_cast<nsILDAPConnection *>
-                              (mConnection.get()))->AddPendingOperation(this);
+  rv = mConnection->AddPendingOperation(mMsgID, this);
 
   if (NS_FAILED(rv)) {
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::AddExt(): abandoned due to rv %x",
             rv));
   }
   return rv;
@@ -957,19 +945,17 @@ nsLDAPOperation::Rename(const nsACString
                        PromiseFlatCString(aNewRDn).get(),
                        PromiseFlatCString(aNewParent).get(),
                        aDeleteOldRDn, 0, 0);
   if (NS_FAILED(rv))
     return rv;
 
   // make sure the connection knows where to call back once the messages
   // for this operation start coming in
-  rv = static_cast<nsLDAPConnection *>
-                  (static_cast<nsILDAPConnection *>
-                              (mConnection.get()))->AddPendingOperation(this);
+  rv = mConnection->AddPendingOperation(mMsgID, this);
 
   if (NS_FAILED(rv)) {
     (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("nsLDAPOperation::AddExt(): abandoned due to rv %x",
             rv));
   }
   return rv;
@@ -997,17 +983,17 @@ nsLDAPOperation::CopyValues(nsILDAPModif
     return NS_ERROR_OUT_OF_MEMORY;
 
   PRUint32 valueIndex;
   for (valueIndex = 0; valueIndex < valuesCount; ++valueIndex) {
     nsCOMPtr<nsILDAPBERValue> value(do_QueryElementAt(values, valueIndex, &rv));
 
     berval* bval = new berval;
     if (NS_FAILED(rv) || !bval) {
-      for (PRUint32 counter = 0; 
+      for (PRUint32 counter = 0;
            counter < valueIndex && counter < valuesCount;
            ++counter)
         delete (*aBValues)[valueIndex];
 
       nsMemory::Free(*aBValues);
       delete bval;
       return NS_ERROR_OUT_OF_MEMORY;
     }
--- a/ldap/xpcom/src/nsLDAPOperation.h
+++ b/ldap/xpcom/src/nsLDAPOperation.h
@@ -43,16 +43,17 @@
 
 #include "ldap.h"
 #include "nsCOMPtr.h"
 #include "nsILDAPConnection.h"
 #include "nsILDAPOperation.h"
 #include "nsILDAPMessageListener.h"
 #include "nsString.h"
 #include "nsIMutableArray.h"
+#include "nsLDAPConnection.h"
 
 // 97a479d0-9a44-47c6-a17a-87f9b00294bb
 #define NS_LDAPOPERATION_CID \
 { 0x97a479d0, 0x9a44, 0x47c6, \
   { 0xa1, 0x7a, 0x87, 0xf9, 0xb0, 0x02, 0x94, 0xbb}}
 
 class nsLDAPOperation : public nsILDAPOperation
 {
@@ -116,17 +117,17 @@ class nsLDAPOperation : public nsILDAPOp
     /**
      * Helper function to copy the values of an nsILDAPModification into an
      * array of berval's.
      */
     static nsresult CopyValues(nsILDAPModification* aMod, berval*** aBValues);
 
     nsCOMPtr<nsILDAPMessageListener> mMessageListener; // results go here
     nsCOMPtr<nsISupports> mClosure;  // private parameter (anything caller desires)
-    nsCOMPtr<nsILDAPConnection> mConnection; // connection this op is on
+    nsRefPtr<nsLDAPConnection> mConnection; // connection this op is on
 
     LDAP *mConnectionHandle; // cache connection handle
     nsCString mSavePassword;
     nsCString mMechanism;
     nsCOMPtr<nsIAuthModule> mAuthModule;
     PRInt32 mMsgID;          // opaque handle to outbound message for this op
 
     nsCOMPtr<nsIMutableArray> mClientControls;