fix bug 67357. link-time bustage in ldap module caused by earlier nsIURI::SchemeIs change. not in default build. r=gagan@netscape.com, sr=scc@mozilla.org

directory/xpcom/base/src/nsLDAPChannel.cpp

@@ -64,6 +64,9 @@ nsLDAPChannel::nsLDAPChannel()
 
 nsLDAPChannel::~nsLDAPChannel()
 {
+    if (mCurrentOperationMonitor) {
+        PR_DestroyMonitor(mCurrentOperationMonitor);
+    }
 }
 
 // initialize the channel
@@ -122,6 +125,13 @@ nsLDAPChannel::Init(nsIURI *uri)
     }
 
 #else   
+    mCurrentOperationMonitor = PR_NewMonitor();
+    if (!mCurrentOperationMonitor) {
+        NS_ERROR("nsLDAPChannel::Init(): could not create "
+                 "mCurrentOperationMonitor");
+        return NS_ERROR_FAILURE;
+    }
+
     mCallback = this;
 #endif
     
@@ -175,13 +185,10 @@ nsLDAPChannel::Cancel(nsresult aStatus)
 {
     nsresult rv;
 
-    // set the status
-    //
-    mStatus = aStatus;
-
     // if there is an operation running, abandon it and remove it from the 
-    // queue
+    // queue.  this will cause all cleanup to be done from the other thread
     //
+    PR_EnterMonitor(mCurrentOperationMonitor);
     if (mCurrentOperation) {
 
         // if this fails in a non-debug build, there's not much we can do
@@ -194,7 +201,14 @@ nsLDAPChannel::Cancel(nsresult aStatus)
         // 
         mCurrentOperation = 0;
 
-    }
+        // XXXdmose wait on a cond var here if we need to be sure that
+        // cancel has finished before returning
+
+    } else {
+
+    // set the status
+    //
+    mStatus = aStatus;
 
     // if the read pipe exists and hasn't already been closed, close it
     //
@@ -214,16 +228,21 @@ nsLDAPChannel::Cancel(nsresult aStatus)
     if (mLoadGroup) {
         rv = mLoadGroup->RemoveChannel(this, mResponseContext, aStatus,
                                        nsnull);
-        if (NS_FAILED(rv)) 
+        NS_ASSERTION(NS_SUCCEEDED(rv), "nsLDAPChannel::Cancel(): "
-            return rv;
+                     "mLoadGroup->RemoveChannel() failed");
     }
 
     // call listener's onstoprequest
     //
     if (mUnproxiedListener) {
         rv = mListener->OnStopRequest(this, mResponseContext, aStatus, nsnull);
-        if (NS_FAILED(rv)) 
+        NS_ASSERTION(NS_SUCCEEDED(rv), "nsLDAPChannel::Cancel(): "
-            return rv;
+                     "mListener->OnStopRequest() failed");
+    } 
+
+    return NS_OK;
+
+        // XXX cleanup on this thread
     }
 
     return NS_OK;
@@ -741,7 +760,7 @@ nsLDAPChannel::AsyncRead(nsIStreamListener* aListener,
     } 
 
     // get an AsyncStreamListener to proxy for mListener, if we're
-    // compiled to have the LDAP callbacks happen on the LDAP connection=
+    // compiled to have the LDAP callbacnks happen on the LDAP connection=
     // thread.
     //
 #if INVOKE_LDAP_CALLBACKS_ON_MAIN_THREAD
@@ -851,6 +870,13 @@ nsLDAPChannel::OnLDAPMessage(nsILDAPMessage *aMessage)
         return NS_ERROR_UNEXPECTED;
     }
 
+    // XXXdmose - we may want a small state machine either here or
+    // or in the nsLDAPConnection object, to make sure that things are 
+    // happening in the right order and timing out appropriately.  this will
+    // certainly depend on how timeouts are implemented, and if binding
+    // gets subsumed into the nsLDAPConnection object as part of making
+    // connections shared.
+    // 
     switch (messageType) {
 
     case LDAP_RES_BIND:
@@ -910,7 +936,7 @@ nsLDAPChannel::OnLDAPBind(nsILDAPMessage *aMessage)
     nsresult rv;
 
     // XXX should call ldap_parse_result() here
-
+    //
     mCurrentOperation = 0;  // done with bind op; make nsCOMPtr release it
 
     // create and initialize an LDAP operation (to be used for the search
@@ -982,6 +1008,7 @@ nsLDAPChannel::OnLDAPSearchResult(nsILDAPMessage *aMessage)
         PR_fprintf(PR_STDERR, " %s\n", ldap_err2string(errorCode));
 #endif
         return NS_ERROR_FAILURE;
+
     }
 
     // we're done with the current operation.  cause nsCOMPtr to Release() it
@@ -1078,7 +1105,7 @@ nsLDAPChannel::OnLDAPSearchEntry(nsILDAPMessage *aMessage)
         if (NS_FAILED(rv)) {
             NS_WARNING("nsLDAPChannel:OnLDAPSearchEntry(): "
                        "aMessage->GetValues() failed\n");
-            NSLDAP_FREE_XPIDL_ARRAY(attrCount, attrs, nsMemory::Free);
+            NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(attrCount, attrs);
             return rv;;
         }
 
@@ -1090,10 +1117,10 @@ nsLDAPChannel::OnLDAPSearchEntry(nsILDAPMessage *aMessage)
             entry.Append(vals[j]);
             entry.Append("\n");
         }
-        NSLDAP_FREE_XPIDL_ARRAY(valueCount, vals, nsMemory::Free);
+        NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(valueCount, vals);
 
     }
-    NSLDAP_FREE_XPIDL_ARRAY(attrCount, attrs, nsMemory::Free);
+    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(attrCount, attrs);
 
     // XXXdmose better error handling
     //
@@ -1134,3 +1161,50 @@ nsLDAPChannel::OnLDAPSearchEntry(nsILDAPMessage *aMessage)
 
     return NS_OK;
 }
+
+/**
+ * Called from the LDAP Connection thread to cleanup resources used in the 
+ * OnLDAP*Message functions.  For nsILDAPMessageListener interface.
+ */
+NS_IMETHODIMP 
+nsLDAPChannel::OnLDAPOperationRemoval()
+{
+    nsresult rv;
+
+    // set the status
+    //
+    nsresult aStatus;
+    mStatus = aStatus;
+
+    // if the read pipe exists and hasn't already been closed, close it
+    //
+    if (mReadPipeOut != 0 && !mReadPipeClosed) {
+        
+        // XXX set mReadPipeClosed?
+
+        // if this fails in a non-debug build, there's not much we can do
+        //
+        rv = mReadPipeOut->Close();
+        NS_ASSERTION(NS_SUCCEEDED(rv), "nsLDAPChannel::Cancel(): "
+                     "mReadPipeOut->Close() failed");
+    }
+
+    // remove self from loadgroup to stop the throbber
+    //
+    if (mLoadGroup) {
+        rv = mLoadGroup->RemoveChannel(this, mResponseContext, aStatus,
+                                       nsnull);
+        NS_ASSERTION(NS_SUCCEEDED(rv), "nsLDAPChannel::Cancel(): "
+                     "mLoadGroup->RemoveChannel() failed");
+    }
+
+    // call listener's onstoprequest
+    //
+    if (mUnproxiedListener) {
+        rv = mListener->OnStopRequest(this, mResponseContext, aStatus, nsnull);
+        NS_ASSERTION(NS_SUCCEEDED(rv), "nsLDAPChannel::Cancel(): "
+                     "mListener->OnStopRequest() failed");
+    } 
+
+    return NS_OK;
+}

directory/xpcom/base/src/nsLDAPChannel.h

@@ -106,7 +106,7 @@ protected:
   PRBool mReadPipeClosed; // has the pipe already been closed?
   nsCOMPtr<nsILDAPMessageListener> mCallback; // callback
   nsCOMPtr<nsIProgressEventSink> mEventSink; // fire status against this
-
+  PRMonitor *mCurrentOperationMonitor; // guard mCurrentOperation
 };
 
 #endif // nsLDAPChannel_h__

directory/xpcom/base/src/nsLDAPConnection.cpp

@@ -38,7 +38,6 @@
 #include "nsIComponentManager.h"
 #include "nsLDAPConnection.h"
 #include "nsLDAPMessage.h"
-#include "nsIEventQueueService.h"
 #include "nsIConsoleService.h"
 
 const char kConsoleServiceContractId[] = "@mozilla.org/consoleservice;1";
@@ -51,6 +50,8 @@ extern "C" int nsLDAPThreadFuncsInit(LDAP *aLDAP);
 nsLDAPConnection::nsLDAPConnection()
 {
   NS_INIT_ISUPPORTS();
+
+  mThreadInitCompleted = 0;
 }
 
 // destructor
@@ -167,7 +168,9 @@ nsLDAPConnection::Init(const char *aHost, PRInt16 aPort, const char *aBindName)
 NS_IMETHODIMP
 nsLDAPConnection::GetBindName(char **_retval)
 {
-    NS_ENSURE_ARG_POINTER(_retval);
+    if (!_retval) {
+        return NS_ERROR_ILLEGAL_VALUE;
+    }
     
     // check for NULL (meaning bind anonymously)
     //
@@ -193,7 +196,9 @@ NS_IMETHODIMP
 nsLDAPConnection::GetLdErrno(char **matched, char **errString, 
                              PRInt32 *_retval)
 {
-    NS_ENSURE_ARG_POINTER(_retval);
+    if (!_retval) {
+        return NS_ERROR_ILLEGAL_VALUE;
+    }
 
     *_retval = ldap_get_lderrno(this->mConnectionHandle, matched, errString);
 
@@ -209,7 +214,9 @@ nsLDAPConnection::GetLdErrno(char **matched, char **errString,
 NS_IMETHODIMP
 nsLDAPConnection::GetErrorString(char **_retval)
 {
-    NS_ENSURE_ARG_POINTER(_retval);
+    if (!_retval) {
+        return NS_ERROR_ILLEGAL_VALUE;
+    }
 
     *_retval = ldap_err2string(ldap_get_lderrno(this->mConnectionHandle, 
                                                 NULL, NULL));
@@ -280,29 +287,22 @@ nsLDAPConnection::AddPendingOperation(nsILDAPOperation *aOperation)
     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 
- *
- * void removePendingOperation(in nsILDAPOperation aOperation);
- */
 NS_IMETHODIMP
 nsLDAPConnection::RemovePendingOperation(nsILDAPOperation *aOperation)
 {
     nsresult rv;
     PRInt32 msgId;
 
-    NS_ENSURE_ARG_POINTER(aOperation);
+    if ( !aOperation ) {
+        return NS_ERROR_ILLEGAL_VALUE;
+    }
 
     // find the message id
     //
     rv = aOperation->GetMessageId(&msgId);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (!rv) {
+        return NS_ERROR_UNEXPECTED;
+    }
 
     // turn it into an nsVoidKey.  note that this is another spot that
     // assumes that sizeof(void*) >= sizeof(PRInt32).  
@@ -345,6 +345,17 @@ nsLDAPConnection::RemovePendingOperation(nsILDAPOperation *aOperation)
 NS_IMETHODIMP
 nsLDAPConnection::Run(void)
 {
+    // XXXdmose all these return NS_ERROR_FAILUREs aren't really very useful
+    // need to marshal this back to the main thread somehow for error handling
+    
+    // XXXdmose right now we timeout of ldap_result on all platforms 
+    // after 5ms (that value was chosen because that's what 
+    // nsSocketTransportService on mac uses.  What we'd really like to do, at
+    // least on platforms that support it, is to use a PR_PollableEvent with 
+    // NSPR under the LDAP C SDK 4.1 or later, and cause it to pop out
+    // of ldap_result whenever we've got something to handle.
+    //
+    struct timeval pollTimeout = {0, 5000};   // 5 milliseconds
     int lderrno;
     nsresult rv;
     PRInt32 returnCode;
@@ -353,16 +364,41 @@ nsLDAPConnection::Run(void)
 
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
            ("nsLDAPConnection::Run() entered\n"));
+    NS_ASSERTION(!mThreadInitCompleted, "nsLDAPConnection::Run(): thread "
+                 "already initialized");
+    if (mThreadInitCompleted)
+        return NS_OK;
 
     // get the console service so we can log messages
     //
-    nsCOMPtr<nsIConsoleService> consoleSvc = 
+    nsCOMPtr<nsIConsoleService> 
-        do_GetService(kConsoleServiceContractId, &rv);
+        consoleSvc(do_GetService(kConsoleServiceContractId, &rv));
     if (NS_FAILED(rv)) {
         NS_ERROR("nsLDAPConnection::Run() couldn't get console service");
         return NS_ERROR_FAILURE;
     }
 
+    // setup a monitored event queue for this thread
+    //
+    nsCOMPtr<nsIEventQueueService> 
+        eventQSvc(do_GetService(NS_EVENTQUEUESERVICE_CONTRACTID, &rv));
+    if (NS_FAILED(rv)) {
+        NS_ERROR("nsLDAPConnection::Run() couldn't get event queue service");
+        return NS_ERROR_FAILURE;
+    }
+    rv = eventQSvc->CreateMonitoredThreadEventQueue();
+    if (NS_FAILED(rv)) {
+        NS_ERROR("nsLDAPConnection::Run() couldn't create event queue");
+        return NS_ERROR_FAILURE;
+    }
+    rv = eventQSvc->GetThreadEventQueue(NS_CURRENT_THREAD, 
+                                        getter_AddRefs(mEventQ));
+    if (NS_FAILED(rv)) {
+        NS_ERROR("nsLDAPConnection::Run() couldn't get current event queue");
+        eventQSvc->DestroyThreadEventQueue();
+        return NS_ERROR_FAILURE;
+    }
+
     // initialize the thread-specific data for the child thread (as necessary)
     //
     if (!nsLDAPThreadDataInit()) {
@@ -371,6 +407,10 @@ nsLDAPConnection::Run(void)
         return NS_ERROR_FAILURE;
     }
 
+    // so that UI thread can ensure that we're ready to go
+    //
+    mThreadInitCompleted = 1;
+
     // wait for results
     //
     while(1) {
@@ -386,7 +426,7 @@ nsLDAPConnection::Run(void)
         // XXX deal with timeouts better
         //
         returnCode = ldap_result(mConnectionHandle, LDAP_RES_ANY,
-                                 LDAP_MSG_ONE, LDAP_NO_LIMIT, &msgHandle);
+                                 LDAP_MSG_ONE, &pollTimeout, &msgHandle);
 
         // if we didn't error or timeout, create an nsILDAPMessage
         //      
@@ -394,12 +434,24 @@ nsLDAPConnection::Run(void)
 
         case 0: // timeout
 
-            // the connection may not exist yet.  sleep for a while
+            // either the connection doesn't exist, or it's time to check for 
-            // and try again
+            // any events (currently just shutdown flag), eventually perhaps
+            // the innards of an event queue loop will be here.
             //
-            PR_LOG(gLDAPLogModule, PR_LOG_WARNING, 
+            PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, 
                    ("ldap_result() timed out.\n"));
-            PR_Sleep(2000); // XXXdmose - reasonable timeslice?
+
+            // handle any pending events
+            //
+            rv = mEventQ->ProcessPendingEvents();
+            if ( NS_FAILED(rv) ) {
+                NS_ERROR("Error processing pending events.");
+            }
+
+            // XXXdmose check for shutdown or timeout of entire operation
+
+            // next loop iteration
+            //
             continue;
 
             break;
@@ -526,7 +578,21 @@ nsLDAPConnection::Run(void)
     // XXX figure out how to break out of the while() loop and get here to
     // so we can expire (though not if DEBUG is defined, since gdb gets ill
     // if threads exit
+
+    // XXXdmose call StopAcceptingEvents() and then ProcessPendingEvents() 
+    // here?
+
+    // make the event queue service release its hold on this queue
     //
+    rv = eventQSvc->DestroyThreadEventQueue();
+    if (NS_FAILED(rv)) {
+        NS_ERROR("Error destroying thread event queue.");
+    }
+
+    // and make nsCOMPtr release our hold on the queue
+    //
+    mEventQ = 0;
+    
     return NS_OK;
 }
 

directory/xpcom/base/src/nsLDAPConnection.h

@@ -43,6 +43,7 @@
 #include "nsILDAPMessageListener.h"
 #include "nsHashtable.h"
 #include "nspr.h"
+#include "nsIEventQueueService.h"
 
 // 0d871e30-1dd2-11b2-8ea9-831778c78e93
 //
@@ -77,8 +78,11 @@ class nsLDAPConnection : public nsILDAPConnection, nsIRunnable
     LDAP *mConnectionHandle;		// the LDAP C SDK's connection object
     nsCString *mBindName; 		// who to bind as
     nsCOMPtr<nsIThread> mThread;       	// thread which marshals results
+    nsCOMPtr<nsIEventQueue> mEventQ;	// some events come to mThread this way
 
     nsSupportsHashtable *mPendingOperations; // keep these around for callbacks
+    PRBool mThreadInitCompleted;	// has initialization of the 
+                                        // connection thread completed?
 };
 
 #endif /* _nsLDAPConnection_h_ */

directory/xpcom/base/src/nsLDAPURL.cpp

@@ -232,6 +232,14 @@ NS_IMETHODIMP nsLDAPURL::Equals(nsIURI *other, PRBool *_retval)
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+/* boolean schemeIs(in PRUint32 scheme); */
+NS_IMETHODIMP nsLDAPURL::SchemeIs(PRUint32 i_Scheme, PRBool *o_Equals)
+{
+    *o_Equals = (i_Scheme == nsIURI::LDAP);
+
+    return NS_OK;
+}
+
 /* nsIURI clone (); */
 NS_IMETHODIMP nsLDAPURL::Clone(nsIURI **_retval)
 {