This patch should fix the shared memory issues on Linux and Solaris. But this code could use some more review and testing. I would like to get mod_auth_ldap and mod_ldap out of experimental if this proves to stabilize the module. On a related issue, I would also like to backport these patches to the 2.0 tree. Does the change to the util_ldap_state_t structure in util_ldap.h require a MMN bump even though mod_ldap is an experimental module?
Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com >>> [EMAIL PROTECTED] Friday, June 25, 2004 1:56:35 PM >>> bnicholes 2004/06/25 12:56:35 Modified: include util_ldap.h modules/experimental util_ldap.c util_ldap_cache.c util_ldap_cache.h Log: Replace the thread reader/writer lock that protects the shared memory cache to a global mutex so that the shared memory is protected across processes. Revision Changes Path 1.20 +2 -1 httpd-2.0/include/util_ldap.h Index: util_ldap.h =================================================================== RCS file: /home/cvs/httpd-2.0/include/util_ldap.h,v retrieving revision 1.19 retrieving revision 1.20 diff -u -r1.19 -r1.20 --- util_ldap.h 16 Jun 2004 23:25:27 -0000 1.19 +++ util_ldap.h 25 Jun 2004 19:56:35 -0000 1.20 @@ -102,8 +102,8 @@ 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_global_mutex_t *util_ldap_cache_lock; apr_size_t cache_bytes; /* Size (in bytes) of shared memory cache */ char *cache_file; /* filename for shm */ @@ -124,6 +124,7 @@ /* cache ald */ void *util_ldap_cache; + char *lock_file; /* filename for shm lock mutex */ } util_ldap_state_t; 1.34 +77 -25 httpd-2.0/modules/experimental/util_ldap.c Index: util_ldap.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap.c,v retrieving revision 1.33 retrieving revision 1.34 diff -u -r1.33 -r1.34 --- util_ldap.c 17 Jun 2004 17:19:01 -0000 1.33 +++ util_ldap.c 25 Jun 2004 19:56:35 -0000 1.34 @@ -88,6 +88,11 @@ "\"http://www.w3.org/TR/REC-html40/frameset.dtd\";>\n" #endif +#define LDAP_CACHE_LOCK() \ + apr_global_mutex_lock(st->util_ldap_cache_lock) +#define LDAP_CACHE_UNLOCK() \ + apr_global_mutex_unlock(st->util_ldap_cache_lock) + static void util_ldap_strdup (char **str, const char *newstr) { @@ -498,11 +503,8 @@ 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); - /* get cache entry (or create one) */ - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); curnode.url = url; curl = util_ald_cache_fetch(st->util_ldap_cache, &curnode); @@ -526,7 +528,7 @@ if (curl) { /* no - it's a server side compare */ - LDAP_CACHE_RDLOCK(); + LDAP_CACHE_LOCK(); /* is it in the compare cache? */ newnode.reqdn = (char *)reqdn; @@ -581,7 +583,7 @@ else { if (curl) { /* compare successful - add to the compare cache */ - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); newnode.reqdn = (char *)reqdn; newnode.dn = (char *)dn; @@ -625,11 +627,8 @@ (util_ldap_state_t *)ap_get_module_config(r->server->module_config, &ldap_module); - /* read lock this function */ - LDAP_CACHE_LOCK_CREATE(st->pool); - /* get cache entry (or create one) */ - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); curnode.url = url; curl = util_ald_cache_fetch(st->util_ldap_cache, &curnode); if (curl == NULL) { @@ -639,7 +638,7 @@ if (curl) { /* make a comparison to the cache */ - LDAP_CACHE_RDLOCK(); + LDAP_CACHE_LOCK(); curtime = apr_time_now(); the_compare_node.dn = (char *)dn; @@ -705,7 +704,7 @@ (LDAP_NO_SUCH_ATTRIBUTE == result)) { if (curl) { /* compare completed; caching result */ - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); the_compare_node.lastcompare = curtime; the_compare_node.result = result; @@ -762,11 +761,8 @@ (util_ldap_state_t *)ap_get_module_config(r->server->module_config, &ldap_module); - /* read lock this function */ - LDAP_CACHE_LOCK_CREATE(st->pool); - /* Get the cache node for this url */ - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); curnode.url = url; curl = (util_url_node_t *)util_ald_cache_fetch(st->util_ldap_cache, &curnode); if (curl == NULL) { @@ -775,7 +771,7 @@ LDAP_CACHE_UNLOCK(); if (curl) { - LDAP_CACHE_RDLOCK(); + LDAP_CACHE_LOCK(); the_search_node.username = filter; search_nodep = util_ald_cache_fetch(curl->search_cache, &the_search_node); if (search_nodep != NULL && search_nodep->bindpw) { @@ -934,7 +930,7 @@ * Add the new username to the search cache. */ if (curl) { - LDAP_CACHE_WRLOCK(); + LDAP_CACHE_LOCK(); the_search_node.username = filter; the_search_node.dn = *binddn; the_search_node.bindpw = bindpw; @@ -1172,14 +1168,36 @@ int rc = LDAP_SUCCESS; apr_status_t result; char buf[MAX_STRING_LEN]; + 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); + void *data; + const char *userdata_key = "util_ldap_init"; + + /* util_ldap_post_config() will be called twice. Don't bother + * going through all of the initialization on the first call + * because it will just be thrown away.*/ + apr_pool_userdata_get(&data, userdata_key, s->process->pool); + if (!data) { + apr_pool_userdata_set((const void *)1, userdata_key, + apr_pool_cleanup_null, s->process->pool); + +#if APR_HAS_SHARED_MEMORY + /* If the cache file already exists then delete it. Otherwise we are + * going to run into problems creating the shared memory. */ + apr_file_remove(st->cache_file, ptemp); + if (st->cache_file) { + char *lck_file = apr_pstrcat (st->pool, st->cache_file, ".lck", NULL); + apr_file_remove(lck_file, ptemp); + } +#endif + return OK; + } + #if APR_HAS_SHARED_MEMORY - server_rec *s_vhost; - util_ldap_state_t *st_vhost; - /* initializing cache if shared memory size is not zero and we already don't have shm address */ if (!st->cache_shm && st->cache_bytes > 0) { #endif @@ -1190,20 +1208,37 @@ "LDAP cache: error while creating a shared memory segment: %s", buf); } + #if APR_HAS_SHARED_MEMORY + if (st->cache_file) { + st->lock_file = apr_pstrcat (st->pool, st->cache_file, ".lck", NULL); + } + else +#endif + st->lock_file = ap_server_root_relative(st->pool, tmpnam(NULL)); + + result = apr_global_mutex_create(&st->util_ldap_cache_lock, st->lock_file, APR_LOCK_DEFAULT, st->pool); + if (result != APR_SUCCESS) { + return result; + } + /* 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); + +#if APR_HAS_SHARED_MEMORY st_vhost->cache_shm = st->cache_shm; st_vhost->cache_rmm = st->cache_rmm; st_vhost->cache_file = st->cache_file; + 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); +#endif + st_vhost->lock_file = st->lock_file; s_vhost = s_vhost->next; } +#if APR_HAS_SHARED_MEMORY } else { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "LDAP cache: LDAPSharedCacheSize is zero, disabling shared memory cache"); @@ -1368,6 +1403,22 @@ return(OK); } +static void util_ldap_child_init(apr_pool_t *p, server_rec *s) +{ + apr_status_t sts; + util_ldap_state_t *st = + (util_ldap_state_t *)ap_get_module_config(s->module_config, &ldap_module); + + sts = apr_global_mutex_child_init(&st->util_ldap_cache_lock, st->lock_file, p); + if (sts != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_CRIT, sts, s, "failed to init caching lock in child process"); + return; + } + else { + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, s, + "INIT global mutex %s in child %d ", st->lock_file, getpid()); + } +} command_rec util_ldap_cmds[] = { AP_INIT_TAKE1("LDAPSharedCacheSize", util_ldap_set_cache_bytes, NULL, RSRC_CONF, @@ -1413,6 +1464,7 @@ { ap_hook_post_config(util_ldap_post_config,NULL,NULL,APR_HOOK_MIDDLE); ap_hook_handler(util_ldap_handler, NULL, NULL, APR_HOOK_MIDDLE); + ap_hook_child_init(util_ldap_child_init, NULL, NULL, APR_HOOK_MIDDLE); } module ldap_module = { 1.20 +5 -1 httpd-2.0/modules/experimental/util_ldap_cache.c Index: util_ldap_cache.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap_cache.c,v retrieving revision 1.19 retrieving revision 1.20 diff -u -r1.19 -r1.20 --- util_ldap_cache.c 21 Jun 2004 19:49:42 -0000 1.19 +++ util_ldap_cache.c 25 Jun 2004 19:56:35 -0000 1.20 @@ -376,7 +376,12 @@ { util_ldap_state_t *st = (util_ldap_state_t *)data; + util_ald_destroy_cache(st->util_ldap_cache); #if APR_HAS_SHARED_MEMORY + if (st->cache_rmm != NULL) { + apr_rmm_destroy (st->cache_rmm); + st->cache_rmm = NULL; + } if (st->cache_shm != NULL) { apr_status_t result = apr_shm_destroy(st->cache_shm); st->cache_shm = NULL; @@ -384,7 +389,6 @@ return result; } #endif - util_ald_destroy_cache(st->util_ldap_cache); return APR_SUCCESS; } 1.16 +0 -16 httpd-2.0/modules/experimental/util_ldap_cache.h Index: util_ldap_cache.h =================================================================== RCS file: /home/cvs/httpd-2.0/modules/experimental/util_ldap_cache.h,v retrieving revision 1.15 retrieving revision 1.16 diff -u -r1.15 -r1.16 --- util_ldap_cache.h 17 Jun 2004 17:19:01 -0000 1.15 +++ util_ldap_cache.h 25 Jun 2004 19:56:35 -0000 1.16 @@ -73,22 +73,6 @@ }; -#if APR_HAS_THREADS -#define LDAP_CACHE_LOCK_CREATE(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(st->util_ldap_cache_lock) -#define LDAP_CACHE_UNLOCK() \ - apr_thread_rwlock_unlock(st->util_ldap_cache_lock) -#define LDAP_CACHE_RDLOCK() \ - apr_thread_rwlock_rdlock(st->util_ldap_cache_lock) -#else -#define LDAP_CACHE_LOCK_CREATE(p) -#define LDAP_CACHE_WRLOCK() -#define LDAP_CACHE_UNLOCK() -#define LDAP_CACHE_RDLOCK() -#endif - #ifndef WIN32 #define ALD_MM_FILE_MODE ( S_IRUSR|S_IWUSR ) #else
