You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2007/11/28 23:19:01 UTC

svn commit: r599164 - in /httpd/httpd/trunk: include/ap_mmn.h include/util_ldap.h modules/ldap/util_ldap.c

Author: covener
Date: Wed Nov 28 14:19:00 2007
New Revision: 599164

URL: http://svn.apache.org/viewvc?rev=599164&view=rev
Log:
Perform all per-LDAP-backend related memory allocations in a standalone pool,
provide a local method to completely remove an LDAP backend connection so
we can someday manage/dispose of extra connections in a reasonable way.

Clarify some commentary around the existing murky close/cleanup API
methods.

Minor bump for new members appended to util_ldap_connection_t, which is not
allocated by consumers of the API.


Modified:
    httpd/httpd/trunk/include/ap_mmn.h
    httpd/httpd/trunk/include/util_ldap.h
    httpd/httpd/trunk/modules/ldap/util_ldap.c

Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=599164&r1=599163&r2=599164&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Wed Nov 28 14:19:00 2007
@@ -140,6 +140,7 @@
  * 20071023.3 (2.3.0-dev)  Declare ap_time_process_request() as part of the
  *                         public scoreboard API.
  * 20071108.1 (2.3.0-dev)  Add the optional kept_body brigade to request_rec
+ * 20071108.2 (2.3.0-dev)  Add st and keep fields to struct util_ldap_connection_t
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -147,7 +148,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20071108
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 1                    /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 2                    /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/trunk/include/util_ldap.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_ldap.h?rev=599164&r1=599163&r2=599164&view=diff
==============================================================================
--- httpd/httpd/trunk/include/util_ldap.h (original)
+++ httpd/httpd/trunk/include/util_ldap.h Wed Nov 28 14:19:00 2007
@@ -107,6 +107,8 @@
     const char *reason;                 /* Reason for an error failure */
 
     struct util_ldap_connection_t *next;
+    struct util_ldap_state_t *st;        /* The LDAP vhost config this connection belongs to */
+    int keep;                            /* Will this connection be kept when it's unlocked */
 } util_ldap_connection_t;
 
 /* LDAP cache state information */ 

Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=599164&r1=599163&r2=599164&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
+++ httpd/httpd/trunk/modules/ldap/util_ldap.c Wed Nov 28 14:19:00 2007
@@ -66,6 +66,8 @@
         apr_global_mutex_unlock(st->util_ldap_cache_lock);      \
 } while (0)
 
+static apr_status_t util_ldap_connection_remove (void *param);
+
 static void util_ldap_strdup (char **str, const char *newstr)
 {
     if (*str) {
@@ -119,9 +121,9 @@
     return OK;
 }
 
-/* ------------------------------------------------------------------ */
 
 
+/* ------------------------------------------------------------------ */
 /*
  * Closes an LDAP connection by unlocking it. The next time
  * uldap_connection_find() is called this connection will be
@@ -141,18 +143,22 @@
      * we don't have to...
      */
 
-    /* mark our connection as available for reuse */
-
+     if (!ldc->keep) { 
+         util_ldap_connection_remove(ldc);
+     }
+     else { 
+         /* mark our connection as available for reuse */
 #if APR_HAS_THREADS
-    apr_thread_mutex_unlock(ldc->lock);
+         apr_thread_mutex_unlock(ldc->lock);
 #endif
+     }
 }
 
 
 /*
  * Destroys an LDAP connection by unbinding and closing the connection to
  * the LDAP server. It is used to bring the connection back to a known
- * state after an error, and during pool cleanup.
+ * state after an error.
  */
 static apr_status_t uldap_connection_unbind(void *param)
 {
@@ -172,6 +178,9 @@
 
 /*
  * Clean up an LDAP connection by unbinding and unlocking the connection.
+ * This cleanup does not remove the util_ldap_connection_t from the 
+ * per-virtualhost list of connections, does not remove the storage
+ * for the util_ldap_connection_t or it's data, and is NOT run automatically.
  */
 static apr_status_t uldap_connection_cleanup(void *param)
 {
@@ -193,8 +202,63 @@
         /* unlock this entry */
         uldap_connection_close(ldc);
 
+     }
+
+    return APR_SUCCESS;
+}
+
+/*
+ * util_ldap_connection_remove frees all storage associated with the LDAP
+ * connection and removes it completely from the per-virtualhost list of
+ * connections
+ *
+ * The caller should hold the lock for this connection
+ */
+static apr_status_t util_ldap_connection_remove (void *param) { 
+    util_ldap_connection_t *ldc = param, *l  = NULL, *prev = NULL;
+    util_ldap_state_t *st = ldc->st;
+
+    if (!ldc) return APR_SUCCESS;
+
+    uldap_connection_unbind(ldc);
+
+#if APR_HAS_THREADS
+    apr_thread_mutex_lock(st->mutex);
+#endif
+
+    /* Remove ldc from the list */
+    for (l=st->connections; l; l=l->next) {
+        if (l == ldc) {
+            if (prev) {
+                prev->next = l->next; 
+            }
+            else { 
+                st->connections = l->next;
+            }
+            break;
+        }
+        prev = l;
+    }
+
+    /* Some unfortunate duplication between this method
+     * and uldap_connection_cleanup()
+    */
+    if (ldc->bindpw) {
+        free((void*)ldc->bindpw);
     }
+    if (ldc->binddn) {
+        free((void*)ldc->binddn);
+    }
+
+#if APR_HAS_THREADS
+    apr_thread_mutex_unlock(ldc->lock);
+    apr_thread_mutex_unlock(st->mutex);
+#endif
+
+    /* Destory the pool associated with this connection */
 
+    apr_pool_destroy(ldc->pool);   
+   
     return APR_SUCCESS;
 }
 
@@ -523,29 +587,34 @@
      * must create one.
      */
     if (!l) {
-
-        /*
-         * Add the new connection entry to the linked list. Note that we
-         * don't actually establish an LDAP connection yet; that happens
-         * the first time authentication is requested.
-         */
-        /* create the details to the pool in st */
-        l = apr_pcalloc(st->pool, sizeof(util_ldap_connection_t));
-        if (apr_pool_create(&l->pool, st->pool) != APR_SUCCESS) { 
+        apr_pool_t *newpool;
+        if (apr_pool_create(&newpool, NULL) != APR_SUCCESS) { 
             ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
                           "util_ldap: Failed to create memory pool");
 #if APR_HAS_THREADS
             apr_thread_mutex_unlock(st->mutex);
 #endif
             return NULL;
-    
         }
+ 
+
+        /*
+         * Add the new connection entry to the linked list. Note that we
+         * don't actually establish an LDAP connection yet; that happens
+         * the first time authentication is requested.
+         */
+
+        /* create the details of this connection in the new pool */
+        l = apr_pcalloc(newpool, sizeof(util_ldap_connection_t));
+        l->pool = newpool;
+        l->st = st;
+   
 #if APR_HAS_THREADS
-        apr_thread_mutex_create(&l->lock, APR_THREAD_MUTEX_DEFAULT, st->pool);
+        apr_thread_mutex_create(&l->lock, APR_THREAD_MUTEX_DEFAULT, l->pool);
         apr_thread_mutex_lock(l->lock);
 #endif
         l->bound = 0;
-        l->host = apr_pstrdup(st->pool, host);
+        l->host = apr_pstrdup(l->pool, host);
         l->port = port;
         l->deref = deref;
         util_ldap_strdup((char**)&(l->binddn), binddn);
@@ -560,6 +629,8 @@
 
         /* save away a copy of the client cert list that is presently valid */
         l->client_certs = apr_array_copy_hdr(l->pool, st->client_certs);
+
+        l->keep = 1;
 
         if (p) {
             p->next = l;