From: Graham Leggett Date: Mon, 4 Oct 2010 12:46:06 +0000 (+0000) Subject: Remove the attempt to pass the cache key into the lock functions, use X-Git-Tag: 2.3.9~361 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7e03f9e8c03c86f7c611595c8d4490b26fd37994;p=apache Remove the attempt to pass the cache key into the lock functions, use cache->key instead for this. Fixes a segfault caused when cache->key was populated, but the passed in key was not. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1004220 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/cache/cache_util.c b/modules/cache/cache_util.c index 576d8fd9f3..9982c20f63 100644 --- a/modules/cache/cache_util.c +++ b/modules/cache/cache_util.c @@ -230,8 +230,8 @@ CACHE_DECLARE(apr_int64_t) ap_cache_current_age(cache_info *info, * no point is it possible for this lock to permanently deny access to * the backend. */ -apr_status_t cache_try_lock(cache_server_conf *conf, - cache_request_rec *cache, request_rec *r, char *key) +apr_status_t cache_try_lock(cache_server_conf *conf, cache_request_rec *cache, + request_rec *r) { apr_status_t status; const char *lockname; @@ -256,12 +256,12 @@ apr_status_t cache_try_lock(cache_server_conf *conf, } /* create the key if it doesn't exist */ - if (!key) { + if (!cache->key) { cache_generate_key(r, r->pool, &cache->key); } /* create a hashed filename from the key, and save it for later */ - lockname = ap_cache_generate_name(r->pool, 0, 0, key); + lockname = ap_cache_generate_name(r->pool, 0, 0, cache->key); /* lock files represent discrete just-went-stale URLs "in flight", so * we support a simple two level directory structure, more is overkill. @@ -324,8 +324,7 @@ apr_status_t cache_try_lock(cache_server_conf *conf, * removed if the bucket brigade contains an EOS bucket. */ apr_status_t cache_remove_lock(cache_server_conf *conf, - cache_request_rec *cache, request_rec *r, char *key, - apr_bucket_brigade *bb) + cache_request_rec *cache, request_rec *r, apr_bucket_brigade *bb) { void *dummy; const char *lockname; @@ -363,12 +362,12 @@ apr_status_t cache_remove_lock(cache_server_conf *conf, char dir[5]; /* create the key if it doesn't exist */ - if (!key) { + if (!cache->key) { cache_generate_key(r, r->pool, &cache->key); } /* create a hashed filename from the key, and save it for later */ - lockname = ap_cache_generate_name(r->pool, 0, 0, key); + lockname = ap_cache_generate_name(r->pool, 0, 0, cache->key); /* lock files represent discrete just-went-stale URLs "in flight", so * we support a simple two level directory structure, more is overkill. @@ -692,7 +691,7 @@ int cache_check_freshness(cache_handle_t *h, * A lock that exceeds a maximum age will be deleted, and another * request gets to make a new lock and try again. */ - status = cache_try_lock(conf, cache, r, (char *)h->cache_obj->key); + status = cache_try_lock(conf, cache, r); if (APR_SUCCESS == status) { /* we obtained a lock, follow the stale path */ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, diff --git a/modules/cache/cache_util.h b/modules/cache/cache_util.h index ffcf8e708f..831ae4b902 100644 --- a/modules/cache/cache_util.h +++ b/modules/cache/cache_util.h @@ -248,8 +248,8 @@ int cache_check_freshness(cache_handle_t *h, cache_request_rec *cache, * no point is it possible for this lock to permanently deny access to * the backend. */ -apr_status_t cache_try_lock(cache_server_conf *conf, - cache_request_rec *cache, request_rec *r, char *key); +apr_status_t cache_try_lock(cache_server_conf *conf, cache_request_rec *cache, + request_rec *r); /** * Remove the cache lock, if present. @@ -264,8 +264,7 @@ apr_status_t cache_try_lock(cache_server_conf *conf, * removed if the bucket brigade contains an EOS bucket. */ apr_status_t cache_remove_lock(cache_server_conf *conf, - cache_request_rec *cache, request_rec *r, char *key, - apr_bucket_brigade *bb); + cache_request_rec *cache, request_rec *r, apr_bucket_brigade *bb); cache_provider_list *cache_get_providers(request_rec *r, cache_server_conf *conf, apr_uri_t uri); diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c index a9475d27bb..708a6cf0fe 100644 --- a/modules/cache/mod_cache.c +++ b/modules/cache/mod_cache.c @@ -138,7 +138,7 @@ static int cache_quick_handler(request_rec *r, int lookup) * backend without any attempt to cache. this stops * duplicated simultaneous attempts to cache an entity. */ - rv = cache_try_lock(conf, cache, r, NULL); + rv = cache_try_lock(conf, cache, r); if (APR_SUCCESS == rv) { /* @@ -384,7 +384,7 @@ static int cache_handler(request_rec *r) * backend without any attempt to cache. this stops * duplicated simultaneous attempts to cache an entity. */ - rv = cache_try_lock(conf, cache, r, NULL); + rv = cache_try_lock(conf, cache, r); if (APR_SUCCESS == rv) { /* @@ -599,8 +599,7 @@ static int cache_save_store(ap_filter_t *f, apr_bucket_brigade *in, ap_remove_output_filter(f); /* give someone else the chance to cache the file */ - cache_remove_lock(conf, cache, f->r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, f->r, NULL); /* give up trying to cache, just step out the way */ APR_BRIGADE_PREPEND(in, cache->out); @@ -620,8 +619,7 @@ static int cache_save_store(ap_filter_t *f, apr_bucket_brigade *in, } /* conditionally remove the lock as soon as we see the eos bucket */ - cache_remove_lock(conf, cache, f->r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, cache->out); + cache_remove_lock(conf, cache, f->r, cache->out); if (APR_BRIGADE_EMPTY(cache->out)) { if (APR_BRIGADE_EMPTY(in)) { @@ -641,8 +639,7 @@ static int cache_save_store(ap_filter_t *f, apr_bucket_brigade *in, ap_remove_output_filter(f); /* give someone else the chance to cache the file */ - cache_remove_lock(conf, cache, f->r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, f->r, NULL); return ap_pass_brigade(f->next, in); } @@ -949,8 +946,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) reason)); /* let someone else attempt to cache */ - cache_remove_lock(conf, cache, r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, r, NULL); return ap_pass_brigade(f->next, bb); } @@ -967,8 +963,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) ap_remove_output_filter(f); /* remove the lock file unconditionally */ - cache_remove_lock(conf, cache, r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, r, NULL); /* ship the data up the stack */ return ap_pass_brigade(f->next, in); @@ -1071,8 +1066,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) /* Caching layer declined the opportunity to cache the response */ ap_remove_output_filter(f); - cache_remove_lock(conf, cache, r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, r, NULL); return ap_pass_brigade(f->next, in); } @@ -1285,8 +1279,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) } /* let someone else attempt to cache */ - cache_remove_lock(conf, cache, r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, r, NULL); return ap_pass_brigade(f->next, bb); } @@ -1300,8 +1293,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) "cache miss: store_headers failed"); ap_remove_output_filter(f); - cache_remove_lock(conf, cache, r, cache->handle ? - (char *)cache->handle->cache_obj->key : NULL, NULL); + cache_remove_lock(conf, cache, r, NULL); return ap_pass_brigade(f->next, in); }