You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Matthieu Estrade <ap...@moresecurity.org> on 2003/09/12 14:01:41 UTC

[PATCH] Bug #18756: fix many problems about ldap cache using shared memory

Hi,

This is my second and new patch for ldap cache using shared memory.
I am using ldap cache with linux redhat and worker mpm.

The problems i found:

1) All shm cache information (shm addr, rmm addr, lock) are declared as
global.
--> in worker mpm, many strange problems happen.

2) shm initialisation is in child init hook
--> it try to create an shm file per child

3) ldap cache using shm is using a default filename to create shm, in
/tmp
--> if 2 different httpd are used, only one can use ldap cache using
shm.


What i did:

1) i moved all this global declaration inside the module config, then i
made all rmm_alloc and rmm_free use these module config data.

2) i moved the shm init in post_config hook, then did some code to merge
all these rmm and shm addr in all vhost module config 

3) i created a directive LDAPSharedCacheFile receiving a filename, to
create the file with apr_root_relative which will be used to create the
shm.

I tested this patch on linux rh9 with 3 vhost using ldap auth, it work
well. I also tested the ldap-status and it work well too.

Feedback are welcome !

regards,

Matthieu


Re: [PATCH] Bug #18756: fix many problems about ldap cache using shared memory

Posted by Matthieu Estrade <ap...@moresecurity.org>.
Hi Jeff,

I don't suspect problems on what you did, but i am unable to do a real 
test right now, i have to wait monday to do it at work.
I compiled it and no problems happened, i setup the ldap cache, but i am 
unable to test it with auth_ldap.

It seems there is a problem with solaris (new comment on bugzilla), but 
i haven't a solaris box to test and debug...
And i don't know if i will be able to reproduce the bug if i install a 
sol9 x86.

Matthieu

Jeff Trawick a écrit:

> Matthieu Estrade wrote:
>
>> This is my second and new patch for ldap cache using shared memory.
>> I am using ldap cache with linux redhat and worker mpm.
>
>
> Matthieu, please see an updated (and untested) patch which is 
> attached.  It solves the problems pointed out by these compiler 
> warnings introduced by your patch:
>
> > util_ldap.c: In function `util_ldap_post_config':
> > util_ldap.c:1152: warning: unsigned int format, pointer arg (arg 7)
> > util_ldap.c:1152: warning: unsigned int format, pointer arg (arg 8)
> > util_ldap_cache.c: In function `util_ldap_cache_module_kill':
> > util_ldap_cache.c:288: warning: implicit declaration of function 
> `unlink'
>
> for the unsigned int issue, which wouldn't work as expected in 64-bit 
> mode, we use %pp instead of %x
>
> for the unlink() issue, we should use apr_file_remove()
>
> Let me know if you suspect a problem with my tiny tweaks to your much 
> appreciated efforts, and we will be on the road to getting your fixes 
> committed as soon as practical.
>
>------------------------------------------------------------------------
>
>Index: include/util_ldap.h
>===================================================================
>RCS file: /home/cvs/httpd-2.0/include/util_ldap.h,v
>retrieving revision 1.11
>diff -u -r1.11 util_ldap.h
>--- include/util_ldap.h	14 Feb 2003 16:04:00 -0000	1.11
>+++ include/util_ldap.h	20 Sep 2003 12:18:39 -0000
>@@ -75,6 +75,10 @@
> #include "http_protocol.h"
> #include "http_request.h"
> 
>+#if APR_HAS_SHARED_MEMORY
>+#include "apr_rmm.h"
>+#include "apr_shm.h"
>+#endif
> 
> /* Create a set of LDAP_DECLARE(type), LDLDAP_DECLARE(type) and 
>  * LDAP_DECLARE_DATA with appropriate export and import tags for the platform
>@@ -97,7 +101,6 @@
> #define LDAP_DECLARE_DATA             __declspec(dllimport)
> #endif
> 
>-
> /*
>  * LDAP Connections
>  */
>@@ -138,9 +141,11 @@
>     apr_pool_t *pool;           /* pool from which this state is allocated */
> #if APR_HAS_THREADS
>     apr_thread_mutex_t *mutex;          /* mutex lock for the connection list */
>+    apr_thread_rwlock_t *util_ldap_cache_lock;
> #endif
> 
>     apr_size_t cache_bytes;     /* Size (in bytes) of shared memory cache */
>+    char *cache_file;		/* filename for shm */
>     long search_cache_ttl;      /* TTL for search cache */
>     long search_cache_size;     /* Size (in entries) of search cache */
>     long compare_cache_ttl;     /* TTL for compare cache */
>@@ -150,6 +155,15 @@
>     char *cert_auth_file; 
>     int   cert_file_type;
>     int   ssl_support;
>+
>+#if APR_HAS_SHARED_MEMORY
>+    apr_shm_t *cache_shm;
>+    apr_rmm_t *cache_rmm;
>+#endif
>+
>+    /* cache ald */
>+    void *util_ldap_cache;
>+
> } util_ldap_state_t;
> 
> 
>@@ -286,21 +300,21 @@
>  * @param reqsize The size of the shared memory segement to request. A size
>  *                of zero requests the max size possible from
>  *                apr_shmem_init()
>- * @deffunc void util_ldap_cache_init(apr_pool_t *p)
>+ * @deffunc void util_ldap_cache_init(apr_pool_t *p, util_ldap_state_t *st)
>  * @return The status code returned is the status code of the
>  *         apr_smmem_init() call. Regardless of the status, the cache
>  *         will be set up at least for in-process or in-thread operation.
>  */
>-apr_status_t util_ldap_cache_init(apr_pool_t *pool, apr_size_t reqsize);
>+apr_status_t util_ldap_cache_init(apr_pool_t *pool, util_ldap_state_t *st);
> 
> /**
>  * Display formatted stats for cache
>  * @param The pool to allocate the returned string from
>  * @tip This function returns a string allocated from the provided pool that describes
>  *      various stats about the cache.
>- * @deffunc char *util_ald_cache_display(apr_pool_t *pool)
>+ * @deffunc char *util_ald_cache_display(apr_pool_t *pool, util_ldap_state_t *st)
>  */
>-char *util_ald_cache_display(apr_pool_t *pool);
>+char *util_ald_cache_display(apr_pool_t *pool, util_ldap_state_t *st);
> 
> 
> /* from apr_ldap_cache_mgr.c */
>@@ -310,9 +324,9 @@
>  * @param The pool to allocate the returned string from
>  * @tip This function returns a string allocated from the provided pool that describes
>  *      various stats about the cache.
>- * @deffunc char *util_ald_cache_display(apr_pool_t *pool)
>+ * @deffunc char *util_ald_cache_display(apr_pool_t *pool, util_ldap_state_t *st)
>  */
>-char *util_ald_cache_display(apr_pool_t *pool);
>+char *util_ald_cache_display(apr_pool_t *pool, util_ldap_state_t *st);
> 
> #endif /* APU_HAS_LDAP */
> #endif /* UTIL_LDAP_H */
>Index: modules/experimental/util_ldap.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap.c,v
>retrieving revision 1.14
>diff -u -r1.14 util_ldap.c
>--- modules/experimental/util_ldap.c	4 Apr 2003 13:47:13 -0000	1.14
>+++ modules/experimental/util_ldap.c	20 Sep 2003 12:18:40 -0000
>@@ -141,6 +141,7 @@
>  */
> int util_ldap_handler(request_rec *r)
> {
>+    util_ldap_state_t *st = (util_ldap_state_t *)ap_get_module_config(r->server->module_config, &ldap_module);
> 
>     r->allowed |= (1 << M_GET);
>     if (r->method_number != M_GET)
>@@ -171,7 +172,7 @@
>              "</tr>\n", r
>             );
> 
>-    ap_rputs(util_ald_cache_display(r->pool), r);
>+    ap_rputs(util_ald_cache_display(r->pool, st), r);
> 
>     ap_rputs("</table>\n</p>\n", r);
> 
>@@ -506,9 +507,7 @@
>     LDAPMessage *res, *entry;
>     char *searchdn;
> 
>-    util_ldap_state_t *st = 
>-        (util_ldap_state_t *)ap_get_module_config(r->server->module_config,
>-        &ldap_module);
>+    util_ldap_state_t *st =  (util_ldap_state_t *)ap_get_module_config(r->server->module_config, &ldap_module);
> 
>     /* read lock this function */
>     LDAP_CACHE_LOCK_CREATE(st->pool);
>@@ -517,7 +516,7 @@
>     LDAP_CACHE_WRLOCK();
> 
>     curnode.url = url;
>-    curl = util_ald_cache_fetch(util_ldap_cache, &curnode);
>+    curl = util_ald_cache_fetch(st->util_ldap_cache, &curnode);
>     if (curl == NULL) {
>         curl = util_ald_create_caches(st, url);
>     }
>@@ -637,7 +636,7 @@
>     /* get cache entry (or create one) */
>     LDAP_CACHE_WRLOCK();
>     curnode.url = url;
>-    curl = util_ald_cache_fetch(util_ldap_cache, &curnode);
>+    curl = util_ald_cache_fetch(st->util_ldap_cache, &curnode);
>     if (curl == NULL) {
>         curl = util_ald_create_caches(st, url);
>     }
>@@ -760,7 +759,7 @@
>     /* Get the cache node for this url */
>     LDAP_CACHE_WRLOCK();
>     curnode.url = url;
>-    curl = (util_url_node_t *)util_ald_cache_fetch(util_ldap_cache, &curnode);
>+    curl = (util_url_node_t *)util_ald_cache_fetch(st->util_ldap_cache, &curnode);
>     if (curl == NULL) {
>         curl = util_ald_create_caches(st, url);
>     }
>@@ -962,6 +961,24 @@
>     return NULL;
> }
> 
>+static const char *util_ldap_set_cache_file(cmd_parms *cmd, void *dummy, const char *file)
>+{
>+    util_ldap_state_t *st = 
>+        (util_ldap_state_t *)ap_get_module_config(cmd->server->module_config, 
>+						  &ldap_module);
>+
>+    if (file)    
>+    	st->cache_file = ap_server_root_relative(st->pool, file);
>+    else
>+        st->cache_file = NULL;
>+
>+    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, cmd->server, 
>+    		      "LDAP cache: Setting shared memory cache file to %s bytes.", 
>+                      st->cache_file);
>+
>+    return NULL;
>+}
>+
> static const char *util_ldap_set_cache_ttl(cmd_parms *cmd, void *dummy, const char *ttl)
> {
>     util_ldap_state_t *st = 
>@@ -1091,22 +1108,6 @@
>     return st;
> }
> 
>-static void util_ldap_init_module(apr_pool_t *pool, server_rec *s)
>-{
>-    util_ldap_state_t *st = 
>-        (util_ldap_state_t *)ap_get_module_config(s->module_config, 
>-						  &ldap_module);
>-
>-    apr_status_t result = util_ldap_cache_init(pool, st->cache_bytes);
>-    char buf[MAX_STRING_LEN];
>-
>-    apr_strerror(result, buf, sizeof(buf));
>-    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, result, s, 
>-                      "[%d] ldap cache init: %s", 
>-                      getpid(), buf);
>-}
>-
>-
> static apr_status_t util_ldap_cleanup_module(void *data)
> {
>     server_rec *s = data;
>@@ -1128,11 +1129,39 @@
>                                  apr_pool_t *ptemp, server_rec *s)
> {
>     int rc = LDAP_SUCCESS;
>+    apr_status_t result;
>+    char buf[MAX_STRING_LEN];
> 
>-    util_ldap_state_t *st = (util_ldap_state_t *)ap_get_module_config(
>-                                                s->module_config, 
>-                                                &ldap_module);
>+    server_rec *s_vhost;
>+    util_ldap_state_t *st_vhost;
>+    
>+    util_ldap_state_t *st = (util_ldap_state_t *)ap_get_module_config(s->module_config, &ldap_module);
> 
>+    /* initializing cache if file is here and we already don't have shm addr*/
>+    if (st->cache_file && !st->cache_shm) {
>+	    result = util_ldap_cache_init(p, st);
>+	    apr_strerror(result, buf, sizeof(buf));
>+	    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, result, s, "LDAP cache init: %s",buf);
>+
>+
>+	/* merge config in all vhost */
>+	s_vhost = s->next;
>+	while (s_vhost) {
>+		ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, result, s, 
>+				"LDAP merging Shared Cache conf: shm=0x%pp rmm=0x%pp for VHOST: %s",
>+				 st->cache_shm, st->cache_rmm, s_vhost->server_hostname);
>+				 
>+		st_vhost = (util_ldap_state_t *)ap_get_module_config(s_vhost->module_config, &ldap_module);	
>+		st_vhost->cache_shm = st->cache_shm;
>+		st_vhost->cache_rmm = st->cache_rmm;
>+		st_vhost->cache_file = st->cache_file;
>+		s_vhost = s_vhost->next;
>+	}
>+
>+    } else {
>+	    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0 , s, "LDAP cache: Unable to init Shared Cache: no file");
>+    }
>+  
>         /* log the LDAP SDK used 
>         */
>     #if APR_HAS_NETSCAPE_LDAPSDK 
>@@ -1297,6 +1326,10 @@
>                   "Sets the size of the shared memory cache in bytes. "
>                   "Zero means disable the shared memory cache. Defaults to 100KB."),
> 
>+    AP_INIT_TAKE1("LDAPSharedCacheFile", util_ldap_set_cache_file, NULL, RSRC_CONF,
>+                  "Sets the file of the shared memory cache."
>+                  "Nothing means disable the shared memory cache."),
>+
>     AP_INIT_TAKE1("LDAPCacheEntries", util_ldap_set_cache_entries, NULL, RSRC_CONF,
>                   "Sets the maximum number of entries that are possible in the LDAP "
>                   "search cache. "
>@@ -1331,7 +1364,6 @@
> static void util_ldap_register_hooks(apr_pool_t *p)
> {
>     ap_hook_post_config(util_ldap_post_config,NULL,NULL,APR_HOOK_MIDDLE);
>-    ap_hook_child_init(util_ldap_init_module, NULL, NULL, APR_HOOK_MIDDLE);
>     ap_hook_handler(util_ldap_handler, NULL, NULL, APR_HOOK_MIDDLE);
> }
> 
>Index: modules/experimental/util_ldap_cache.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap_cache.c,v
>retrieving revision 1.8
>diff -u -r1.8 util_ldap_cache.c
>--- modules/experimental/util_ldap_cache.c	21 Jan 2003 10:53:09 -0000	1.8
>+++ modules/experimental/util_ldap_cache.c	20 Sep 2003 12:18:40 -0000
>@@ -85,14 +85,14 @@
>     return(strcmp(na->url, nb->url) == 0);
> }
> 
>-void *util_ldap_url_node_copy(void *c)
>+void *util_ldap_url_node_copy(util_ald_cache_t *cache, void *c)
> {
>     util_url_node_t *n = (util_url_node_t *)c;
>-    util_url_node_t *node = (util_url_node_t *)util_ald_alloc(sizeof(util_url_node_t));
>+    util_url_node_t *node = (util_url_node_t *)util_ald_alloc(cache->rmm_addr, sizeof(util_url_node_t));
> 
>     if (node) {
>-        if (!(node->url = util_ald_strdup(n->url))) {
>-            util_ald_free(node->url);
>+        if (!(node->url = util_ald_strdup(cache->rmm_addr, n->url))) {
>+            util_ald_free(cache->rmm_addr, node->url);
>             return NULL;
>         }
>         node->search_cache = n->search_cache;
>@@ -105,15 +105,15 @@
>     }
> }
> 
>-void util_ldap_url_node_free(void *n)
>+void util_ldap_url_node_free(util_ald_cache_t *cache, void *n)
> {
>     util_url_node_t *node = (util_url_node_t *)n;
> 
>-    util_ald_free(node->url);
>+    util_ald_free(cache->rmm_addr, node->url);
>     util_ald_destroy_cache(node->search_cache);
>     util_ald_destroy_cache(node->compare_cache);
>     util_ald_destroy_cache(node->dn_compare_cache);
>-    util_ald_free(node);
>+    util_ald_free(cache->rmm_addr, node);
> }
> 
> /* ------------------------------------------------------------------ */
>@@ -131,10 +131,10 @@
> 		  ((util_search_node_t *)b)->username) == 0);
> }
> 
>-void *util_ldap_search_node_copy(void *c)
>+void *util_ldap_search_node_copy(util_ald_cache_t *cache, void *c)
> {
>     util_search_node_t *node = (util_search_node_t *)c;
>-    util_search_node_t *newnode = util_ald_alloc(sizeof(util_search_node_t));
>+    util_search_node_t *newnode = util_ald_alloc(cache->rmm_addr, sizeof(util_search_node_t));
> 
>     /* safety check */
>     if (newnode) {
>@@ -144,13 +144,13 @@
>             int k = 0;
>             int i = 0;
>             while (node->vals[k++]);
>-            if (!(newnode->vals = util_ald_alloc(sizeof(char *) * (k+1)))) {
>-                util_ldap_search_node_free(newnode);
>+            if (!(newnode->vals = util_ald_alloc(cache->rmm_addr, sizeof(char *) * (k+1)))) {
>+                util_ldap_search_node_free(cache, newnode);
>                 return NULL;
>             }
>             while (node->vals[i]) {
>-                if (!(newnode->vals[i] = util_ald_strdup(node->vals[i]))) {
>-                    util_ldap_search_node_free(newnode);
>+                if (!(newnode->vals[i] = util_ald_strdup(cache->rmm_addr, node->vals[i]))) {
>+                    util_ldap_search_node_free(cache, newnode);
>                     return NULL;
>                 }
>                 i++;
>@@ -159,10 +159,10 @@
>         else {
>             newnode->vals = NULL;
>         }
>-        if (!(newnode->username = util_ald_strdup(node->username)) ||
>-            !(newnode->dn = util_ald_strdup(node->dn)) ||
>-            !(newnode->bindpw = util_ald_strdup(node->bindpw)) ) {
>-            util_ldap_search_node_free(newnode);
>+        if (!(newnode->username = util_ald_strdup(cache->rmm_addr, node->username)) ||
>+            !(newnode->dn = util_ald_strdup(cache->rmm_addr, node->dn)) ||
>+            !(newnode->bindpw = util_ald_strdup(cache->rmm_addr, node->bindpw)) ) {
>+            util_ldap_search_node_free(cache, newnode);
>             return NULL;
>         }
>         newnode->lastbind = node->lastbind;
>@@ -171,20 +171,20 @@
>     return (void *)newnode;
> }
> 
>-void util_ldap_search_node_free(void *n)
>+void util_ldap_search_node_free(util_ald_cache_t *cache, void *n)
> {
>     int i = 0;
>     util_search_node_t *node = (util_search_node_t *)n;
>     if (node->vals) {
>         while (node->vals[i]) {
>-            util_ald_free(node->vals[i++]);
>+            util_ald_free(cache->rmm_addr, node->vals[i++]);
>         }
>-        util_ald_free(node->vals);
>+        util_ald_free(cache->rmm_addr, node->vals);
>     }
>-    util_ald_free(node->username);
>-    util_ald_free(node->dn);
>-    util_ald_free(node->bindpw);
>-    util_ald_free(node);
>+    util_ald_free(cache->rmm_addr, node->username);
>+    util_ald_free(cache->rmm_addr, node->dn);
>+    util_ald_free(cache->rmm_addr, node->bindpw);
>+    util_ald_free(cache->rmm_addr, node);
> }
> 
> /* ------------------------------------------------------------------ */
>@@ -204,16 +204,16 @@
> 	    strcmp(na->value, nb->value) == 0);
> }
> 
>-void *util_ldap_compare_node_copy(void *c)
>+void *util_ldap_compare_node_copy(util_ald_cache_t *cache, void *c)
> {
>     util_compare_node_t *n = (util_compare_node_t *)c;
>-    util_compare_node_t *node = (util_compare_node_t *)util_ald_alloc(sizeof(util_compare_node_t));
>+    util_compare_node_t *node = (util_compare_node_t *)util_ald_alloc(cache->rmm_addr, sizeof(util_compare_node_t));
> 
>     if (node) {
>-        if (!(node->dn = util_ald_strdup(n->dn)) ||
>-            !(node->attrib = util_ald_strdup(n->attrib)) ||
>-            !(node->value = util_ald_strdup(n->value))) {
>-            util_ldap_compare_node_free(node);
>+        if (!(node->dn = util_ald_strdup(cache->rmm_addr, n->dn)) ||
>+            !(node->attrib = util_ald_strdup(cache->rmm_addr, n->attrib)) ||
>+            !(node->value = util_ald_strdup(cache->rmm_addr, n->value))) {
>+            util_ldap_compare_node_free(cache, node);
>             return NULL;
>         }
>         node->lastcompare = n->lastcompare;
>@@ -225,13 +225,13 @@
>     }
> }
> 
>-void util_ldap_compare_node_free(void *n)
>+void util_ldap_compare_node_free(util_ald_cache_t *cache, void *n)
> {
>     util_compare_node_t *node = (util_compare_node_t *)n;
>-    util_ald_free(node->dn);
>-    util_ald_free(node->attrib);
>-    util_ald_free(node->value);
>-    util_ald_free(node);
>+    util_ald_free(cache->rmm_addr, node->dn);
>+    util_ald_free(cache->rmm_addr, node->attrib);
>+    util_ald_free(cache->rmm_addr, node->value);
>+    util_ald_free(cache->rmm_addr, node);
> }
> 
> /* ------------------------------------------------------------------ */
>@@ -247,14 +247,14 @@
> 		   ((util_dn_compare_node_t *)b)->reqdn) == 0);
> }
> 
>-void *util_ldap_dn_compare_node_copy(void *c)
>+void *util_ldap_dn_compare_node_copy(util_ald_cache_t *cache, void *c)
> {
>     util_dn_compare_node_t *n = (util_dn_compare_node_t *)c;
>-    util_dn_compare_node_t *node = (util_dn_compare_node_t *)util_ald_alloc(sizeof(util_dn_compare_node_t));
>+    util_dn_compare_node_t *node = (util_dn_compare_node_t *)util_ald_alloc(cache->rmm_addr, sizeof(util_dn_compare_node_t));
>     if (node) {
>-        if (!(node->reqdn = util_ald_strdup(n->reqdn)) ||
>-            !(node->dn = util_ald_strdup(n->dn))) {
>-            util_ldap_dn_compare_node_free(node);
>+        if (!(node->reqdn = util_ald_strdup(cache->rmm_addr, n->reqdn)) ||
>+            !(node->dn = util_ald_strdup(cache->rmm_addr, n->dn))) {
>+            util_ldap_dn_compare_node_free(cache, node);
>             return NULL;
>         }
>         return node;
>@@ -264,12 +264,12 @@
>     }
> }
> 
>-void util_ldap_dn_compare_node_free(void *n)
>+void util_ldap_dn_compare_node_free(util_ald_cache_t *cache, void *n)
> {
>     util_dn_compare_node_t *node = (util_dn_compare_node_t *)n;
>-    util_ald_free(node->reqdn);
>-    util_ald_free(node->dn);
>-    util_ald_free(node);
>+    util_ald_free(cache->rmm_addr, node->reqdn);
>+    util_ald_free(cache->rmm_addr, node->dn);
>+    util_ald_free(cache->rmm_addr, node);
> }
> 
> 
>@@ -279,42 +279,48 @@
> 
> apr_status_t util_ldap_cache_module_kill(void *data)
> {
>+    util_ldap_state_t *st = (util_ldap_state_t *)data;
>+
> #if APR_HAS_SHARED_MEMORY
>-    if (util_ldap_shm != NULL) {
>-        apr_status_t result = apr_shm_destroy(util_ldap_shm);
>-        util_ldap_shm = NULL;
>+    if (st->cache_shm != NULL) {
>+        apr_status_t result = apr_shm_destroy(st->cache_shm);
>+        st->cache_shm = NULL;
>+	apr_file_remove(st->cache_file, st->pool);
>         return result;
>     }
> #endif
>-    util_ald_destroy_cache(util_ldap_cache);
>+    util_ald_destroy_cache(st->util_ldap_cache);
>     return APR_SUCCESS;
> }
> 
>-apr_status_t util_ldap_cache_init(apr_pool_t *pool, apr_size_t reqsize)
>+apr_status_t util_ldap_cache_init(apr_pool_t *pool, util_ldap_state_t *st)
> {
> #if APR_HAS_SHARED_MEMORY
>     apr_status_t result;
> 
>-    result = apr_shm_create(&util_ldap_shm, reqsize, MODLDAP_SHMEM_CACHE, pool);
>+    if (!st->cache_file) {
>+    	return -1;
>+    }
>+
>+    result = apr_shm_create(&st->cache_shm, st->cache_bytes, st->cache_file, st->pool);
>     if (result == EEXIST) {
>         /*
>          * The cache could have already been created (i.e. we may be a child process).  See
>          * if we can attach to the existing shared memory
>          */
>-        result = apr_shm_attach(&util_ldap_shm, MODLDAP_SHMEM_CACHE, pool);
>+        result = apr_shm_attach(&st->cache_shm, st->cache_file, st->pool);
>     } 
>     if (result != APR_SUCCESS) {
>         return result;
>     }
> 
>     /* This will create a rmm "handler" to get into the shared memory area */
>-    apr_rmm_init(&util_ldap_rmm, NULL,
>-			(void *)apr_shm_baseaddr_get(util_ldap_shm), reqsize, pool);
>+    apr_rmm_init(&st->cache_rmm, NULL, (void *)apr_shm_baseaddr_get(st->cache_shm), st->cache_bytes, st->pool);
> #endif
> 
>-    apr_pool_cleanup_register(pool, NULL, util_ldap_cache_module_kill, apr_pool_cleanup_null);
>+    apr_pool_cleanup_register(st->pool, st , util_ldap_cache_module_kill, apr_pool_cleanup_null);
> 
>-    util_ldap_cache = util_ald_create_cache(50,
>+    st->util_ldap_cache = util_ald_create_cache(st,
> 				     util_ldap_url_node_hash,
> 				     util_ldap_url_node_compare,
> 				     util_ldap_url_node_copy,
>Index: modules/experimental/util_ldap_cache.h
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap_cache.h,v
>retrieving revision 1.6
>diff -u -r1.6 util_ldap_cache.h
>--- modules/experimental/util_ldap_cache.h	21 Jan 2003 10:53:09 -0000	1.6
>+++ modules/experimental/util_ldap_cache.h	20 Sep 2003 12:18:40 -0000
>@@ -77,16 +77,18 @@
>     struct util_cache_node_t *next;
> } util_cache_node_t;
> 
>-typedef struct util_ald_cache_t {
>-    unsigned long size;		/* Size of cache array */
>-    unsigned long maxentries;	/* Maximum number of cache entries */
>-    unsigned long numentries;	/* Current number of cache entries */
>-    unsigned long fullmark;	/* Used to keep track of when cache becomes 3/4 full */
>-    apr_time_t marktime;	/* Time that the cache became 3/4 full */
>-    unsigned long (*hash)(void *);  /* Func to hash the payload */
>-    int (*compare)(void *, void *); /* Func to compare two payloads */
>-    void * (*copy)(void *);	/* Func to alloc mem and copy payload to new mem */
>-    void (*free)(void *);	/* Func to free mem used by the payload */
>+typedef struct util_ald_cache util_ald_cache_t;
>+
>+struct util_ald_cache {
>+    unsigned long size;			/* Size of cache array */
>+    unsigned long maxentries;		/* Maximum number of cache entries */
>+    unsigned long numentries;		/* Current number of cache entries */
>+    unsigned long fullmark;		/* Used to keep track of when cache becomes 3/4 full */
>+    apr_time_t marktime;		/* Time that the cache became 3/4 full */
>+    unsigned long (*hash)(void *);  		/* Func to hash the payload */
>+    int (*compare)(void *, void *); 		/* Func to compare two payloads */
>+    void * (*copy)(util_ald_cache_t *cache, void *);	/* Func to alloc mem and copy payload to new mem */
>+    void (*free)(util_ald_cache_t *cache, void *);		/* Func to free mem used by the payload */
>     util_cache_node_t **nodes;
> 
>     unsigned long numpurges;	/* No. of times the cache has been purged */
>@@ -100,24 +102,23 @@
>     unsigned long hits;		/* Number of cache hits */
>     unsigned long inserts;	/* Number of inserts */
>     unsigned long removes;	/* Number of removes */
>-} util_ald_cache_t;
> 
> #if APR_HAS_SHARED_MEMORY
>-apr_shm_t *util_ldap_shm;
>-apr_rmm_t *util_ldap_rmm;
>+    apr_shm_t *shm_addr;
>+    apr_rmm_t *rmm_addr;
> #endif
>-util_ald_cache_t *util_ldap_cache;
>+
>+};
> 
> #if APR_HAS_THREADS
>-apr_thread_rwlock_t *util_ldap_cache_lock;
> #define LDAP_CACHE_LOCK_CREATE(p) \
>-    if (!util_ldap_cache_lock) apr_thread_rwlock_create(&util_ldap_cache_lock, p)
>+    if (!st->util_ldap_cache_lock) apr_thread_rwlock_create(&st->util_ldap_cache_lock, st->pool)
> #define LDAP_CACHE_WRLOCK() \
>-    apr_thread_rwlock_wrlock(util_ldap_cache_lock)
>+    apr_thread_rwlock_wrlock(st->util_ldap_cache_lock)
> #define LDAP_CACHE_UNLOCK() \
>-    apr_thread_rwlock_unlock(util_ldap_cache_lock)
>+    apr_thread_rwlock_unlock(st->util_ldap_cache_lock)
> #define LDAP_CACHE_RDLOCK() \
>-    apr_thread_rwlock_rdlock(util_ldap_cache_lock)
>+    apr_thread_rwlock_rdlock(st->util_ldap_cache_lock)
> #else
> #define LDAP_CACHE_LOCK_CREATE(p)
> #define LDAP_CACHE_WRLOCK()
>@@ -192,35 +193,39 @@
> /* util_ldap_cache.c */
> unsigned long util_ldap_url_node_hash(void *n);
> int util_ldap_url_node_compare(void *a, void *b);
>-void *util_ldap_url_node_copy(void *c);
>-void util_ldap_url_node_free(void *n);
>+void *util_ldap_url_node_copy(util_ald_cache_t *cache, void *c);
>+void util_ldap_url_node_free(util_ald_cache_t *cache, void *n);
> unsigned long util_ldap_search_node_hash(void *n);
> int util_ldap_search_node_compare(void *a, void *b);
>-void *util_ldap_search_node_copy(void *c);
>-void util_ldap_search_node_free(void *n);
>+void *util_ldap_search_node_copy(util_ald_cache_t *cache, void *c);
>+void util_ldap_search_node_free(util_ald_cache_t *cache, void *n);
> unsigned long util_ldap_compare_node_hash(void *n);
> int util_ldap_compare_node_compare(void *a, void *b);
>-void *util_ldap_compare_node_copy(void *c);
>-void util_ldap_compare_node_free(void *n);
>+void *util_ldap_compare_node_copy(util_ald_cache_t *cache, void *c);
>+void util_ldap_compare_node_free(util_ald_cache_t *cache, void *n);
> unsigned long util_ldap_dn_compare_node_hash(void *n);
> int util_ldap_dn_compare_node_compare(void *a, void *b);
>-void *util_ldap_dn_compare_node_copy(void *c);
>-void util_ldap_dn_compare_node_free(void *n);
>+void *util_ldap_dn_compare_node_copy(util_ald_cache_t *cache, void *c);
>+void util_ldap_dn_compare_node_free(util_ald_cache_t *cache, void *n);
> 
> 
> /* util_ldap_cache_mgr.c */
> 
>-void util_ald_free(const void *ptr);
>-void *util_ald_alloc(unsigned long size);
>-const char *util_ald_strdup(const char *s);
>+/* Cache alloc and free function, dealing or not with shm */
>+void util_ald_free(apr_rmm_t *rmm_addr, const void *ptr);
>+void *util_ald_alloc(apr_rmm_t *rmm_addr, unsigned long size);
>+const char *util_ald_strdup(apr_rmm_t *rmm_addr, const char *s);
>+
>+/* Cache managing function */
> unsigned long util_ald_hash_string(int nstr, ...);
> void util_ald_cache_purge(util_ald_cache_t *cache);
> util_url_node_t *util_ald_create_caches(util_ldap_state_t *s, const char *url);
>-util_ald_cache_t *util_ald_create_cache(unsigned long maxentries,
>+util_ald_cache_t *util_ald_create_cache(util_ldap_state_t *st,
>                                 unsigned long (*hashfunc)(void *), 
>                                 int (*comparefunc)(void *, void *),
>-                                void * (*copyfunc)(void *),
>-                                void (*freefunc)(void *));
>+                                void * (*copyfunc)(util_ald_cache_t *cache, void *),
>+                                void (*freefunc)(util_ald_cache_t *cache, void *));
>+                                
> void util_ald_destroy_cache(util_ald_cache_t *cache);
> void *util_ald_cache_fetch(util_ald_cache_t *cache, void *payload);
> void util_ald_cache_insert(util_ald_cache_t *cache, void *payload);
>Index: modules/experimental/util_ldap_cache_mgr.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap_cache_mgr.c,v
>retrieving revision 1.5
>diff -u -r1.5 util_ldap_cache_mgr.c
>--- modules/experimental/util_ldap_cache_mgr.c	21 Jan 2003 10:53:09 -0000	1.5
>+++ modules/experimental/util_ldap_cache_mgr.c	20 Sep 2003 12:18:41 -0000
>@@ -112,14 +112,16 @@
>   0
> };
> 
>-void util_ald_free(const void *ptr)
>+void util_ald_free(apr_rmm_t *rmm_addr, const void *ptr)
> {
> #if APR_HAS_SHARED_MEMORY
>-    if (util_ldap_shm) {
>+    if (rmm_addr) {
>         if (ptr)
>-            apr_rmm_free(util_ldap_rmm, apr_rmm_offset_get(util_ldap_rmm, (void *)ptr));
>+            /* Free in shared memory */
>+            apr_rmm_free(rmm_addr, apr_rmm_offset_get(rmm_addr, (void *)ptr));
>     } else {
>         if (ptr)
>+            /* Cache shm is not used */
>             free((void *)ptr);
>     }
> #else
>@@ -128,14 +130,16 @@
> #endif
> }
> 
>-void *util_ald_alloc(unsigned long size)
>+void *util_ald_alloc(apr_rmm_t *rmm_addr, unsigned long size)
> {
>     if (0 == size)
>         return NULL;
> #if APR_HAS_SHARED_MEMORY
>-    if (util_ldap_shm) {
>-        return (void *)apr_rmm_addr_get(util_ldap_rmm, apr_rmm_calloc(util_ldap_rmm, size));
>+    if (rmm_addr) {
>+        /* allocate from shared memory */
>+        return (void *)apr_rmm_addr_get(rmm_addr, apr_rmm_calloc(rmm_addr, size));
>     } else {
>+    	/* Cache shm is not used */
>         return (void *)calloc(sizeof(char), size);
>     }
> #else
>@@ -143,11 +147,12 @@
> #endif
> }
> 
>-const char *util_ald_strdup(const char *s)
>+const char *util_ald_strdup(apr_rmm_t *rmm_addr, const char *s)
> {
> #if APR_HAS_SHARED_MEMORY
>-    if (util_ldap_shm) {
>-        char *buf = (char *)apr_rmm_addr_get(util_ldap_rmm, apr_rmm_calloc(util_ldap_rmm, strlen(s)+1));
>+    if (rmm_addr) {
>+    	/* allocate from shared memory */
>+        char *buf = (char *)apr_rmm_addr_get(rmm_addr, apr_rmm_calloc(rmm_addr, strlen(s)+1));
>         if (buf) {
>             strcpy(buf, s);
>             return buf;
>@@ -156,6 +161,7 @@
>             return NULL;
>         }
>     } else {
>+        /* Cache shm is not used */
>         return strdup(s);
>     }
> #else
>@@ -217,8 +223,8 @@
>         while (p != NULL) {
>             if (p->add_time < cache->marktime) {
> 	        q = p->next;
>-	        (*cache->free)(p->payload);
>-	        util_ald_free(p);
>+	        (*cache->free)(cache, p->payload);
>+	        util_ald_free(cache->rmm_addr, p);
> 	        cache->numentries--;
> 	        cache->npurged++;
> 	        p = q;
>@@ -247,17 +253,17 @@
>     util_ald_cache_t *dn_compare_cache;
> 
>     /* create the three caches */
>-    search_cache = util_ald_create_cache(st->search_cache_size,
>+    search_cache = util_ald_create_cache(st,
> 					  util_ldap_search_node_hash,
> 					  util_ldap_search_node_compare,
> 					  util_ldap_search_node_copy,
> 					  util_ldap_search_node_free);
>-    compare_cache = util_ald_create_cache(st->compare_cache_size,
>+    compare_cache = util_ald_create_cache(st,
> 					   util_ldap_compare_node_hash,
> 					   util_ldap_compare_node_compare,
> 					   util_ldap_compare_node_copy,
> 					   util_ldap_compare_node_free);
>-    dn_compare_cache = util_ald_create_cache(st->compare_cache_size,
>+    dn_compare_cache = util_ald_create_cache(st,
> 					      util_ldap_dn_compare_node_hash,
> 					      util_ldap_dn_compare_node_compare,
> 					      util_ldap_dn_compare_node_copy,
>@@ -272,7 +278,7 @@
>         curl->compare_cache = compare_cache;
>         curl->dn_compare_cache = dn_compare_cache;
> 
>-        util_ald_cache_insert(util_ldap_cache, curl);
>+        util_ald_cache_insert(st->util_ldap_cache, curl);
> 
>     }
> 
>@@ -280,32 +286,32 @@
> }
> 
> 
>-util_ald_cache_t *util_ald_create_cache(unsigned long maxentries,
>+util_ald_cache_t *util_ald_create_cache(util_ldap_state_t *st,
>                                 unsigned long (*hashfunc)(void *), 
>                                 int (*comparefunc)(void *, void *),
>-                                void * (*copyfunc)(void *),
>-                                void (*freefunc)(void *))
>+                                void * (*copyfunc)(util_ald_cache_t *cache, void *),
>+                                void (*freefunc)(util_ald_cache_t *cache, void *))
> {
>     util_ald_cache_t *cache;
>     unsigned long i;
> 
>-    if (maxentries <= 0)
>+    if (st->search_cache_size <= 0)
>         return NULL;
> 
>-    cache = (util_ald_cache_t *)util_ald_alloc(sizeof(util_ald_cache_t));
>+    cache = (util_ald_cache_t *)util_ald_alloc(st->cache_rmm, sizeof(util_ald_cache_t));
>     if (!cache)
>         return NULL;
> 
>-    cache->maxentries = maxentries;
>+    cache->maxentries = st->search_cache_size;
>     cache->numentries = 0;
>-    cache->size = maxentries / 3;
>+    cache->size = st->search_cache_size / 3;
>     if (cache->size < 64) cache->size = 64;
>         for (i = 0; primes[i] && primes[i] < cache->size; ++i) ;
>             cache->size = primes[i]? primes[i] : primes[i-1];
> 
>-    cache->nodes = (util_cache_node_t **)util_ald_alloc(cache->size * sizeof(util_cache_node_t *));
>+    cache->nodes = (util_cache_node_t **)util_ald_alloc(cache->rmm_addr, cache->size * sizeof(util_cache_node_t *));
>     if (!cache->nodes) {
>-        util_ald_free(cache);
>+        util_ald_free(cache->rmm_addr, cache);
>         return NULL;
>     }
> 
>@@ -345,13 +351,13 @@
>         q = NULL;
>         while (p != NULL) {
>             q = p->next;
>-           (*cache->free)(p->payload);
>-           util_ald_free(p);
>+           (*cache->free)(cache, p->payload);
>+           util_ald_free(cache->rmm_addr, p);
>            p = q;
>         }
>     }
>-    util_ald_free(cache->nodes);
>-    util_ald_free(cache);
>+    util_ald_free(cache->rmm_addr, cache->nodes);
>+    util_ald_free(cache->rmm_addr, cache);
> }
> 
> void *util_ald_cache_fetch(util_ald_cache_t *cache, void *payload)
>@@ -392,9 +398,9 @@
> 
>     cache->inserts++;
>     hashval = (*cache->hash)(payload) % cache->size;
>-    node = (util_cache_node_t *)util_ald_alloc(sizeof(util_cache_node_t));
>+    node = (util_cache_node_t *)util_ald_alloc(cache->rmm_addr, sizeof(util_cache_node_t));
>     node->add_time = apr_time_now();
>-    node->payload = (*cache->copy)(payload);
>+    node->payload = (*cache->copy)(cache, payload);
>     node->next = cache->nodes[hashval];
>     cache->nodes[hashval] = node;
>     if (++cache->numentries == cache->fullmark) 
>@@ -431,8 +437,8 @@
>         /* We found the node and it's not the first in the list */
>         q->next = p->next;
>     }
>-    (*cache->free)(p->payload);
>-    util_ald_free(p);
>+    (*cache->free)(cache, p->payload);
>+    util_ald_free(cache->rmm_addr, p);
>     cache->numentries--;
> }
> 
>@@ -499,16 +505,19 @@
>     return buf;
> }
> 
>-char *util_ald_cache_display(apr_pool_t *pool)
>+char *util_ald_cache_display(apr_pool_t *pool, util_ldap_state_t *st)
> {
>     unsigned long i;
>     char *buf, *t1, *t2, *t3;
> 
>+    util_ald_cache_t *util_ldap_cache = st->util_ldap_cache;
>+
>+
>     if (!util_ldap_cache) {
>         return "<tr valign='top'><td nowrap colspan=7>Cache has not been enabled/initialised.</td></tr>";
>     }
> 
>-    buf = util_ald_cache_display_stats(pool, util_ldap_cache, "LDAP URL Cache");
>+    buf = util_ald_cache_display_stats(pool, st->util_ldap_cache, "LDAP URL Cache");
> 
>     for (i=0; i < util_ldap_cache->size; ++i) {
>         util_cache_node_t *p;
>  
>



Re: [PATCH] Bug #18756: fix many problems about ldap cache using shared memory

Posted by Jeff Trawick <tr...@attglobal.net>.
Matthieu Estrade wrote:

> This is my second and new patch for ldap cache using shared memory.
> I am using ldap cache with linux redhat and worker mpm.

Matthieu, please see an updated (and untested) patch which is attached. 
  It solves the problems pointed out by these compiler warnings 
introduced by your patch:

 > util_ldap.c: In function `util_ldap_post_config':
 > util_ldap.c:1152: warning: unsigned int format, pointer arg (arg 7)
 > util_ldap.c:1152: warning: unsigned int format, pointer arg (arg 8)
 > util_ldap_cache.c: In function `util_ldap_cache_module_kill':
 > util_ldap_cache.c:288: warning: implicit declaration of function `unlink'

for the unsigned int issue, which wouldn't work as expected in 64-bit 
mode, we use %pp instead of %x

for the unlink() issue, we should use apr_file_remove()

Let me know if you suspect a problem with my tiny tweaks to your much 
appreciated efforts, and we will be on the road to getting your fixes 
committed as soon as practical.


Re: [PATCH] Bug #18756: fix many problems about ldap cache using shared memory

Posted by Graham Leggett <mi...@sharp.fm>.
Matthieu Estrade wrote:

> 1) i moved all this global declaration inside the module config, then i
> made all rmm_alloc and rmm_free use these module config data.
> 
> 2) i moved the shm init in post_config hook, then did some code to merge
> all these rmm and shm addr in all vhost module config 
> 
> 3) i created a directive LDAPSharedCacheFile receiving a filename, to
> create the file with apr_root_relative which will be used to create the
> shm.

+1 from me - I'll commit to 2.1 in the next few days assuming there are 
no objections?

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."