Bug 557928 - Grab a local reference to mConnection at the beginning of nsLDAPOperation::SimpleBind and use it instead of the member variable. r=dbienvenu,a=Standard8
authorLeon Sha <leon.sha@oracle.com>
Tue, 08 May 2012 18:38:09 -0400
changeset 9902 c0a7bf8041beb0032bdeea6b7f206e10b1d7f1cf
parent 9901 5fbea4fc8fa3e1b99c601336d8c9347605077e0a
child 9903 a9b450bec1f70d37a5ac37530fb5596052be3822
child 9905 7a855d117838407343e51badd1916c2a177f7cef
child 9906 bcb794d70dc97f414945dc0e3ace0148a01624a0
push id35
push userbugzilla@standard8.plus.com
push dateFri, 05 Oct 2012 15:46:41 +0000
reviewersdbienvenu, Standard8
bugs557928
Bug 557928 - Grab a local reference to mConnection at the beginning of nsLDAPOperation::SimpleBind and use it instead of the member variable. r=dbienvenu,a=Standard8
ldap/xpcom/src/nsLDAPOperation.cpp
--- a/ldap/xpcom/src/nsLDAPOperation.cpp
+++ b/ldap/xpcom/src/nsLDAPOperation.cpp
@@ -285,56 +285,60 @@ nsLDAPOperation::SaslStep(const char *to
 }
 
 
 // wrapper for ldap_simple_bind()
 //
 NS_IMETHODIMP
 nsLDAPOperation::SimpleBind(const nsACString& passwd)
 {
+    nsRefPtr<nsLDAPConnection> connection = mConnection;
+    // There is a possibilty that mConnection can be cleared by another
+    // thread. Grabbing a local reference to mConnection may avoid this.
+    // See https://bugzilla.mozilla.org/show_bug.cgi?id=557928#c1
     nsresult rv;
     nsCAutoString bindName;
     PRInt32 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
     // 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);
+    rv = connection->GetBindName(bindName);
     if (NS_FAILED(rv))
         return rv;
 
     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)
-      mConnection->RemovePendingOperation(originalMsgID);
+      connection->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 = mConnection->AddPendingOperation(mMsgID, this);
+    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: