Bug 1576364 - Dispatch LDAP operations on socket thread. r=kaie
authorBen Campbell <benc@thunderbird.net>
Wed, 06 Nov 2019 01:58:00 +0100
changeset 36392 d040a773e7ad648eaca582cb85a88c61fef82d52
parent 36391 056d8cc39c8b91c735da616caee4335e5f3d7c4d
child 36393 d4f11632f5ea1773c4ed6fe3d09a20e79270fede
push id2523
push usermozilla@jorgk.com
push dateThu, 07 Nov 2019 13:59:32 +0000
treeherdercomm-beta@4c356f38873a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskaie
bugs1576364
Bug 1576364 - Dispatch LDAP operations on socket thread. r=kaie
ldap/xpcom/src/nsLDAPConnection.cpp
ldap/xpcom/src/nsLDAPConnection.h
ldap/xpcom/src/nsLDAPOperation.cpp
ldap/xpcom/src/nsLDAPOperation.h
ldap/xpcom/src/nsLDAPSecurityGlue.cpp
--- a/ldap/xpcom/src/nsLDAPConnection.cpp
+++ b/ldap/xpcom/src/nsLDAPConnection.cpp
@@ -22,16 +22,17 @@
 #include "nsIClassInfoImpl.h"
 #include "nsILDAPURL.h"
 #include "nsIObserverService.h"
 #include "mozilla/Services.h"
 #include "nsMemory.h"
 #include "nsLDAPUtils.h"
 #include "nsProxyRelease.h"
 #include "mozilla/Attributes.h"
+#include "nsNetUtil.h"
 
 using namespace mozilla;
 
 const char kDNSServiceContractId[] = "@mozilla.org/network/dns-service;1";
 
 // constructor
 //
 nsLDAPConnection::nsLDAPConnection()
@@ -71,16 +72,21 @@ NS_IMPL_CI_INTERFACE_GETTER(nsLDAPConnec
 NS_IMETHODIMP
 nsLDAPConnection::Init(nsILDAPURL *aUrl, const nsACString &aBindName,
                        nsILDAPMessageListener *aMessageListener,
                        nsISupports *aClosure, uint32_t aVersion) {
   NS_ENSURE_ARG_POINTER(aUrl);
   NS_ENSURE_ARG_POINTER(aMessageListener);
 
   nsresult rv;
+
+  // Cache the STS thread we'll use to dispatch IO on.
+  mSTS = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsCOMPtr<nsIObserverService> obsServ =
       do_GetService(NS_OBSERVERSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
   // We have to abort all LDAP pending operation before shutdown.
   obsServ->AddObserver(this, "profile-change-net-teardown", true);
 
   // Save various items that we'll use later
   mBindName.Assign(aBindName);
@@ -183,19 +189,16 @@ void nsLDAPConnection::Close() {
       MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Warning,
               ("nsLDAPConnection::Close(): %s", ldap_err2string(rc)));
     }
     mConnectionHandle = nullptr;
   }
 
   MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug, ("unbound"));
 
-  NS_ASSERTION(!mThread || 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 = nullptr;
   }
   mInitListener = nullptr;
@@ -297,38 +300,48 @@ nsLDAPConnection::GetErrorString(char16_
 
 /**
  * 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(uint32_t aOperationID,
                                                nsILDAPOperation *aOperation) {
-  NS_ENSURE_ARG_POINTER(aOperation);
+  MOZ_ASSERT(aOperation != nullptr);
 
   nsIRunnable *runnable =
       new nsLDAPConnectionRunnable(aOperationID, aOperation, this);
   {
     MutexAutoLock lock(mPendingOperationsMutex);
     mPendingOperations.Put((uint32_t)aOperationID, aOperation);
     MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
             ("pending operation added; total pending operations now = %d",
              mPendingOperations.Count()));
   }
 
   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);
+  rv = mSTS->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
+  if (rv != NS_OK) {
+    RemovePendingOperation(aOperationID);
+    // Tell the server we want the operation cancelled, and also
+    // ditch any responses we've already received but not collected.
+    (void)ldap_abandon_ext(mConnectionHandle, aOperationID, 0, 0);
+    // At this point we should be letting the listener know that there's
+    // an error, but listener doesn't have a suitable callback.
+    // See Bug 1592449.
+    // For now, just log it and leave it at that.
+    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Error,
+            ("nsLDAPConnection::AddPendingOperation() failed, rv=%" PRIx32,
+             static_cast<uint32_t>(rv)));
   }
+  return rv;
+}
 
-  return NS_OK;
+nsresult nsLDAPConnection::StartOp(nsIRunnable *aOp) {
+  return mSTS->Dispatch(aOp, nsIEventTarget::DISPATCH_NORMAL);
 }
 
 /**
  * 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
@@ -507,16 +520,17 @@ nsLDAPConnection::OnLookupComplete(nsICa
         ldap_init(mResolvedIP.get(),
                   mPort == -1 ? (mSSL ? LDAPS_PORT : LDAP_PORT) : mPort);
     // Check that we got a proper connection, and if so, setup the
     // threading functions for this connection.
     //
     if (!mConnectionHandle) {
       rv = NS_ERROR_FAILURE;  // LDAP C SDK API gives no useful error
     } else {
+      ldap_set_option(mConnectionHandle, LDAP_OPT_ASYNC_CONNECT, LDAP_OPT_ON);
 #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,
       // tell it so.
       //
--- a/ldap/xpcom/src/nsLDAPConnection.h
+++ b/ldap/xpcom/src/nsLDAPConnection.h
@@ -58,23 +58,16 @@ class nsLDAPConnection : public nsILDAPC
   NS_DECL_NSILDAPCONNECTION
   NS_DECL_NSIDNSLISTENER
   NS_DECL_NSIOBSERVER
 
   // constructor & destructor
   //
   nsLDAPConnection();
 
- protected:
-  virtual ~nsLDAPConnection();
-  // invoke the callback associated with a given message, and possibly
-  // delete it from the connection queue
-  //
-  nsresult InvokeMessageCallback(LDAPMessage *aMsgHandle, nsILDAPMessage *aMsg,
-                                 int32_t aOperation, bool 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
@@ -91,20 +84,41 @@ class nsLDAPConnection : public nsILDAPC
    *
    * @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
    */
   nsresult RemovePendingOperation(uint32_t 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
+ protected:
+  virtual ~nsLDAPConnection();
+
+  /** invoke the callback associated with a given message, and possibly
+   * delete it from the connection queue.
+   */
+  nsresult InvokeMessageCallback(LDAPMessage *aMsgHandle, nsILDAPMessage *aMsg,
+                                 int32_t aOperation, bool aRemoveOpFromConnQ);
+
+  /**
+   * Dispatch an operation to the socket thread. This is intended for use by
+   * the nsLDAPOperation code.
+   */
+  nsresult StartOp(nsIRunnable *aOp);
+
+  void Close();             // close the connection
+  LDAP *mConnectionHandle;  // the LDAP C SDK's connection object
+  nsCString mBindName;      // who to bind as
+
+  // We'll be dispatching operations on the SocketTransportService. This is
+  // because there might be some SSL/TLS security handshaking happening
+  // under the hood (the handshaking is deferred until the first IO Send).
+  // The handshaking expects to be running on the STS thread (see Bug 1576364).
+  // It also saves us spinning up a new thread to handle LDAP IO.
+  nsCOMPtr<nsIEventTarget> mSTS;
 
   Mutex mPendingOperationsMutex;
   nsInterfaceHashtable<nsUint32HashKey, nsILDAPOperation> mPendingOperations;
 
   int32_t mPort;      // The LDAP port we're binding to
   bool mSSL;          // the options
   uint32_t mVersion;  // LDAP protocol version
 
--- a/ldap/xpcom/src/nsLDAPOperation.cpp
+++ b/ldap/xpcom/src/nsLDAPOperation.cpp
@@ -13,16 +13,55 @@
 #include "nspr.h"
 #include "nsISimpleEnumerator.h"
 #include "nsLDAPControl.h"
 #include "nsILDAPErrors.h"
 #include "nsIClassInfoImpl.h"
 #include "nsIAuthModule.h"
 #include "nsArrayUtils.h"
 #include "nsMemory.h"
+#include "nsThreadUtils.h"
+
+// Declare helper fns for dealing with C++ LDAP <-> libldap mismatch.
+static nsresult convertValues(nsIArray *values, berval ***aBValues);
+static void freeValues(berval **aVals);
+static nsresult convertMods(nsIArray *aMods, LDAPMod ***aOut);
+static void freeMods(LDAPMod **aMods);
+static nsresult convertControlArray(nsIArray *aXpcomArray,
+                                    LDAPControl ***aArray);
+
+/**
+ * OpRunnable is a helper class to dispatch ldap operations on the socket
+ * thread.
+ */
+class OpRunnable : public mozilla::Runnable {
+ public:
+  OpRunnable(const char *name, nsLDAPOperation *aOperation)
+      : mozilla::Runnable(name), mOp(aOperation) {}
+  RefPtr<nsLDAPOperation> mOp;
+
+ protected:
+  virtual ~OpRunnable() {}
+
+  // Provide access to protected members we need in nsLDAPOperation, without
+  // declaring every individual Runnable as a friend class.
+  LDAP *LDAPHandle() { return mOp->mConnectionHandle; }
+  void SetID(int32_t id) { mOp->mMsgID = id; }
+  nsLDAPConnection *Conn() { return mOp->mConnection; }
+
+  void NotifyLDAPError() {
+    // At this point we should be letting the listener know that there's
+    // an error, but listener doesn't have a suitable callback.
+    // See Bug 1592449.
+    // For now, just log it and leave it at that.
+    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Error,
+            ("nsLDAPOperation failed id=%d, lderrno=%d", mOp->mMsgID,
+             ldap_get_lderrno(LDAPHandle(), 0, 0)));
+  }
+};
 
 // Helper function
 static nsresult TranslateLDAPErrorToNSError(const int ldapError) {
   switch (ldapError) {
     case LDAP_SUCCESS:
       return NS_OK;
 
     case LDAP_ENCODING_ERROR:
@@ -145,56 +184,88 @@ nsLDAPOperation::GetMessageListener(nsIL
   }
 
   *aMessageListener = mMessageListener;
   NS_IF_ADDREF(*aMessageListener);
 
   return NS_OK;
 }
 
+/**
+ * SaslBindRunnable - wraps up an ldap_sasl_bind operation so it can
+ * be dispatched to the socket thread.
+ */
+class SaslBindRunnable : public OpRunnable {
+ public:
+  SaslBindRunnable(nsLDAPOperation *aOperation, const nsACString &bindName,
+                   const nsACString &mechanism, uint8_t *credData,
+                   unsigned int credLen)
+      : OpRunnable("SaslBindRunnable", aOperation),
+        mBindName(bindName),
+        mMechanism(mechanism) {
+    mCreds.bv_val = (char *)credData;
+    mCreds.bv_len = credLen;
+  }
+  virtual ~SaslBindRunnable() { free(mCreds.bv_val); }
+
+  nsCString mBindName;
+  nsCString mMechanism;
+  BerValue mCreds;
+
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    const int ret =
+        ldap_sasl_bind(LDAPHandle(), mBindName.get(), mMechanism.get(), &mCreds,
+                       NULL, NULL, &msgID);
+    if (ret != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
+  }
+};
+
 NS_IMETHODIMP
 nsLDAPOperation::SaslBind(const nsACString &service,
                           const nsACString &mechanism,
                           nsIAuthModule *authModule) {
   nsresult rv;
   nsAutoCString 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, nullptr,
                     NS_ConvertUTF8toUTF16(bindName).get(), nullptr);
 
-  rv = mAuthModule->GetNextToken(nullptr, 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);
-  free(creds.bv_val);
+  uint8_t *credData = nullptr;
+  unsigned int credLen;
+  rv = mAuthModule->GetNextToken(nullptr, 0, (void **)&credData, &credLen);
+  if (NS_FAILED(rv) || !credData) return rv;
 
-  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 = mConnection->AddPendingOperation(mMsgID, this);
-
-  if (NS_FAILED(rv)) (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-
-  return rv;
+  nsCOMPtr<nsIRunnable> op =
+      new SaslBindRunnable(this, bindName, mMechanism, credData, credLen);
+  mConnection->StartOp(op);
+  return NS_OK;
 }
 
+/**
+ * SaslStep is called by nsLDAPConnection behind the scenes to continue
+ * a SaslBind.
+ * This is called from nsLDAPConnectionRunnable, which will already be running
+ * on the socket thread, so we don't need to do any fancy dispatch stuff here.
+ */
 NS_IMETHODIMP
 nsLDAPOperation::SaslStep(const char *token, uint32_t tokenLen) {
   nsresult rv;
   nsAutoCString bindName;
   struct berval clientCreds;
   struct berval serverCreds;
   unsigned int credlen;
 
@@ -218,21 +289,50 @@ nsLDAPOperation::SaslStep(const char *to
                      &clientCreds, NULL, NULL, &mMsgID);
 
   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 = mConnection->AddPendingOperation(mMsgID, this);
-  if (NS_FAILED(rv)) (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
+  return mConnection->AddPendingOperation(mMsgID, this);
+}
+
+/**
+ * SimpleBindRunnable - wraps up an ldap_simple_bind operation so it can
+ * be dispatched to the socket thread.
+ */
+class SimpleBindRunnable : public OpRunnable {
+ public:
+  SimpleBindRunnable(nsLDAPOperation *aOperation, const nsACString &bindName,
+                     const nsACString &passwd)
+      : OpRunnable("SimpleBindRunnable", aOperation),
+        mBindName(bindName),
+        mPasswd(passwd) {}
+  virtual ~SimpleBindRunnable() {}
 
-  return rv;
-}
+  nsCString mBindName;
+  nsCString mPasswd;
+
+  NS_IMETHOD Run() override {
+    LDAP *ld = LDAPHandle();
+    int32_t msgID = ldap_simple_bind(ld, mBindName.get(), mPasswd.get());
+
+    if (msgID == -1) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
+  }
+};
 
 // wrapper for ldap_simple_bind()
 //
 NS_IMETHODIMP
 nsLDAPOperation::SimpleBind(const nsACString &passwd) {
   RefPtr<nsLDAPConnection> connection = mConnection;
   // There is a possibility that mConnection can be cleared by another
   // thread. Grabbing a local reference to mConnection may avoid this.
@@ -260,48 +360,21 @@ nsLDAPOperation::SimpleBind(const nsACSt
 
   // this (nsLDAPOperation) may be released by RemovePendingOperation()
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=1063829.
   RefPtr<nsLDAPOperation> kungFuDeathGrip = this;
 
   // If this is a second try at binding, remove the operation from pending ops
   // because msg id has changed...
   if (originalMsgID) connection->RemovePendingOperation(originalMsgID);
-
-  mMsgID =
-      ldap_simple_bind(mConnectionHandle, bindName.get(), mSavePassword.get());
-
-  if (mMsgID == -1) {
-    // XXX Should NS_ERROR_LDAP_SERVER_DOWN cause a rebind here?
-    return TranslateLDAPErrorToNSError(
-        ldap_get_lderrno(mConnectionHandle, 0, 0));
-  }
+  mMsgID = 0;
 
-  // make sure the connection knows where to call back once the messages
-  // for this operation start coming in
-  rv = connection->AddPendingOperation(mMsgID, this);
-  switch (rv) {
-    case NS_OK:
-      break;
-
-      // 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:
-    case NS_ERROR_ILLEGAL_VALUE:
-    default:
-      (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-      return NS_ERROR_UNEXPECTED;
-  }
-
+  nsCOMPtr<nsIRunnable> op =
+      new SimpleBindRunnable(this, bindName, mSavePassword);
+  mConnection->StartOp(op);
   return NS_OK;
 }
 
 /**
  * Given an nsIArray of nsILDAPControls, return the appropriate
  * zero-terminated array of LDAPControls ready to pass in to the C SDK.
  */
 static nsresult convertControlArray(nsIArray *aXpcomArray,
@@ -376,16 +449,74 @@ NS_IMETHODIMP nsLDAPOperation::GetReques
   return NS_OK;
 }
 
 NS_IMETHODIMP nsLDAPOperation::SetRequestNum(uint32_t aRequestNum) {
   mRequestNum = aRequestNum;
   return NS_OK;
 }
 
+/**
+ * SearchExtRunnable - wraps up an ldap_search_ext operation so it can
+ * be dispatched to the socket thread.
+ */
+class SearchExtRunnable : public OpRunnable {
+ public:
+  SearchExtRunnable(nsLDAPOperation *aOperation, const nsACString &aBaseDn,
+                    int32_t aScope, const nsACString &aFilter, char **aAttrs,
+                    LDAPControl **aServerctls, LDAPControl **aClientctls,
+                    int32_t aSizeLimit)
+      : OpRunnable("SearchExtRunnable", aOperation),
+        mBaseDn(aBaseDn),
+        mScope(aScope),
+        mFilter(aFilter),
+        mAttrs(aAttrs),
+        mServerctls(aServerctls),
+        mClientctls(aClientctls),
+        mSizeLimit(aSizeLimit) {}
+  virtual ~SearchExtRunnable() {
+    // clean up
+    ldap_controls_free(mServerctls);
+    ldap_controls_free(mClientctls);
+    // The last attr entry is null, so no need to free that.
+    int numAttrs = 0;
+    while (mAttrs[numAttrs]) {
+      ++numAttrs;
+    }
+    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(numAttrs, mAttrs);
+  }
+
+  nsCString mBaseDn;
+  int32_t mScope;
+  nsCString mFilter;
+  char **mAttrs;
+  LDAPControl **mServerctls;
+  LDAPControl **mClientctls;
+  int32_t mSizeLimit;
+
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    LDAP *ld = LDAPHandle();
+    int retVal =
+        ldap_search_ext(ld, PromiseFlatCString(mBaseDn).get(), mScope,
+                        PromiseFlatCString(mFilter).get(), mAttrs, 0,
+                        mServerctls, mClientctls, 0, mSizeLimit, &msgID);
+    // Did the operation succeed?
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
+  }
+};
+
 NS_IMETHODIMP
 nsLDAPOperation::SearchExt(const nsACString &aBaseDn, int32_t aScope,
                            const nsACString &aFilter,
                            const nsACString &aAttributes,
                            PRIntervalTime aTimeOut, int32_t aSizeLimit) {
   if (!mMessageListener) {
     NS_ERROR("nsLDAPOperation::SearchExt(): mMessageListener not set");
     return NS_ERROR_NOT_INITIALIZED;
@@ -437,49 +568,21 @@ nsLDAPOperation::SearchExt(const nsACStr
 
     for (uint32_t i = 0; i < origLength; ++i)
       attrs[i] = ToNewCString(attrArray[i]);
 
     attrs[origLength] = 0;
   }
 
   // XXX deal with timeout here
-  int retVal =
-      ldap_search_ext(mConnectionHandle, PromiseFlatCString(aBaseDn).get(),
-                      aScope, PromiseFlatCString(aFilter).get(), attrs, 0,
-                      serverctls, clientctls, 0, aSizeLimit, &mMsgID);
 
-  // clean up
-  ldap_controls_free(serverctls);
-  ldap_controls_free(clientctls);
-  // The last entry is null, so no need to free that.
-  NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(origLength, 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 = mConnection->AddPendingOperation(mMsgID, this);
-  if (NS_FAILED(rv)) {
-    switch (rv) {
-      case NS_ERROR_OUT_OF_MEMORY:
-        (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-        return NS_ERROR_OUT_OF_MEMORY;
-
-      default:
-        (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-        NS_ERROR(
-            "nsLDAPOperation::SearchExt(): unexpected error in "
-            "mConnection->AddPendingOperation");
-        return NS_ERROR_UNEXPECTED;
-    }
-  }
-
+  nsCOMPtr<nsIRunnable> op =
+      new SearchExtRunnable(this, aBaseDn, aScope, aFilter, attrs, serverctls,
+                            clientctls, aSizeLimit);
+  mConnection->StartOp(op);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLDAPOperation::GetMessageID(int32_t *aMsgID) {
   if (!aMsgID) {
     return NS_ERROR_ILLEGAL_VALUE;
   }
@@ -487,62 +590,82 @@ nsLDAPOperation::GetMessageID(int32_t *a
   *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
 //
+
+/**
+ * AbandonExtRunnable - wraps up an ldap_abandon_ext operation so it can be
+ * dispatched to the socket thread.
+ */
+class AbandonExtRunnable : public OpRunnable {
+ public:
+  AbandonExtRunnable(nsLDAPOperation *aOperation, int aMsgID)
+      : OpRunnable("AbandonExtRunnable", aOperation), mMsgID(aMsgID) {}
+  virtual ~AbandonExtRunnable() {}
+
+  int32_t mMsgID;
+
+  NS_IMETHOD Run() override {
+    LDAP *ld = LDAPHandle();
+    int retVal = ldap_abandon_ext(ld, mMsgID, 0, 0);
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+
+    // 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),
+    // so we only pay attention to this in debug builds.
+    //
+    // check Connection 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 (Conn()) {
+      nsresult rv = Conn()->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
+        // to verify.
+        //
+        NS_WARNING(
+            "nsLDAPOperation::AbandonExt: "
+            "mConnection->RemovePendingOperation(this) failed.");
+      }
+      SetID(0);
+    }
+    return NS_OK;
+  }
+};
+
 NS_IMETHODIMP
 nsLDAPOperation::AbandonExt() {
-  nsresult rv;
-  nsresult retStatus = NS_OK;
-
   if (!mMessageListener || mMsgID == 0) {
     NS_ERROR(
         "nsLDAPOperation::AbandonExt(): mMessageListener or "
         "mMsgId not initialized");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   // XXX handle controls here
   if (mServerControls || mClientControls) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
-  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),
-  // 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
-  // theorize that ::Clearing the operation is nulling out the mConnection
-  // from another thread.
-  if (mConnection) {
-    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
-      // to verify.
-      //
-      NS_WARNING(
-          "nsLDAPOperation::AbandonExt: "
-          "mConnection->RemovePendingOperation(this) failed.");
-    }
-  }
-
-  return retStatus;
+  nsCOMPtr<nsIRunnable> op = new AbandonExtRunnable(this, mMsgID);
+  mConnection->StartOp(op);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLDAPOperation::GetClientControls(nsIMutableArray **aControls) {
   NS_IF_ADDREF(*aControls = mClientControls);
   return NS_OK;
 }
 
@@ -557,362 +680,387 @@ NS_IMETHODIMP nsLDAPOperation::GetServer
   return NS_OK;
 }
 
 NS_IMETHODIMP nsLDAPOperation::SetServerControls(nsIMutableArray *aControls) {
   mServerControls = aControls;
   return NS_OK;
 }
 
-// wrappers for ldap_add_ext
-//
-nsresult nsLDAPOperation::AddExt(const char *base, nsIArray *mods,
-                                 LDAPControl **serverctrls,
-                                 LDAPControl **clientctrls) {
+/**
+ * AddExtRunnable - wraps up an ldap_add_ext operation so it can be dispatched
+ * to the socket thread.
+ */
+class AddExtRunnable : public OpRunnable {
+ public:
+  AddExtRunnable(nsLDAPOperation *aOperation, const nsACString &aDn,
+                 LDAPMod **aMods)
+      : OpRunnable("AddExtRunnable", aOperation), mDn(aDn), mMods(aMods) {}
+  virtual ~AddExtRunnable() { freeMods(mMods); }
+
+  nsCString mDn;
+  LDAPMod **mMods;
+
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    LDAP *ld = LDAPHandle();
+    int retVal =
+        ldap_add_ext(ld, PromiseFlatCString(mDn).get(), mMods, 0, 0, &msgID);
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
+  }
+};
+
+/**
+ * wrapper for ldap_add_ext(): kicks off an async add request.
+ *
+ * @param aBaseDn           Base DN to search
+ * @param aMods             Array of modifications
+ *
+ * XXX doesn't currently handle LDAPControl params
+ *
+ */
+NS_IMETHODIMP
+nsLDAPOperation::AddExt(const nsACString &aBaseDn, nsIArray *aMods) {
   if (!mMessageListener) {
     NS_ERROR("nsLDAPOperation::AddExt(): mMessageListener not set");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  LDAPMod **attrs = 0;
-  int retVal = LDAP_SUCCESS;
-  uint32_t modCount = 0;
-  nsresult rv = mods->GetLength(&modCount);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (mods && modCount) {
-    attrs = static_cast<LDAPMod **>(
-        moz_xmalloc((modCount + 1) * sizeof(LDAPMod *)));
-    if (!attrs) {
-      NS_ERROR("nsLDAPOperation::AddExt: out of memory ");
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
+  MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
+          ("nsLDAPOperation::AddExt(): called with aBaseDn = '%s'",
+           PromiseFlatCString(aBaseDn).get()));
+  LDAPMod **rawMods;
 
-    nsAutoCString type;
-    uint32_t index;
-    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;
-
+  nsresult rv = convertMods(aMods, &rawMods);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (rawMods) {
 #ifdef NS_DEBUG
-      int32_t operation;
-      NS_ASSERTION(NS_SUCCEEDED(modif->GetOperation(&operation)) &&
-                       ((operation & ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD),
+    // Sanity check - only LDAP_MOD_ADD modifications allowed.
+    for (int i = 0; rawMods[i]; ++i) {
+      int32_t op = rawMods[i]->mod_op;
+      NS_ASSERTION(((op & ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD),
                    "AddExt can only add.");
+    }
 #endif
-
-      attrs[index]->mod_op = LDAP_MOD_ADD | LDAP_MOD_BVALUES;
-
-      nsresult rv = modif->GetType(type);
-      if (NS_FAILED(rv)) break;
-
-      attrs[index]->mod_type = ToNewCString(type);
-
-      rv = CopyValues(modif, &attrs[index]->mod_bvalues);
-      if (NS_FAILED(rv)) break;
-    }
-
-    if (NS_SUCCEEDED(rv)) {
-      attrs[modCount] = 0;
-
-      retVal = ldap_add_ext(mConnectionHandle, base, attrs, serverctrls,
-                            clientctrls, &mMsgID);
-    } else
-      // reset the modCount so we correctly free the array.
-      modCount = index;
+    nsCOMPtr<nsIRunnable> op = new AddExtRunnable(this, aBaseDn, rawMods);
+    mConnection->StartOp(op);
   }
-
-  for (uint32_t counter = 0; counter < modCount; ++counter)
-    delete attrs[counter];
-
-  free(attrs);
-
-  return NS_FAILED(rv) ? rv : TranslateLDAPErrorToNSError(retVal);
+  return NS_OK;
 }
 
 /**
- * wrapper for ldap_add_ext(): kicks off an async add request.
- *
- * @param aBaseDn           Base DN to search
- * @param aModCount         Number of modifications
- * @param aMods             Array of modifications
- *
- * XXX doesn't currently handle LDAPControl params
- *
- * void addExt (in AUTF8String aBaseDn, in unsigned long aModCount,
- *              [array, size_is (aModCount)] in nsILDAPModification aMods);
+ * DeleteExtRunnable - wraps up an ldap_delete_ext operation so it can be
+ * dispatched to the socket thread.
  */
-NS_IMETHODIMP
-nsLDAPOperation::AddExt(const nsACString &aBaseDn, nsIArray *aMods) {
-  MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-          ("nsLDAPOperation::AddExt(): called with aBaseDn = '%s'",
-           PromiseFlatCString(aBaseDn).get()));
+class DeleteExtRunnable : public OpRunnable {
+ public:
+  DeleteExtRunnable(nsLDAPOperation *aOperation, const nsACString &aDn)
+      : OpRunnable("DeleteExtRunnable", aOperation), mDn(aDn) {}
+  virtual ~DeleteExtRunnable() {}
 
-  nsresult rv = AddExt(PromiseFlatCString(aBaseDn).get(), aMods, 0, 0);
-  if (NS_FAILED(rv)) return rv;
+  nsCString mDn;
 
-  // make sure the connection knows where to call back once the messages
-  // for this operation start coming in
-  rv = mConnection->AddPendingOperation(mMsgID, this);
-
-  if (NS_FAILED(rv)) {
-    (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-            ("nsLDAPOperation::AddExt(): abandoned due to rv %" PRIx32,
-             static_cast<uint32_t>(rv)));
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    LDAP *ld = LDAPHandle();
+    int retVal =
+        ldap_delete_ext(ld, PromiseFlatCString(mDn).get(), 0, 0, &msgID);
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
   }
-  return rv;
-}
-
-// wrappers for ldap_delete_ext
-//
-nsresult nsLDAPOperation::DeleteExt(const char *base, LDAPControl **serverctrls,
-                                    LDAPControl **clientctrls) {
-  if (!mMessageListener) {
-    NS_ERROR("nsLDAPOperation::DeleteExt(): mMessageListener not set");
-    return NS_ERROR_NOT_INITIALIZED;
-  }
-
-  return TranslateLDAPErrorToNSError(ldap_delete_ext(
-      mConnectionHandle, base, serverctrls, clientctrls, &mMsgID));
-}
+};
 
 /**
  * wrapper for ldap_delete_ext(): kicks off an async delete request.
  *
  * @param aBaseDn               Base DN to delete
  *
  * XXX doesn't currently handle LDAPControl params
  *
  * void deleteExt(in AUTF8String aBaseDn);
  */
 NS_IMETHODIMP
-nsLDAPOperation::DeleteExt(const nsACString &aBaseDn) {
-  MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-          ("nsLDAPOperation::DeleteExt(): called with aBaseDn = '%s'",
-           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 = mConnection->AddPendingOperation(mMsgID, this);
-
-  if (NS_FAILED(rv)) {
-    (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-            ("nsLDAPOperation::AddExt(): abandoned due to rv %" PRIx32,
-             static_cast<uint32_t>(rv)));
-  }
-  return rv;
-}
-
-// wrappers for ldap_modify_ext
-//
-nsresult nsLDAPOperation::ModifyExt(const char *base, nsIArray *mods,
-                                    LDAPControl **serverctrls,
-                                    LDAPControl **clientctrls) {
+nsLDAPOperation::DeleteExt(const nsACString &aDn) {
   if (!mMessageListener) {
-    NS_ERROR("nsLDAPOperation::ModifyExt(): mMessageListener not set");
+    NS_ERROR("nsLDAPOperation::DeleteExt(): mMessageListener not set");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  LDAPMod **attrs = 0;
-  int retVal = 0;
-  uint32_t modCount = 0;
-  nsresult rv = mods->GetLength(&modCount);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (modCount && mods) {
-    attrs = static_cast<LDAPMod **>(
-        moz_xmalloc((modCount + 1) * sizeof(LDAPMod *)));
-    if (!attrs) {
-      NS_ERROR("nsLDAPOperation::ModifyExt: out of memory ");
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
+  MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
+          ("nsLDAPOperation::DeleteExt(): called with aDn = '%s'",
+           PromiseFlatCString(aDn).get()));
 
-    nsAutoCString type;
-    uint32_t index;
-    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;
-
-      int32_t operation;
-      nsresult rv = modif->GetOperation(&operation);
-      if (NS_FAILED(rv)) break;
+  nsCOMPtr<nsIRunnable> op = new DeleteExtRunnable(this, aDn);
+  mConnection->StartOp(op);
+  return NS_OK;
+}
 
-      attrs[index]->mod_op = operation | LDAP_MOD_BVALUES;
-
-      rv = modif->GetType(type);
-      if (NS_FAILED(rv)) break;
-
-      attrs[index]->mod_type = ToNewCString(type);
+/**
+ * ModifyExtRunnable - wraps up an ldap_modify_ext operation so it can be
+ * dispatched to the socket thread.
+ */
+class ModifyExtRunnable : public OpRunnable {
+ public:
+  ModifyExtRunnable(nsLDAPOperation *aOperation, const nsACString &aDn,
+                    LDAPMod **aMods)
+      : OpRunnable("ModifyExtRunnable", aOperation), mDn(aDn), mMods(aMods) {}
+  virtual ~ModifyExtRunnable() { freeMods(mMods); }
 
-      rv = CopyValues(modif, &attrs[index]->mod_bvalues);
-      if (NS_FAILED(rv)) break;
-    }
-
-    if (NS_SUCCEEDED(rv)) {
-      attrs[modCount] = 0;
+  nsCString mDn;
+  LDAPMod **mMods;
 
-      retVal = ldap_modify_ext(mConnectionHandle, base, attrs, serverctrls,
-                               clientctrls, &mMsgID);
-    } else
-      // reset the modCount so we correctly free the array.
-      modCount = index;
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    LDAP *ld = LDAPHandle();
+    int retVal =
+        ldap_modify_ext(ld, PromiseFlatCString(mDn).get(), mMods, 0, 0, &msgID);
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
   }
-
-  for (uint32_t counter = 0; counter < modCount; ++counter)
-    delete attrs[counter];
-
-  free(attrs);
-
-  return NS_FAILED(rv) ? rv : TranslateLDAPErrorToNSError(retVal);
-}
+};
 
 /**
  * wrapper for ldap_modify_ext(): kicks off an async modify request.
  *
  * @param aBaseDn           Base DN to modify
  * @param aModCount         Number of modifications
  * @param aMods             Array of modifications
  *
  * XXX doesn't currently handle LDAPControl params
  *
  * void modifyExt (in AUTF8String aBaseDn, in unsigned long aModCount,
  *                 [array, size_is (aModCount)] in nsILDAPModification aMods);
  */
 NS_IMETHODIMP
 nsLDAPOperation::ModifyExt(const nsACString &aBaseDn, nsIArray *aMods) {
+  if (!mMessageListener) {
+    NS_ERROR("nsLDAPOperation::ModifyExt(): mMessageListener not set");
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+
   MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
           ("nsLDAPOperation::ModifyExt(): called with aBaseDn = '%s'",
            PromiseFlatCString(aBaseDn).get()));
 
-  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 = mConnection->AddPendingOperation(mMsgID, this);
-
-  if (NS_FAILED(rv)) {
-    (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-            ("nsLDAPOperation::AddExt(): abandoned due to rv %" PRIx32,
-             static_cast<uint32_t>(rv)));
+  LDAPMod **rawMods;
+  nsresult rv = convertMods(aMods, &rawMods);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (rawMods) {
+    nsCOMPtr<nsIRunnable> op = new ModifyExtRunnable(this, aBaseDn, rawMods);
+    mConnection->StartOp(op);
   }
-  return rv;
+  return NS_OK;
 }
 
-// wrappers for ldap_rename
-//
-nsresult nsLDAPOperation::Rename(const char *base, const char *newRDn,
-                                 const char *newParent, bool deleteOldRDn,
-                                 LDAPControl **serverctrls,
-                                 LDAPControl **clientctrls) {
-  if (!mMessageListener) {
-    NS_ERROR("nsLDAPOperation::Rename(): mMessageListener not set");
-    return NS_ERROR_NOT_INITIALIZED;
+/**
+ * RenameRunnable - wraps up an ldap_modify_ext operation so it can be
+ * dispatched to the socket thread.
+ */
+class RenameRunnable : public OpRunnable {
+ public:
+  RenameRunnable(nsLDAPOperation *aOperation, const nsACString &aBaseDn,
+                 const nsACString &aNewRDn, const nsACString &aNewParent,
+                 bool aDeleteOldRDn)
+      : OpRunnable("RenameRunnable", aOperation),
+        mBaseDn(aBaseDn),
+        mNewRDn(aNewRDn),
+        mNewParent(aNewParent),
+        mDeleteOldRDn(aDeleteOldRDn) {}
+  virtual ~RenameRunnable() {}
+
+  nsCString mBaseDn;
+  nsCString mNewRDn;
+  nsCString mNewParent;
+  bool mDeleteOldRDn;
+
+  NS_IMETHOD Run() override {
+    int32_t msgID;
+    int retVal = ldap_rename(LDAPHandle(), PromiseFlatCString(mBaseDn).get(),
+                             PromiseFlatCString(mNewRDn).get(),
+                             PromiseFlatCString(mNewParent).get(),
+                             mDeleteOldRDn, 0, 0, &msgID);
+    if (retVal != LDAP_SUCCESS) {
+      NotifyLDAPError();
+      return NS_OK;
+    }
+    SetID(msgID);
+    // Register the operation to pick up responses.
+    Conn()->AddPendingOperation(msgID, mOp);
+    return NS_OK;
   }
-
-  return TranslateLDAPErrorToNSError(
-      ldap_rename(mConnectionHandle, base, newRDn, newParent, deleteOldRDn,
-                  serverctrls, clientctrls, &mMsgID));
-}
+};
 
 /**
  * wrapper for ldap_rename(): kicks off an async rename request.
  *
  * @param aBaseDn               Base DN to rename
  * @param aNewRDn               New relative DN
  * @param aNewParent            DN of the new parent under which to move the
  *
  * XXX doesn't currently handle LDAPControl params
  *
  * void rename(in AUTF8String aBaseDn, in AUTF8String aNewRDn,
  *             in AUTF8String aNewParent, in boolean aDeleteOldRDn);
  */
 NS_IMETHODIMP
 nsLDAPOperation::Rename(const nsACString &aBaseDn, const nsACString &aNewRDn,
                         const nsACString &aNewParent, bool aDeleteOldRDn) {
+  if (!mMessageListener) {
+    NS_ERROR("nsLDAPOperation::Rename(): mMessageListener not set");
+    return NS_ERROR_NOT_INITIALIZED;
+  }
   MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
           ("nsLDAPOperation::Rename(): called with aBaseDn = '%s'",
            PromiseFlatCString(aBaseDn).get()));
 
-  nsresult rv = Rename(
-      PromiseFlatCString(aBaseDn).get(), 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 = mConnection->AddPendingOperation(mMsgID, this);
-
-  if (NS_FAILED(rv)) {
-    (void)ldap_abandon_ext(mConnectionHandle, mMsgID, 0, 0);
-    MOZ_LOG(gLDAPLogModule, mozilla::LogLevel::Debug,
-            ("nsLDAPOperation::AddExt(): abandoned due to rv %" PRIx32,
-             static_cast<uint32_t>(rv)));
-  }
-  return rv;
+  nsCOMPtr<nsIRunnable> op =
+      new RenameRunnable(this, aBaseDn, aNewRDn, aNewParent, aDeleteOldRDn);
+  mConnection->StartOp(op);
+  return NS_OK;
 }
 
-// wrappers for ldap_search_ext
-//
-
-/* static */
-nsresult nsLDAPOperation::CopyValues(nsILDAPModification *aMod,
-                                     berval ***aBValues) {
-  nsCOMPtr<nsIArray> values;
-  nsresult rv = aMod->GetValues(getter_AddRefs(values));
-  NS_ENSURE_SUCCESS(rv, rv);
-
+/**
+ * Convert nsILDAPBERValue array to null-terminated array of berval ptrs.
+ * The returned array should be freed with freeValues().
+ */
+static nsresult convertValues(nsIArray *values, berval ***aBValues) {
   uint32_t valuesCount;
-  rv = values->GetLength(&valuesCount);
+  nsresult rv = values->GetLength(&valuesCount);
   NS_ENSURE_SUCCESS(rv, rv);
 
   *aBValues =
       static_cast<berval **>(moz_xmalloc((valuesCount + 1) * sizeof(berval *)));
   if (!*aBValues) return NS_ERROR_OUT_OF_MEMORY;
 
   uint32_t valueIndex;
   for (valueIndex = 0; valueIndex < valuesCount; ++valueIndex) {
     nsCOMPtr<nsILDAPBERValue> value = do_QueryElementAt(values, valueIndex);
 
     nsTArray<uint8_t> tmp;
     rv = value->Get(tmp);
-    if (NS_FAILED(rv)) {
-      goto bailout;
-    }
+    if (NS_FAILED(rv)) break;
+
     berval *bval = new berval;
     bval->bv_len = tmp.Length() * sizeof(uint8_t);
     bval->bv_val = static_cast<char *>(moz_xmalloc(bval->bv_len));
     if (!bval->bv_val) {
       rv = NS_ERROR_OUT_OF_MEMORY;
-      delete bval;
-      goto bailout;
+      break;
     }
     memcpy(bval->bv_val, tmp.Elements(), bval->bv_len);
     (*aBValues)[valueIndex] = bval;
   }
+  (*aBValues)[valueIndex] = nullptr;
 
-  (*aBValues)[valuesCount] = 0;
+  if (NS_FAILED(rv)) {
+    freeValues(*aBValues);
+    *aBValues = nullptr;
+    return rv;
+  }
   return NS_OK;
+}
+
+static void freeValues(berval **aVals) {
+  if (!aVals) {
+    return;
+  }
+  for (int i = 0; aVals[i]; ++i) {
+    free(aVals[i]->bv_val);
+    delete (aVals[i]);
+  }
+  free(aVals);
+}
+
+/**
+ * Convert nsILDAPModifications to null-terminated array of LDAPMod ptrs.
+ * If input aMods is missing/empty, will return null ptr (and NS_OK).
+ * Will return null upon error.
+ * The returned array should be freed with freeMods().
+ */
+static nsresult convertMods(nsIArray *aMods, LDAPMod ***aOut) {
+  *aOut = nullptr;
+
+  uint32_t modCount = 0;
+  nsresult rv = aMods->GetLength(&modCount);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (aMods && modCount) {
+    *aOut = static_cast<LDAPMod **>(
+        moz_xmalloc((modCount + 1) * sizeof(LDAPMod *)));
+    if (!*aOut) {
+      NS_ERROR("nsLDAPOperation::AddExt: out of memory ");
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+
+    nsAutoCString type;
+    uint32_t index;
+    for (index = 0; index < modCount && NS_SUCCEEDED(rv); ++index) {
+      LDAPMod *mod = new LDAPMod();
 
-bailout:
-  if (*aBValues) {
-    // Free all the entries we created before the failure.
-    for (uint32_t i = 0; i < valueIndex; ++i) {
-      berval *bval = (*aBValues)[i];
-      free(bval->bv_val);
-      delete bval;
+      nsCOMPtr<nsILDAPModification> modif(do_QueryElementAt(aMods, index, &rv));
+      if (NS_FAILED(rv)) break;
+
+      int32_t operation;
+      rv = modif->GetOperation(&operation);
+      if (NS_FAILED(rv)) break;
+      mod->mod_op = operation | LDAP_MOD_BVALUES;
+
+      nsresult rv = modif->GetType(type);
+      if (NS_FAILED(rv)) break;
+      mod->mod_type = ToNewCString(type);
+
+      nsCOMPtr<nsIArray> values;
+      rv = modif->GetValues(getter_AddRefs(values));
+      if (NS_FAILED(rv)) break;
+      rv = convertValues(values, &mod->mod_bvalues);
+      if (NS_FAILED(rv)) {
+        free(mod->mod_type);
+        break;
+      }
+      (*aOut)[index] = mod;
     }
-    free(*aBValues);
+    (*aOut)[index] = nullptr;  // Always terminate array, even if failed.
+
+    if (NS_FAILED(rv)) {
+      // clean up.
+      freeMods(*aOut);
+      *aOut = nullptr;
+      return rv;
+    }
   }
-  return rv;
+  return NS_OK;
 }
+
+/**
+ * Free an LDAPMod array created by convertMods().
+ */
+static void freeMods(LDAPMod **aMods) {
+  if (!aMods) {
+    return;
+  }
+  int i;
+  for (i = 0; aMods[i]; ++i) {
+    LDAPMod *mod = aMods[i];
+    free(mod->mod_type);
+    freeValues(mod->mod_bvalues);
+    delete mod;
+  }
+  free(aMods);
+}
--- a/ldap/xpcom/src/nsLDAPOperation.h
+++ b/ldap/xpcom/src/nsLDAPOperation.h
@@ -22,16 +22,17 @@
   {                                                  \
     0x97a479d0, 0x9a44, 0x47c6, {                    \
       0xa1, 0x7a, 0x87, 0xf9, 0xb0, 0x02, 0x94, 0xbb \
     }                                                \
   }
 
 class nsLDAPOperation : public nsILDAPOperation {
  public:
+  friend class OpRunnable;
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSILDAPOPERATION
 
   // constructor & destructor
   //
   nsLDAPOperation();
 
   /**
@@ -40,59 +41,17 @@ class nsLDAPOperation : public nsILDAPOp
   void Clear();
 
   // Stores the request number for later check of the operation is still valid
   uint32_t mRequestNum;
 
  private:
   virtual ~nsLDAPOperation();
 
-  /**
-   * wrapper for ldap_add_ext()
-   *
-   * XXX should move to idl, once LDAPControls have an IDL representation
-   */
-  nsresult AddExt(const char *base,  // base DN to add
-                  nsIArray *mods,    // Array of modifications
-                  LDAPControl **serverctrls, LDAPControl **clientctrls);
-
-  /**
-   * wrapper for ldap_delete_ext()
-   *
-   * XXX should move to idl, once LDAPControls have an IDL representation
-   */
-  nsresult DeleteExt(const char *base,  // base DN to delete
-                     LDAPControl **serverctrls, LDAPControl **clientctrls);
-
-  /**
-   * wrapper for ldap_modify_ext()
-   *
-   * XXX should move to idl, once LDAPControls have an IDL representation
-   */
-  nsresult ModifyExt(const char *base,  // base DN to modify
-                     nsIArray *mods,    // array of modifications
-                     LDAPControl **serverctrls, LDAPControl **clientctrls);
-
-  /**
-   * wrapper for ldap_rename()
-   *
-   * XXX should move to idl, once LDAPControls have an IDL representation
-   */
-  nsresult Rename(const char *base,       // base DN to rename
-                  const char *newRDn,     // new RDN
-                  const char *newParent,  // DN of the new parent
-                  bool deleteOldRDn,      // remove old RDN in the entry?
-                  LDAPControl **serverctrls, LDAPControl **clientctrls);
-
-  /**
-   * Helper function to copy the values of an nsILDAPModification into an
-   * array of berval's.
-   */
-  static nsresult CopyValues(nsILDAPModification *aMod, berval ***aBValues);
-
+ protected:
   nsCOMPtr<nsILDAPMessageListener> mMessageListener;  // results go here
   nsCOMPtr<nsISupports>
       mClosure;  // private parameter (anything caller desires)
   RefPtr<nsLDAPConnection> mConnection;  // connection this op is on
 
   LDAP *mConnectionHandle;  // cache connection handle
   nsCString mSavePassword;
   nsCString mMechanism;
--- a/ldap/xpcom/src/nsLDAPSecurityGlue.cpp
+++ b/ldap/xpcom/src/nsLDAPSecurityGlue.cpp
@@ -99,16 +99,18 @@ extern "C" int LDAP_CALLBACK nsLDAPSSLCo
   // Ensure secure option is set.  Also, clear secure bit in options
   // the we pass to the standard connect() function (since it doesn't know
   // how to handle the secure option).
   //
   NS_ASSERTION(options & LDAP_X_EXTIOF_OPT_SECURE,
                "nsLDAPSSLConnect(): called for non-secure connection");
   options &= ~LDAP_X_EXTIOF_OPT_SECURE;
 
+  NS_ASSERTION(options & LDAP_X_EXTIOF_OPT_NONBLOCKING,
+               "nsLDAPSSLConnect(): called for blocking connection");
   // Retrieve session info. so we can store a pointer to our session info.
   // in our socket info. later.
   //
   memset(&sessionInfo, 0, sizeof(sessionInfo));
   sessionInfo.seinfo_size = PRLDAP_SESSIONINFO_SIZE;
   if (prldap_get_session_info(nullptr, sessionarg, &sessionInfo) !=
       LDAP_SUCCESS) {
     NS_ERROR("nsLDAPSSLConnect(): unable to get session info");