[EMAIL PROTECTED] wrote:
jerenkrantz 2004/09/21 15:56:23
Modified: . CHANGES
modules/experimental cache_storage.c cache_util.c
mod_cache.c mod_cache.h mod_disk_cache.c
mod_mem_cache.c
Log:
Fix Expires (freshness) handling in mod_cache.
Previously, if the cached copy was stale, the response would go into an
indeterminate state. Therefore, the freshness check must be done before we
'accept' the response and, if it fails (i.e. stale), we can't allow any side
effects.
This caused a number of changes to how mod_disk_cache reads its headers as
ap_scan_script_header_err() purposely has side-effects and that's
unacceptable. So, factor out only what we need.
Also, remove the broken conditional filter code as you can't reliably alter the
filter list once the response is started. (Regardless, cache_select_url()
has the freshness checks now.)
Assist to Sascha Schumann for reporting mod_cache was busted.
Revision Changes Path
1.1596 +5 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.1595
retrieving revision 1.1596
diff -u -u -r1.1595 -r1.1596
--- CHANGES 21 Sep 2004 13:23:47 -0000 1.1595
+++ CHANGES 21 Sep 2004 22:56:22 -0000 1.1596
@@ -2,6 +2,11 @@
[Remove entries to the current 2.0 section below, when backported]
+ *) Fix Expires handling in mod_cache. [Justin Erenkrantz]
+
+ *) Alter mod_expires to run at a different filter priority to allow
+ proper Expires storage by mod_cache. [Justin Erenkrantz]
+
*) Fix the global mutex crash when the global mutex is never allocated due
to disabled/empty caches. [Jess Holle <jessh ptc.com>]
1.37 +82 -19 httpd-2.0/modules/experimental/cache_storage.c
Index: cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.36
retrieving revision 1.37
diff -u -u -r1.36 -r1.37
--- cache_storage.c 5 Aug 2004 19:12:34 -0000 1.36
+++ cache_storage.c 21 Sep 2004 22:56:23 -0000 1.37
@@ -98,6 +98,57 @@
return DECLINED;
}
<snip>
/*
* select a specific URL entity in the cache
*
@@ -118,12 +169,12 @@
cache_request_rec *cache = (cache_request_rec *) ap_get_module_config(r->request_config, &cache_module);
- rv = cache_generate_key(r,r->pool,&key);
+ rv = cache_generate_key(r, r->pool, &key);
if (rv != APR_SUCCESS) {
return rv;
}
/* go through the cache types till we get a match */
- h = cache->handle = apr_palloc(r->pool, sizeof(cache_handle_t));
+ h = apr_palloc(r->pool, sizeof(cache_handle_t));
This little gem causes a regression. Because cache->handle is left NULL, we never cleanup stale entries in the cache (in cache_save_filter). Once CacheMaxExpires hits, the stale entry will never be garbage collected from the cache and replaced by a new entry. Two ways to fix this come to mind right off hand (neither are optimal i expect):
Patch 1:
Index: cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.38
diff -u -r1.38 cache_storage.c
--- cache_storage.c 22 Sep 2004 22:25:45 -0000 1.38
+++ cache_storage.c 23 Sep 2004 18:49:17 -0000
@@ -248,6 +248,7 @@
/* Is our cached response fresh enough? */
fresh = ap_cache_check_freshness(h, r);
if (!fresh) {
+ cache->provider->remove_entity(h);
return DECLINED;
}
Patch 2: Index: cache_storage.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v retrieving revision 1.38 diff -u -r1.38 cache_storage.c --- cache_storage.c 22 Sep 2004 22:25:45 -0000 1.38 +++ cache_storage.c 23 Sep 2004 18:51:10 -0000 @@ -248,6 +248,7 @@ /* Is our cached response fresh enough? */ fresh = ap_cache_check_freshness(h, r); if (!fresh) { + cache->provider->remove_entity(h); return DECLINED; }
If no one ventures an opinion by this evening, I'll dig into it and come up with a solution. No time at the moment.
Bill
