From: Jim Jagielski Date: Fri, 18 Dec 2009 14:09:40 +0000 (+0000) Subject: Merge r822458 from trunk: X-Git-Tag: 2.2.15~132 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96c0a6d3bf8687ce78f9dbe7b7fb81fc33a2d124;p=thirdparty%2Fapache%2Fhttpd.git Merge r822458 from trunk: Work around broken cache management in mod_ldap: If LDAPSharedCacheSize is too small, try to free some memory by purging the cache and log a warning. Also increase the default LDAPSharedCacheSize to 500000. This is a more realistic size suitable for the default values of 1024 for LdapCacheEntries and LdapOpCacheEntries. PR: 46749 Submitted by: sf Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@892257 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 763b39d7d54..a71324f3c2d 100644 --- a/CHANGES +++ b/CHANGES @@ -15,6 +15,12 @@ Changes with Apache 2.2.15 *) mod_charset_lite: Honor 'CharsetOptions NoImplicitAdd'. [Eric Covener] + *) mod_ldap: If LDAPSharedCacheSize is too small, try harder to purge + some cache entries and log a warning. Also increase the default + LDAPSharedCacheSize to 500000. This is a more realistic size suitable + for the default values of 1024 for LdapCacheEntries/LdapOpCacheEntries. + PR 46749. [Stefan Fritsch] + *) mod_rewrite: Make sure that a hostname:port isn't fully qualified if the request is a CONNECT request. PR 47928 [Bill Zajac ] diff --git a/STATUS b/STATUS index 73a7ebb787e..22dd1e71ced 100644 --- a/STATUS +++ b/STATUS @@ -87,15 +87,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_ldap: Work around broken cache management in mod_ldap: - If LDAPSharedCacheSize is too small, try to free some memory by purging - the cache and log a warning. - Also increase the default LDAPSharedCacheSize to 500000. - PR 46749. - Trunk Patch: http://svn.apache.org/viewvc?rev=822458&view=rev - 2.2.x Patch: trunk patch works with some offsets - +1: sf, rpluem, rjung, jim - * mod_mime: Make RemoveType override the info from TypesConfig PR 38330. Trunk Patch: http://svn.apache.org/viewvc?rev=821298&view=rev diff --git a/docs/manual/mod/mod_ldap.xml b/docs/manual/mod/mod_ldap.xml index e7764d991ea..02655c6b5d9 100644 --- a/docs/manual/mod/mod_ldap.xml +++ b/docs/manual/mod/mod_ldap.xml @@ -67,7 +67,7 @@ by other LDAP modules # be loaded. Change the "yourdomain.example.com" to
# match your domain.

- LDAPSharedCacheSize 200000
+ LDAPSharedCacheSize 500000
LDAPCacheEntries 1024
LDAPCacheTTL 600
LDAPOpCacheEntries 1024
@@ -385,12 +385,12 @@ by other LDAP modules LDAPSharedCacheSize Size in bytes of the shared-memory cache LDAPSharedCacheSize bytes -LDAPSharedCacheSize 102400 +LDAPSharedCacheSize 500000 server config

Specifies the number of bytes to allocate for the shared - memory cache. The default is 100kb. If set to 0, shared memory + memory cache. The default is 500kb. If set to 0, shared memory caching will not be used.

 diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c index 5b432696f9e..a894b6ea0a9 100644 --- a/modules/ldap/util_ldap.c +++ b/modules/ldap/util_ldap.c @@ -1820,7 +1820,7 @@ static void *util_ldap_create_config(apr_pool_t *p, server_rec *s) apr_thread_mutex_create(&st->mutex, APR_THREAD_MUTEX_DEFAULT, st->pool); #endif - st->cache_bytes = 100000; + st->cache_bytes = 500000; st->search_cache_ttl = 600000000; st->search_cache_size = 1024; st->compare_cache_ttl = 600000000; diff --git a/modules/ldap/util_ldap_cache_mgr.c b/modules/ldap/util_ldap_cache_mgr.c index 56b8cbecece..6bb066ab357 100644 --- a/modules/ldap/util_ldap_cache_mgr.c +++ b/modules/ldap/util_ldap_cache_mgr.c @@ -385,6 +385,7 @@ void *util_ald_cache_fetch(util_ald_cache_t *cache, void *payload) void *util_ald_cache_insert(util_ald_cache_t *cache, void *payload) { unsigned long hashval; + void *tmp_payload; util_cache_node_t *node; /* sanity check */ @@ -397,21 +398,68 @@ void *util_ald_cache_insert(util_ald_cache_t *cache, void *payload) util_ald_cache_purge(cache); if (cache->numentries >= cache->maxentries) { /* if the purge was not effective, we leave now to avoid an overflow */ + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, + "Purge of LDAP cache failed"); return NULL; } } - /* should be safe to add an entry */ - if ((node = (util_cache_node_t *)util_ald_alloc(cache, sizeof(util_cache_node_t))) == NULL) { - return NULL; + node = (util_cache_node_t *)util_ald_alloc(cache, + sizeof(util_cache_node_t)); + if (node == NULL) { + /* + * XXX: The cache management should be rewritten to work + * properly when LDAPSharedCacheSize is too small. + */ + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, + "LDAPSharedCacheSize is too small. Increase it or " + "reduce LDAPCacheEntries/LDAPOpCacheEntries!"); + if (cache->numentries < cache->fullmark) { + /* + * We have not even reached fullmark, trigger a complete purge. + * This is still better than not being able to add new entries + * at all. + */ + cache->marktime = apr_time_now(); + } + util_ald_cache_purge(cache); + node = (util_cache_node_t *)util_ald_alloc(cache, + sizeof(util_cache_node_t)); + if (node == NULL) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, + "Could not allocate memory for LDAP cache entry"); + return NULL; + } } /* Take a copy of the payload before proceeeding. */ - payload = (*cache->copy)(cache, payload); - if (!payload) { - util_ald_free(cache, node); - return NULL; + tmp_payload = (*cache->copy)(cache, payload); + if (tmp_payload == NULL) { + /* + * XXX: The cache management should be rewritten to work + * properly when LDAPSharedCacheSize is too small. + */ + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, + "LDAPSharedCacheSize is too small. Increase it or " + "reduce LDAPCacheEntries/LDAPOpCacheEntries!"); + if (cache->numentries < cache->fullmark) { + /* + * We have not even reached fullmark, trigger a complete purge. + * This is still better than not being able to add new entries + * at all. + */ + cache->marktime = apr_time_now(); + } + util_ald_cache_purge(cache); + tmp_payload = (*cache->copy)(cache, payload); + if (tmp_payload == NULL) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, + "Could not allocate memory for LDAP cache value"); + util_ald_free(cache, node); + return NULL; + } } + payload = tmp_payload; /* populate the entry */ cache->inserts++;