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.
--
Colm MacCárthaigh Public Key: [EMAIL PROTECTED]
Index: mod_cache.c
===================================================================
--- mod_cache.c (revision 230608)
+++ mod_cache.c (working copy)
@@ -666,7 +666,13 @@
ap_cache_accept_headers(cache->handle, r, 1);
}
- /* Write away header information to cache. */
+ /* Write away header information to cache. It is possible that we are
+ * trying to update headers for an entity which has already been cached.
+ *
+ * This may fail, due to an unwritable cache area. E.g. filesystem full,
+ * permissions problems or a read-only (re)mount. This must be handled
+ * later.
+ */
rv = cache->provider->store_headers(cache->handle, r, info);
/* Did we just update the cached headers on a revalidated response?
@@ -675,7 +681,7 @@
* the same way as with a regular response, but conditions are now checked
* against the cached or merged response headers.
*/
- if (rv == APR_SUCCESS && cache->stale_handle) {
+ if (cache->stale_handle) {
apr_bucket_brigade *bb;
apr_bucket *bkt;
int status;
@@ -699,12 +705,42 @@
}
cache->block_response = 1;
+
+ /* Before returning we need to handle the possible case of an
+ * unwritable cache. Rather than leaving the entity in the cache
+ * and having it constantly re-validated, now that we have recalled
+ * the body it is safe to try and remove the url from the cache.
+ */
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+ "cache: updating headers with store_headers failed. "
+ "Removing cached url.");
+
+ if (cache->provider->remove_url(url) != OK) {
+ /* Probably a mod_disk_cache cache area has been (re)mounted
+ * read-only, or that there is a permissions problem.
+ *
+ * XXX: right now mod_disk_cache's remove_url doesn't do
+ * anything and always returns OK. Once it does, this codepath
+ * will make more sense.
+ */
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+ "cache: attempt to remove url from cache unsuccessful.");
+ }
+ }
+
return ap_pass_brigade(f->next, bb);
}
+
+ if(rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+ "cache: store_headers failed");
+ ap_remove_output_filter(f);
- if (rv == APR_SUCCESS) {
- rv = cache->provider->store_body(cache->handle, r, in);
+ return ap_pass_brigade(f->next, in);
}
+
+ rv = cache->provider->store_body(cache->handle, r, in);
if (rv != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
"cache: store_body failed");
