Fixed LDAP cleanup on graceful restarts. LDAP connections are now

cleaned up when the connection pool pool is cleaned up.
PR:
Obtained from:
Submitted by:
Reviewed by:

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@90336 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index fb4694b7cd1c08292614d88052cb50bfc632285a..7231b6398b88ca74f2aef950d245072267fb37b1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 Changes with Apache 2.0.25-dev
 
+  *) Fixed LDAP cleanup on graceful restarts. LDAP connections are now
+     cleaned up when the connection pool pool is cleaned up.
+     [Graham Leggett]
+
   *) Fix a minor issue with Jeff Trawick's mod_include
      patch. Without this patch, the code will just allocate
      more bytes in get_combined_directive than are needed.

diff --git a/include/util_ldap.h b/include/util_ldap.h
index 10dc2eabe559bd536bc16308f03ece04f274ad89..bd9327e26f7a344be90b2f989b6dbc47daebafe5 100644 (file)
--- a/include/util_ldap.h
+++ b/include/util_ldap.h
@@ -90,6 +90,7 @@ typedef enum {
 /* Structure representing an LDAP connection */
 typedef struct util_ldap_connection_t {
     LDAP *ldap;
+    apr_pool_t *pool;                  /* Pool from which this connection is created */
     apr_lock_t *lock;                  /* Lock to indicate this connection is in use */
     int bound;                         /* Flag to indicate whether this connection is bound yet */
 
@@ -145,7 +146,7 @@ int util_ldap_connection_open(util_ldap_connection_t *ldc);
 /**
  * Close a connection to an LDAP server
  * @param ldc A structure containing the expanded details of the server
-              that was connected.
+ *            that was connected.
  * @tip This function unbinds from the LDAP server, and clears ldc->ldap.
  *      It is possible to rebind to this server again using the same ldc
  *      structure, using apr_ldap_open_connection().
@@ -153,6 +154,16 @@ int util_ldap_connection_open(util_ldap_connection_t *ldc);
  */
 void util_ldap_connection_close(util_ldap_connection_t *ldc);
 
+/**
+ * Destroy a connection to an LDAP server
+ * @param ldc A structure containing the expanded details of the server
+ *            that was connected.
+ * @tip This function is registered with the pool cleanup to close down the
+ *      LDAP connections when the server is finished with them.
+ * @deffunc apr_status_t util_ldap_connection_destroy(util_ldap_connection_t *ldc)
+ */
+apr_status_t util_ldap_connection_destroy(void *param);
+
 /**
  * Find a connection in a list of connections
  * @param r The request record

diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c
index 02689bfa125a2049d2d7cb3d88fcdfc646a48893..40c804a22ff7e33a4ff0e35b8d4af46d7fc63b58 100644 (file)
--- a/modules/ldap/util_ldap.c
+++ b/modules/ldap/util_ldap.c
@@ -182,14 +182,9 @@ int util_ldap_handler(request_rec *r)
 
 
 /*
- * Closes an LDAP connection by unbinding. Sets the boundas value for the
- * http connection config record and clears the bound dn string in the
- * global connection record. The next time util_ldap_connection_open() is
- * called, the connection will be recreated.
- *
- * If the log parameter is set, adds a debug entry to the log that the
- * server was down and it's reconnecting.
- *
+ * Closes an LDAP connection by unlocking it. The next time
+ * util_ldap_connection_find() is called this connection will be
+ * available for reuse.
  */
 void util_ldap_connection_close(util_ldap_connection_t *ldc)
 {
@@ -205,19 +200,32 @@ void util_ldap_connection_close(util_ldap_connection_t *ldc)
      * we don't have to...
      */
 
-    /* unbinding from the LDAP server */
-/* FIXME: add this to pool cleanup instead */ 
+    /* mark our connection as available for reuse */
+    apr_lock_release(ldc->lock);
+
+}
+
+
 /*
+ * Destroys an LDAP connection by unbinding. This function is registered
+ * with the pool cleanup function - causing the LDAP connections to be
+ * shut down cleanly on thread exit.
+ */
+apr_status_t util_ldap_connection_destroy(void *param)
+{
+    util_ldap_connection_t *ldc = param;
+
+    /* unbinding from the LDAP server */
     if (ldc->ldap) {
         ldap_unbind_s(ldc->ldap);
         ldc->bound = 0;
         ldc->ldap = NULL;
     }
-*/
 
-    /* mark our connection as available for reuse */
+    /* release the lock we were using */
     apr_lock_release(ldc->lock);
 
+    return APR_SUCCESS;
 }
 
 
@@ -226,8 +234,6 @@ void util_ldap_connection_close(util_ldap_connection_t *ldc)
  * connected (i.e. ldc->ldap is non-NULL.) Does not bind if already bound.
  *
  * Returns LDAP_SUCCESS on success; and an error code on failure
- * XXX FIXME: Make these APR error codes, not LDAP error codes
- *
  */
 int util_ldap_connection_open(util_ldap_connection_t *ldc)
 {
@@ -251,6 +257,11 @@ start_over:
             return -1;
         }
 
+       /* add the cleanup to the pool */
+        apr_pool_cleanup_register(ldc->pool, ldc,
+                                  util_ldap_connection_destroy,
+                                  apr_pool_cleanup_null);
+
         /* Set the alias dereferencing option */
 #if LDAP_VERSION_MAX == 2
         ldc->ldap->ld_deref = ldc->deref;
@@ -443,6 +454,7 @@ util_ldap_connection_t *util_ldap_connection_find(request_rec *r, const char *ho
         l = apr_pcalloc(st->pool, sizeof(util_ldap_connection_t));
         apr_lock_create(&l->lock, APR_MUTEX, APR_INTRAPROCESS, NULL, st->pool);
         apr_lock_acquire(l->lock);
+        l->pool = st->pool;
         l->bound = 0;
         l->host = apr_pstrdup(st->pool, host);
         l->port = port;