Colm MacCarthaigh wrote:
> I finally developed some time to look into this. mod_cache doesn't
> behave very nicely when the cache area fills. Of course administators
> should make sure it doesn't fill in the first place, but nevertheless a
> few people have hit this bug (me included) and I think mod_cache should
> handle the problem gracefully.
>
> Anyway, the problem occurs when the cache is unwritable, and mod_cache
> needs to revalidate a cached entity. cache_select_url handles this by
> rewriting headers_in to become a conditional request. However the code
> in cache_save_filter which turns the request back into its original
> (possibly unconditional) format is itself conditional on store_headers()
> working.
>
> The patch I've attached should be reasonably self-documenting, any
> questions - just ask.
>
As you already mentioned the remove_url implementation
of mod_disk_cache is currently an empty dummy :-).
I had a similar problem with 404 responses, and wrote a patch for this which is
currently in discussion (attached patch again to this mail):
http://mail-archives.apache.org/mod_mbox/httpd-dev/200507.mbox/[EMAIL PROTECTED]
It actually does implement a removal of the files in mod_disk_cache and
should also handle your problem. If it does not, I am pretty sure that a small
modification
to the patch would do it.
I would really appreciate if you find some time to review my patch.
Thanks and regards
Rüdiger
Index: modules/cache/mod_mem_cache.c
===================================================================
--- modules/cache/mod_mem_cache.c (Revision 220022)
+++ modules/cache/mod_mem_cache.c (Arbeitskopie)
@@ -601,7 +601,7 @@
/* remove_url()
* Notes:
*/
-static int remove_url(const char *key)
+static int remove_url(cache_handle_t *h, apr_pool_t *p)
{
cache_object_t *obj;
int cleanup = 0;
@@ -609,8 +609,8 @@
if (sconf->lock) {
apr_thread_mutex_lock(sconf->lock);
}
-
- obj = cache_find(sconf->cache_cache, key);
+
+ obj = h->cache_obj;
if (obj) {
cache_remove(sconf->cache_cache, obj);
/* For performance, cleanup cache object after releasing the lock */
Index: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c (Revision 220022)
+++ modules/cache/mod_cache.c (Arbeitskopie)
@@ -29,6 +29,7 @@
*/
static ap_filter_rec_t *cache_save_filter_handle;
static ap_filter_rec_t *cache_out_filter_handle;
+static ap_filter_rec_t *cache_remove_url_filter_handle;
/*
* CACHE handler
@@ -123,6 +124,22 @@
/* add cache_save filter to cache this request */
ap_add_output_filter_handle(cache_save_filter_handle, NULL, r,
r->connection);
+
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
+ "Adding CACHE_REMOVE_URL filter.");
+
+ /*
+ * add cache_remove_url filter to this request to remove the
+ * cache entry if it is needed. Store the filter in the cache
+ * request rec for easy removal if it turns out that we do not
+ * need it, because we are caching it. Also put the current
+ * cache request rec in the filter context, as the request that
+ * is available later during running the filter maybe
+ * different due to an internal redirect.
+ */
+ cache->cache_remove_url_filter =
+
ap_add_output_filter_handle(cache_remove_url_filter_handle, cache, r,
+ r->connection);
}
else if (cache->stale_headers) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
@@ -436,11 +453,6 @@
if (reason) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"cache: %s not cached. Reason: %s", url, reason);
- /* remove this object from the cache
- * BillS Asks.. Why do we need to make this call to remove_url?
- * leave it in for now..
- */
- cache_remove_url(r, url);
/* remove this filter from the chain */
ap_remove_output_filter(f);
@@ -542,6 +554,15 @@
"cache: Caching url: %s", url);
/*
+ * We are actually caching this response. So it does not
+ * make sense to remove this entry in cache_remove_url_filter
+ * So remove it.
+ */
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "cache: Removing CACHE_REMOVE_URL filter.");
+ ap_remove_output_filter(cache->cache_remove_url_filter);
+
+ /*
* We now want to update the cache file header information with
* the new date, last modified, expire and content length and write
* it away to our cache file. First, we determine these values from
@@ -709,6 +730,53 @@
return ap_pass_brigade(f->next, in);
}
+/*
+ * CACHE_REMOVE_URL filter
+ * ---------------
+ *
+ * This filter gets added in the quick handler every time the CACHE_SAVE
+ * filter gets inserted. Its purpose is to remove a possible cache
+ * entry from the cache.
+ *
+ * CACHE_REMOVAL_URL has to be a protocol filter to ensure that is
+ * run even if the response is a canned error message, which
+ * removes the content filters and thus the CACHE_SAVE filter from
+ * the chain.
+ *
+ * CACHE_REMOVAL_URL expects cache request rec within its context
+ * because the request this filter runs on can be different from
+ * the one whose cache entry should be removed, due to internal
+ * redirects.
+ */
+
+static int cache_remove_url_filter(ap_filter_t *f, apr_bucket_brigade *in)
+{
+ request_rec *r = f->r;
+ cache_request_rec *cache;
+
+ /* Setup cache_request_rec */
+ cache = (cache_request_rec *) f->ctx;
+
+ if (!cache) {
+ /* user likely configured CACHE_SAVE manually; they should really use
+ * mod_cache configuration to do that. So:
+ * 1. Remove ourselves
+ * 2. Do nothing and bail out
+ */
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "cache_remove_url_filter: No cache request rec defined");
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f->next, in);
+ }
+ /*
+ * Now remove this cache entry from the cache
+ */
+ cache_remove_url(cache, r->pool);
+ /* remove ourselves */
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f->next, in);
+}
+
/* -------------------------------------------------------------- */
/* Setup configurable data */
@@ -962,6 +1030,7 @@
return OK;
}
+
static const command_rec cache_cmds[] =
{
/* XXX
@@ -1028,6 +1097,16 @@
cache_out_filter,
NULL,
AP_FTYPE_CONTENT_SET+1);
+ /*
+ * CACHE_REMOVAL_URL has to be a protocol filter to ensure that is
+ * run even if the response is a canned error message, which
+ * removes the content filters.
+ */
+ cache_remove_url_filter_handle =
+ ap_register_output_filter("CACHE_REMOVE_URL",
+ cache_remove_url_filter,
+ NULL,
+ AP_FTYPE_PROTOCOL);
ap_hook_post_config(cache_post_config, NULL, NULL, APR_HOOK_REALLY_FIRST);
}
Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h (Revision 220022)
+++ modules/cache/mod_cache.h (Arbeitskopie)
@@ -197,7 +197,7 @@
const char *urlkey, apr_off_t len);
int (*open_entity) (cache_handle_t *h, request_rec *r,
const char *urlkey);
- int (*remove_url) (const char *urlkey);
+ int (*remove_url) (cache_handle_t *h, apr_pool_t *p);
} cache_provider;
/* A linked-list of authn providers. */
@@ -225,6 +225,7 @@
apr_time_t exp; /* expiration */
apr_time_t lastmod; /* last-modified time */
cache_info *info; /* current cache info */
+ ap_filter_t *cache_remove_url_filter; /* cache_remove_url_filter for this
request */
} cache_request_rec;
@@ -271,7 +272,7 @@
/**
* cache_storage.c
*/
-int cache_remove_url(request_rec *r, char *url);
+int cache_remove_url(cache_request_rec *cache, apr_pool_t *p);
int cache_create_entity(request_rec *r, char *url, apr_off_t size);
int cache_select_url(request_rec *r, char *url);
apr_status_t cache_generate_key_default( request_rec *r, apr_pool_t*p,
char**key );
Index: modules/cache/mod_disk_cache.c
===================================================================
--- modules/cache/mod_disk_cache.c (Revision 220022)
+++ modules/cache/mod_disk_cache.c (Arbeitskopie)
@@ -514,9 +514,55 @@
return OK;
}
-static int remove_url(const char *key)
+static int remove_url(cache_handle_t *h, apr_pool_t *p)
{
- /* XXX: Delete file from cache! */
+ apr_status_t rc;
+ int error = 0;
+ disk_cache_object_t *dobj;
+
+
+ /* Get disk cache object from cache handle */
+ dobj = (disk_cache_object_t *) h->cache_obj->vobj;
+
+ if (!dobj) {
+ return DECLINED;
+ }
+
+ /* Delete headers file */
+ if (dobj->hdrsfile) {
+ rc = apr_file_remove(dobj->hdrsfile, p);
+ if ((rc != APR_SUCCESS) && (rc != APR_ENOENT)) {
+ /*
+ * Will only result in an output if httpd is started with -e debug.
+ * For reason see log_error_core for the case s == NULL.
+ */
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rc, NULL,
+ "disk_cache: Failed to delete headers file %s from
cache.",
+ dobj->datafile);
+ error = 1;
+ }
+ }
+
+ /* Delete data file */
+ if (dobj->datafile) {
+ rc = apr_file_remove(dobj->datafile, p);
+ if ((rc != APR_SUCCESS) && (rc != APR_ENOENT)) {
+ /*
+ * Will only result in an output if httpd is started with -e debug.
+ * For reason see log_error_core for the case s == NULL.
+ */
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rc, NULL,
+ "disk_cache: Failed to delete data file %s from
cache.",
+ dobj->hdrsfile);
+ error = 1;
+ }
+ }
+
+ /* return DECLINED in case of any error except file not found */
+ if (error) {
+ return DECLINED;
+ }
+
return OK;
}
Index: modules/cache/cache_storage.c
===================================================================
--- modules/cache/cache_storage.c (Revision 220022)
+++ modules/cache/cache_storage.c (Arbeitskopie)
@@ -28,24 +28,30 @@
* delete all URL entities from the cache
*
*/
-int cache_remove_url(request_rec *r, char *url)
+int cache_remove_url(cache_request_rec *cache, apr_pool_t *p)
{
cache_provider_list *list;
apr_status_t rv;
char *key;
- cache_request_rec *cache = (cache_request_rec *)
- ap_get_module_config(r->request_config,
&cache_module);
+ cache_handle_t *h;
- rv = cache_generate_key(r,r->pool,&key);
- if (rv != APR_SUCCESS) {
- return rv;
- }
list = cache->providers;
+ /*
+ * Check if we have a cache handle. Check on the regular one first
+ * and if it's NULL we try our luck with the stale_handle.
+ * If this also fails bail out.
+ */
+ h = (cache_handle_t *) (cache->handle ?
+ cache->handle : cache->stale_handle);
+ if (!h) {
+ return OK;
+ }
+
/* for each specified cache type, delete the URL */
while(list) {
- list->provider->remove_url(key);
+ list->provider->remove_url(h, p);
list = list->next;
}
return OK;
