httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From minf...@apache.org
Subject svn commit: r997545 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h modules/cache/mod_cache.c modules/cache/mod_cache.h modules/cache/mod_disk_cache.c modules/cache/mod_disk_cache.h
Date Thu, 16 Sep 2010 00:05:15 GMT
Author: minfrin
Date: Thu Sep 16 00:05:14 2010
New Revision: 997545

URL: http://svn.apache.org/viewvc?rev=997545&view=rev
Log:
mod_cache: Add a discrete commit_entity() provider function within the
mod_cache provider interface which is called to indicate to the
provider that caching is complete, giving the provider the opportunity
to commit temporary files permanently to the cache in an atomic
fashion. Move all "rename" functionality of temporary files to permanent
files within mod_disk_cache from ad hoc locations in the code to the
commit_entity() function. Instead of reusing the same variables for
temporary file handling in mod_disk_cache, introduce separate discrete
structures for each of the three cache file types, the headers file,
vary file and data file, so that the atomic rename of all three file
types within commit_entity() becomes possible. Replace the inconsistent
use of error cleanups with a formal set of pool cleanups attached to
a subpool, which is destroyed on error.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/include/ap_mmn.h
    httpd/httpd/trunk/modules/cache/mod_cache.c
    httpd/httpd/trunk/modules/cache/mod_cache.h
    httpd/httpd/trunk/modules/cache/mod_disk_cache.c
    httpd/httpd/trunk/modules/cache/mod_disk_cache.h

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Sep 16 00:05:14 2010
@@ -2,6 +2,20 @@
 
 Changes with Apache 2.3.9
 
+  *) mod_cache: Add a discrete commit_entity() provider function within the
+     mod_cache provider interface which is called to indicate to the
+     provider that caching is complete, giving the provider the opportunity
+     to commit temporary files permanently to the cache in an atomic
+     fashion. Move all "rename" functionality of temporary files to permanent
+     files within mod_disk_cache from ad hoc locations in the code to the
+     commit_entity() function. Instead of reusing the same variables for
+     temporary file handling in mod_disk_cache, introduce separate discrete
+     structures for each of the three cache file types, the headers file,
+     vary file and data file, so that the atomic rename of all three file
+     types within commit_entity() becomes possible. Replace the inconsistent
+     use of error cleanups with a formal set of pool cleanups attached to
+     a subpool, which is destroyed on error. [Graham Leggett]
+
   *) mod_cache: Change the signature of the store_body() provider function
      within the mod_cache provider interface to support an "in" brigade
      and an "out" brigade instead of just a single input brigade. This

Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Thu Sep 16 00:05:14 2010
@@ -251,12 +251,14 @@
  * 20100905.1 (2.3.9-dev)  Add ap_cache_check_allowed()
  * 20100912.0 (2.3.9-dev)  Add an additional "out" brigade parameter to the
  *                         mod_cache store_body() provider function.
+ * 20100916.0 (2.3.9-dev)  Add commit_entity() to the mod_cache provider
+ *                         interface.
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20100912
+#define MODULE_MAGIC_NUMBER_MAJOR 20100916
 #endif
 #define MODULE_MAGIC_NUMBER_MINOR 0                     /* 0...n */
 

Modified: httpd/httpd/trunk/modules/cache/mod_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.c?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.c Thu Sep 16 00:05:14 2010
@@ -571,12 +571,14 @@ static int cache_out_filter(ap_filter_t 
  * case where the provider can't swallow the full brigade. In this
  * case, we write the brigade we were passed out downstream, and
  * loop around to try and cache some more until the in brigade is
- * completely empty.
+ * completely empty. As soon as the out brigade contains eos, call
+ * commit_entity() to finalise the cached element.
  */
 static int cache_save_store(ap_filter_t *f, apr_bucket_brigade *in,
         cache_server_conf *conf, cache_request_rec *cache)
 {
     int rv = APR_SUCCESS;
+    apr_bucket *e;
 
     /* pass the brigade in into the cache provider, which is then
      * expected to move cached buckets to the out brigade, for us
@@ -601,6 +603,17 @@ static int cache_save_store(ap_filter_t 
 
         }
 
+        /* does the out brigade contain eos? if so, we're done, commit! */
+        for (e = APR_BRIGADE_FIRST(cache->out);
+             e != APR_BRIGADE_SENTINEL(cache->out);
+             e = APR_BUCKET_NEXT(e))
+        {
+            if (APR_BUCKET_IS_EOS(e)) {
+                rv = cache->provider->commit_entity(cache->handle, f->r);
+                break;
+            }
+        }
+
         /* conditionally remove the lock as soon as we see the eos bucket */
         ap_cache_remove_lock(conf, f->r, cache->handle ?
                 (char *)cache->handle->cache_obj->key : NULL, cache->out);
@@ -1154,6 +1167,13 @@ static int cache_save_filter(ap_filter_t
         apr_bucket *bkt;
         int status;
 
+        /* We're just saving response headers, so we are done. Commit
+         * the response at this point, unless there was a previous error.
+         */
+        if (rv == APR_SUCCESS) {
+            rv = cache->provider->commit_entity(cache->handle, r);
+        }
+
         bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
 
         /* Restore the original request headers and see if we need to

Modified: httpd/httpd/trunk/modules/cache/mod_cache.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.h?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.h (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.h Thu Sep 16 00:05:14 2010
@@ -240,6 +240,7 @@ typedef struct {
     int (*open_entity) (cache_handle_t *h, request_rec *r,
                            const char *urlkey);
     int (*remove_url) (cache_handle_t *h, apr_pool_t *p);
+    apr_status_t (*commit_entity)(cache_handle_t *h, request_rec *r);
 } cache_provider;
 
 /* A linked-list of authn providers. */

Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Thu Sep 16 00:05:14 2010
@@ -154,49 +154,57 @@ static apr_status_t safe_file_rename(dis
     return rv;
 }
 
-static apr_status_t file_cache_el_final(disk_cache_object_t *dobj,
+static apr_status_t file_cache_el_final(disk_cache_conf *conf, disk_cache_file_t *file,
                                         request_rec *r)
 {
-    /* move the data over */
-    if (dobj->tfd) {
-        apr_status_t rv;
-
-        apr_file_close(dobj->tfd);
-
-        /* This assumes that the tempfile is on the same file system
-         * as the cache_root. If not, then we need a file copy/move
-         * rather than a rename.
-         */
-        rv = apr_file_rename(dobj->tempfile, dobj->datafile, r->pool);
+    apr_status_t rv;
+
+    /* This assumes that the tempfiles are on the same file system
+     * as the cache_root. If not, then we need a file copy/move
+     * rather than a rename.
+     */
+
+    /* move the file over */
+    if (file->tempfd) {
+
+        rv = safe_file_rename(conf, file->tempfile, file->file, file->pool);
         if (rv != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
-                         "disk_cache: rename tempfile to datafile failed:"
-                         " %s -> %s", dobj->tempfile, dobj->datafile);
-            apr_file_remove(dobj->tempfile, r->pool);
+                         "disk_cache: rename tempfile to file failed:"
+                         " %s -> %s", file->tempfile, file->file);
+            apr_file_remove(file->tempfile, file->pool);
         }
 
-        dobj->tfd = NULL;
+        file->tempfd = NULL;
     }
 
-    return APR_SUCCESS;
+    return rv;
 }
 
-static apr_status_t file_cache_errorcleanup(disk_cache_object_t *dobj, request_rec *r)
-{
-    /* Remove the header file and the body file. */
-    apr_file_remove(dobj->hdrsfile, r->pool);
-    apr_file_remove(dobj->datafile, r->pool);
-
-    /* If we opened the temporary data file, close and remove it. */
-    if (dobj->tfd) {
-        apr_file_close(dobj->tfd);
-        apr_file_remove(dobj->tempfile, r->pool);
-        dobj->tfd = NULL;
+static apr_status_t file_cache_temp_cleanup(void *dummy) {
+    disk_cache_file_t *file = (disk_cache_file_t *)dummy;
+
+    /* clean up the temporary file */
+    if (file->tempfd) {
+        apr_file_remove(file->tempfile, file->pool);
+        file->tempfd = NULL;
     }
+    file->tempfile = NULL;
+    file->pool = NULL;
 
     return APR_SUCCESS;
 }
 
+static apr_status_t file_cache_create(disk_cache_conf *conf, disk_cache_file_t *file,
+                                      apr_pool_t *pool)
+{
+    file->pool = pool;
+    file->tempfile = apr_pstrcat(pool, conf->cache_root, AP_TEMPFILE, NULL);
+
+    apr_pool_cleanup_register(pool, file, file_cache_temp_cleanup, file_cache_temp_cleanup);
+
+    return APR_SUCCESS;
+}
 
 /* These two functions get and put state information into the data
  * file for an ap_cache_el, this state information will be read
@@ -329,6 +337,7 @@ static int create_entity(cache_handle_t 
                                                  &disk_cache_module);
     cache_object_t *obj;
     disk_cache_object_t *dobj;
+    apr_pool_t *pool;
 
     if (conf->cache_root == NULL) {
         return DECLINED;
@@ -369,9 +378,16 @@ static int create_entity(cache_handle_t 
     /* Save the cache root */
     dobj->root = apr_pstrndup(r->pool, conf->cache_root, conf->cache_root_len);
     dobj->root_len = conf->cache_root_len;
-    dobj->datafile = data_file(r->pool, conf, dobj, key);
-    dobj->hdrsfile = header_file(r->pool, conf, dobj, key);
-    dobj->tempfile = apr_pstrcat(r->pool, conf->cache_root, AP_TEMPFILE, NULL);
+
+    apr_pool_create(&pool, r->pool);
+
+    file_cache_create(conf, &dobj->hdrs, pool);
+    file_cache_create(conf, &dobj->vary, pool);
+    file_cache_create(conf, &dobj->data, pool);
+
+    dobj->data.file = data_file(r->pool, conf, dobj, key);
+    dobj->hdrs.file = header_file(r->pool, conf, dobj, key);
+    dobj->vary.file = header_file(r->pool, conf, dobj, key);
 
     return OK;
 }
@@ -394,6 +410,7 @@ static int open_entity(cache_handle_t *h
     cache_info *info;
     disk_cache_object_t *dobj;
     int flags;
+    apr_pool_t *pool;
 
     h->cache_obj = NULL;
 
@@ -420,42 +437,43 @@ static int open_entity(cache_handle_t *h
     dobj->root = apr_pstrndup(r->pool, conf->cache_root, conf->cache_root_len);
     dobj->root_len = conf->cache_root_len;
 
-    dobj->hdrsfile = header_file(r->pool, conf, dobj, key);
+    dobj->vary.file = header_file(r->pool, conf, dobj, key);
     flags = APR_READ|APR_BINARY|APR_BUFFERED;
-    rc = apr_file_open(&dobj->hfd, dobj->hdrsfile, flags, 0, r->pool);
+    rc = apr_file_open(&dobj->vary.fd, dobj->vary.file, flags, 0, r->pool);
     if (rc != APR_SUCCESS) {
         return DECLINED;
     }
 
     /* read the format from the cache file */
     len = sizeof(format);
-    apr_file_read_full(dobj->hfd, &format, len, &len);
+    apr_file_read_full(dobj->vary.fd, &format, len, &len);
 
     if (format == VARY_FORMAT_VERSION) {
         apr_array_header_t* varray;
         apr_time_t expire;
 
         len = sizeof(expire);
-        apr_file_read_full(dobj->hfd, &expire, len, &len);
+        apr_file_read_full(dobj->vary.fd, &expire, len, &len);
 
         varray = apr_array_make(r->pool, 5, sizeof(char*));
-        rc = read_array(r, varray, dobj->hfd);
+        rc = read_array(r, varray, dobj->vary.fd);
         if (rc != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_ERR, rc, r->server,
                          "disk_cache: Cannot parse vary header file: %s",
-                         dobj->hdrsfile);
+                         dobj->vary.file);
+            apr_file_close(dobj->vary.fd);
             return DECLINED;
         }
-        apr_file_close(dobj->hfd);
+        apr_file_close(dobj->vary.fd);
 
         nkey = regen_key(r->pool, r->headers_in, varray, key);
 
         dobj->hashfile = NULL;
-        dobj->prefix = dobj->hdrsfile;
-        dobj->hdrsfile = header_file(r->pool, conf, dobj, nkey);
+        dobj->prefix = dobj->vary.file;
+        dobj->hdrs.file = header_file(r->pool, conf, dobj, nkey);
 
         flags = APR_READ|APR_BINARY|APR_BUFFERED;
-        rc = apr_file_open(&dobj->hfd, dobj->hdrsfile, flags, 0, r->pool);
+        rc = apr_file_open(&dobj->hdrs.fd, dobj->hdrs.file, flags, 0, r->pool);
         if (rc != APR_SUCCESS) {
             return DECLINED;
         }
@@ -463,56 +481,74 @@ static int open_entity(cache_handle_t *h
     else if (format != DISK_FORMAT_VERSION) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                      "disk_cache: File '%s' has a version mismatch. File had version: %d.",
-                     dobj->hdrsfile, format);
+                     dobj->vary.file, format);
+        apr_file_close(dobj->vary.fd);
         return DECLINED;
     }
     else {
         apr_off_t offset = 0;
+
+        /* oops, not vary as it turns out */
+        dobj->hdrs.fd = dobj->vary.fd;
+        dobj->vary.fd = NULL;
+        dobj->hdrs.file = dobj->vary.file;
+
         /* This wasn't a Vary Format file, so we must seek to the
          * start of the file again, so that later reads work.
          */
-        apr_file_seek(dobj->hfd, APR_SET, &offset);
+        apr_file_seek(dobj->hdrs.fd, APR_SET, &offset);
         nkey = key;
     }
 
     obj->key = nkey;
     dobj->key = nkey;
     dobj->name = key;
-    dobj->datafile = data_file(r->pool, conf, dobj, nkey);
-    dobj->tempfile = apr_pstrcat(r->pool, conf->cache_root, AP_TEMPFILE, NULL);
+
+    apr_pool_create(&pool, r->pool);
+
+    file_cache_create(conf, &dobj->hdrs, pool);
+    file_cache_create(conf, &dobj->vary, pool);
+    file_cache_create(conf, &dobj->data, pool);
+
+    dobj->data.file = data_file(r->pool, conf, dobj, nkey);
 
     /* Open the data file */
     flags = APR_READ|APR_BINARY;
 #ifdef APR_SENDFILE_ENABLED
     /* When we are in the quick handler we don't have the per-directory
-     * configuration, so this check only takes the globel setting of
+     * configuration, so this check only takes the global setting of
      * the EnableSendFile directive into account.
      */
     flags |= AP_SENDFILE_ENABLED(coreconf->enable_sendfile);
 #endif
-    rc = apr_file_open(&dobj->fd, dobj->datafile, flags, 0, r->pool);
+    rc = apr_file_open(&dobj->data.fd, dobj->data.file, flags, 0, r->pool);
     if (rc != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, rc, r->server,
-                 "disk_cache: Cannot open info header file %s",  dobj->datafile);
+                 "disk_cache: Cannot open data file %s",  dobj->data.file);
+        apr_file_close(dobj->hdrs.fd);
         return DECLINED;
     }
 
-    rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, dobj->fd);
+    rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, dobj->data.fd);
     if (rc == APR_SUCCESS) {
         dobj->file_size = finfo.size;
     }
 
     /* Read the bytes to setup the cache_info fields */
-    rc = file_cache_recall_mydata(dobj->hfd, info, dobj, r);
+    rc = file_cache_recall_mydata(dobj->hdrs.fd, info, dobj, r);
     if (rc != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, rc, r->server,
-                 "disk_cache: Cannot read header file %s",  dobj->hdrsfile);
+                 "disk_cache: Cannot read header file %s",  dobj->hdrs.file);
+        apr_file_close(dobj->hdrs.fd);
         return DECLINED;
     }
 
     /* Initialize the cache_handle callback functions */
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                  "disk_cache: Recalled cached URL info header %s",  dobj->name);
+
+    apr_file_close(dobj->hdrs.fd);
+
     return OK;
 }
 
@@ -535,35 +571,35 @@ static int remove_url(cache_handle_t *h,
     }
 
     /* Delete headers file */
-    if (dobj->hdrsfile) {
+    if (dobj->hdrs.file) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
-                     "disk_cache: Deleting %s from cache.", dobj->hdrsfile);
+                     "disk_cache: Deleting %s from cache.", dobj->hdrs.file);
 
-        rc = apr_file_remove(dobj->hdrsfile, p);
+        rc = apr_file_remove(dobj->hdrs.file, p);
         if ((rc != APR_SUCCESS) && !APR_STATUS_IS_ENOENT(rc)) {
             /* 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->hdrsfile);
+                         dobj->hdrs.file);
             return DECLINED;
         }
     }
 
-     /* Delete data file */
-    if (dobj->datafile) {
+    /* Delete data file */
+    if (dobj->data.file) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
-                     "disk_cache: Deleting %s from cache.", dobj->datafile);
+                     "disk_cache: Deleting %s from cache.", dobj->data.file);
 
-        rc = apr_file_remove(dobj->datafile, p);
+        rc = apr_file_remove(dobj->data.file, p);
         if ((rc != APR_SUCCESS) && !APR_STATUS_IS_ENOENT(rc)) {
             /* 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->datafile);
+                         dobj->data.file);
             return DECLINED;
         }
     }
@@ -572,7 +608,7 @@ static int remove_url(cache_handle_t *h,
     if (dobj->root) {
         const char *str_to_copy;
 
-        str_to_copy = dobj->hdrsfile ? dobj->hdrsfile : dobj->datafile;
+        str_to_copy = dobj->hdrs.file ? dobj->hdrs.file : dobj->data.file;
         if (str_to_copy) {
             char *dir, *slash, *q;
 
@@ -770,7 +806,7 @@ static apr_status_t recall_headers(cache
     disk_cache_object_t *dobj = (disk_cache_object_t *) h->cache_obj->vobj;
 
     /* This case should not happen... */
-    if (!dobj->hfd) {
+    if (!dobj->hdrs.fd) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                  "disk_cache: recalling headers; but no header fd for %s", dobj->name);
         return APR_NOTFOUND;
@@ -780,10 +816,10 @@ static apr_status_t recall_headers(cache
     h->resp_hdrs = apr_table_make(r->pool, 20);
 
     /* Call routine to read the header lines/status line */
-    read_table(h, r, h->resp_hdrs, dobj->hfd);
-    read_table(h, r, h->req_hdrs, dobj->hfd);
+    read_table(h, r, h->resp_hdrs, dobj->hdrs.fd);
+    read_table(h, r, h->req_hdrs, dobj->hdrs.fd);
 
-    apr_file_close(dobj->hfd);
+    apr_file_close(dobj->hdrs.fd);
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                  "disk_cache: Recalled headers for URL %s",  dobj->name);
@@ -795,7 +831,7 @@ static apr_status_t recall_body(cache_ha
     apr_bucket *e;
     disk_cache_object_t *dobj = (disk_cache_object_t*) h->cache_obj->vobj;
 
-    apr_brigade_insert_file(bb, dobj->fd, 0, dobj->file_size, p);
+    apr_brigade_insert_file(bb, dobj->data.fd, 0, dobj->file_size, p);
 
     e = apr_bucket_eos_create(bb->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(bb, e);
@@ -865,66 +901,53 @@ static apr_status_t store_headers(cache_
              * vary format hints in the appropriate directory.
              */
             if (dobj->prefix) {
-                dobj->hdrsfile = dobj->prefix;
+                dobj->hdrs.file = dobj->prefix;
                 dobj->prefix = NULL;
             }
 
-            rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
+            rv = mkdir_structure(conf, dobj->hdrs.file, r->pool);
 
-            rv = apr_file_mktemp(&dobj->tfd, dobj->tempfile,
+            rv = apr_file_mktemp(&dobj->vary.tempfd, dobj->vary.tempfile,
                                  APR_CREATE | APR_WRITE | APR_BINARY | APR_EXCL,
-                                 r->pool);
+                                 dobj->vary.pool);
 
             if (rv != APR_SUCCESS) {
                 ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                     "disk_cache: could not create temp file %s",
-                    dobj->tempfile);
+                    dobj->vary.tempfile);
                 return rv;
             }
 
             amt = sizeof(format);
-            apr_file_write(dobj->tfd, &format, &amt);
+            apr_file_write(dobj->vary.tempfd, &format, &amt);
 
             amt = sizeof(info->expire);
-            apr_file_write(dobj->tfd, &info->expire, &amt);
+            apr_file_write(dobj->vary.tempfd, &info->expire, &amt);
 
             varray = apr_array_make(r->pool, 6, sizeof(char*));
             tokens_to_array(r->pool, tmp, varray);
 
-            store_array(dobj->tfd, varray);
+            store_array(dobj->vary.tempfd, varray);
 
-            apr_file_close(dobj->tfd);
+            apr_file_close(dobj->vary.tempfd);
 
-            dobj->tfd = NULL;
-
-            rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile,
-                                  r->pool);
-            if (rv != APR_SUCCESS) {
-                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
-                    "disk_cache: rename tempfile to varyfile failed: %s -> %s",
-                    dobj->tempfile, dobj->hdrsfile);
-                    apr_file_remove(dobj->tempfile, r->pool);
-                return rv;
-            }
-
-            dobj->tempfile = apr_pstrcat(r->pool, conf->cache_root, AP_TEMPFILE,
NULL);
             tmp = regen_key(r->pool, r->headers_in, varray, dobj->name);
-            dobj->prefix = dobj->hdrsfile;
+            dobj->prefix = dobj->hdrs.file;
             dobj->hashfile = NULL;
-            dobj->datafile = data_file(r->pool, conf, dobj, tmp);
-            dobj->hdrsfile = header_file(r->pool, conf, dobj, tmp);
+            dobj->data.file = data_file(r->pool, conf, dobj, tmp);
+            dobj->hdrs.file = header_file(r->pool, conf, dobj, tmp);
         }
     }
 
 
-    rv = apr_file_mktemp(&dobj->hfd, dobj->tempfile,
+    rv = apr_file_mktemp(&dobj->hdrs.tempfd, dobj->hdrs.tempfile,
                          APR_CREATE | APR_WRITE | APR_BINARY |
-                         APR_BUFFERED | APR_EXCL, r->pool);
+                         APR_BUFFERED | APR_EXCL, dobj->hdrs.pool);
 
     if (rv != APR_SUCCESS) {
        ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
            "disk_cache: could not create temp file %s",
-           dobj->tempfile);
+           dobj->hdrs.tempfile);
         return rv;
     }
 
@@ -943,11 +966,12 @@ static apr_status_t store_headers(cache_
     iov[1].iov_base = (void*)dobj->name;
     iov[1].iov_len = disk_info.name_len;
 
-    rv = apr_file_writev(dobj->hfd, (const struct iovec *) &iov, 2, &amt);
+    rv = apr_file_writev(dobj->hdrs.tempfd, (const struct iovec *) &iov, 2, &amt);
     if (rv != APR_SUCCESS) {
-       ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+        ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
            "disk_cache: could not write info to header file %s",
-           dobj->hdrsfile);
+           dobj->hdrs.tempfile);
+        apr_file_close(dobj->hdrs.tempfd);
         return rv;
     }
 
@@ -956,11 +980,12 @@ static apr_status_t store_headers(cache_
 
         headers_out = ap_cache_cacheable_headers_out(r);
 
-        rv = store_table(dobj->hfd, headers_out);
+        rv = store_table(dobj->hdrs.tempfd, headers_out);
         if (rv != APR_SUCCESS) {
-           ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                "disk_cache: could not write out-headers to header file %s",
-               dobj->hdrsfile);
+               dobj->hdrs.tempfile);
+            apr_file_close(dobj->hdrs.tempfd);
             return rv;
         }
     }
@@ -972,39 +997,18 @@ static apr_status_t store_headers(cache_
 
         headers_in = ap_cache_cacheable_headers_in(r);
 
-        rv = store_table(dobj->hfd, headers_in);
+        rv = store_table(dobj->hdrs.tempfd, headers_in);
         if (rv != APR_SUCCESS) {
-           ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                "disk_cache: could not write in-headers to header file %s",
-               dobj->hdrsfile);
+               dobj->hdrs.tempfile);
+            apr_file_close(dobj->hdrs.tempfd);
             return rv;
         }
     }
 
-    apr_file_close(dobj->hfd); /* flush and close */
-
-    /* Remove old file with the same name. If remove fails, then
-     * perhaps we need to create the directory tree where we are
-     * about to write the new headers file.
-     */
-    rv = apr_file_remove(dobj->hdrsfile, r->pool);
-    if (rv != APR_SUCCESS) {
-        rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
-    }
-
-    rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile, r->pool);
-    if (rv != APR_SUCCESS) {
-        ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
-                     "disk_cache: rename tempfile to hdrsfile failed: %s -> %s",
-                     dobj->tempfile, dobj->hdrsfile);
-        apr_file_remove(dobj->tempfile, r->pool);
-        return rv;
-    }
-
-    dobj->tempfile = apr_pstrcat(r->pool, conf->cache_root, AP_TEMPFILE, NULL);
+    apr_file_close(dobj->hdrs.tempfd); /* flush and close */
 
-    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                 "disk_cache: Stored headers for URL %s",  dobj->name);
     return APR_SUCCESS;
 }
 
@@ -1022,10 +1026,10 @@ static apr_status_t store_body(cache_han
     /* We write to a temp file and then atomically rename the file over
      * in file_cache_el_final().
      */
-    if (!dobj->tfd) {
-        rv = apr_file_mktemp(&dobj->tfd, dobj->tempfile,
+    if (!dobj->data.tempfd) {
+        rv = apr_file_mktemp(&dobj->data.tempfd, dobj->data.tempfile,
                              APR_CREATE | APR_WRITE | APR_BINARY |
-                             APR_BUFFERED | APR_EXCL, r->pool);
+                             APR_BUFFERED | APR_EXCL, dobj->data.pool);
         if (rv != APR_SUCCESS) {
             return rv;
         }
@@ -1092,19 +1096,19 @@ static apr_status_t store_body(cache_han
                          "disk_cache: Error when reading bucket for URL %s",
                          h->cache_obj->key);
             /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            file_cache_errorcleanup(dobj, r);
+            apr_pool_destroy(dobj->data.pool);
             APR_BRIGADE_CONCAT(out, dobj->bb);
             return rv;
         }
 
         /* write to the cache, leave if we fail */
-        rv = apr_file_write_full(dobj->tfd, str, length, &written);
+        rv = apr_file_write_full(dobj->data.tempfd, str, length, &written);
         if (rv != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                          "disk_cache: Error when writing cache file for URL %s",
                          h->cache_obj->key);
             /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            file_cache_errorcleanup(dobj, r);
+            apr_pool_destroy(dobj->data.pool);
             APR_BRIGADE_CONCAT(out, dobj->bb);
             return rv;
         }
@@ -1115,7 +1119,7 @@ static apr_status_t store_body(cache_han
                          "(%" APR_OFF_T_FMT ">%" APR_OFF_T_FMT ")",
                          h->cache_obj->key, dobj->file_size, conf->maxfs);
             /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            file_cache_errorcleanup(dobj, r);
+            apr_pool_destroy(dobj->data.pool);
             APR_BRIGADE_CONCAT(out, dobj->bb);
             return APR_EGENERAL;
         }
@@ -1145,13 +1149,15 @@ static apr_status_t store_body(cache_han
     if (seen_eos) {
         const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
 
+        apr_file_close(dobj->data.tempfd);
+
         if (r->connection->aborted || r->no_cache) {
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
                          "disk_cache: Discarding body for URL %s "
                          "because connection has been aborted.",
                          h->cache_obj->key);
             /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            file_cache_errorcleanup(dobj, r);
+            apr_pool_destroy(dobj->data.pool);
             return APR_EGENERAL;
         }
         if (dobj->file_size < conf->minfs) {
@@ -1160,7 +1166,7 @@ static apr_status_t store_body(cache_han
                          "(%" APR_OFF_T_FMT "<%" APR_OFF_T_FMT ")",
                          h->cache_obj->key, dobj->file_size, conf->minfs);
             /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            file_cache_errorcleanup(dobj, r);
+            apr_pool_destroy(dobj->data.pool);
             return APR_EGENERAL;
         }
         if (cl_header) {
@@ -1170,17 +1176,47 @@ static apr_status_t store_body(cache_han
                              "disk_cache: URL %s didn't receive complete response, not caching",
                              h->cache_obj->key);
                 /* Remove the intermediate cache file and return non-APR_SUCCESS */
-                file_cache_errorcleanup(dobj, r);
+                apr_pool_destroy(dobj->data.pool);
                 return APR_EGENERAL;
             }
         }
 
-        /* All checks were fine. Move tempfile to final destination */
-        /* Link to the perm file, and close the descriptor */
-        file_cache_el_final(dobj, r);
+        /* All checks were fine, we're good to go when the commit comes */
+    }
+
+    return APR_SUCCESS;
+}
+
+static apr_status_t commit_entity(cache_handle_t *h, request_rec *r)
+{
+    disk_cache_conf *conf = ap_get_module_config(r->server->module_config,
+                                                 &disk_cache_module);
+    disk_cache_object_t *dobj = (disk_cache_object_t *) h->cache_obj->vobj;
+    apr_status_t rv;
+
+    /* move header and data tempfiles to the final destination */
+    rv = file_cache_el_final(conf, &dobj->hdrs, r);
+    if (APR_SUCCESS == rv) {
+        rv = file_cache_el_final(conf, &dobj->vary, r);
+    }
+    if (APR_SUCCESS == rv) {
+        rv = file_cache_el_final(conf, &dobj->data, r);
+    }
+
+    /* remove the cached items completely on any failure */
+    if (APR_SUCCESS != rv) {
+        remove_url(h, r->pool);
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
-                     "disk_cache: Body for URL %s cached.",  dobj->name);
+                     "disk_cache: commit_entity: URL '%s' not cached due to earlier disk
error.",
+                     dobj->name);
     }
+    else {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                     "disk_cache: commit_entity: Headers and body for URL %s cached.",
+                     dobj->name);
+    }
+
+    apr_pool_destroy(dobj->data.pool);
 
     return APR_SUCCESS;
 }
@@ -1359,6 +1395,7 @@ static const cache_provider cache_disk_p
     &create_entity,
     &open_entity,
     &remove_url,
+    &commit_entity
 };
 
 static void disk_cache_register_hook(apr_pool_t *p)

Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.h?rev=997545&r1=997544&r2=997545&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.h (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.h Thu Sep 16 00:05:14 2010
@@ -51,24 +51,29 @@ typedef struct {
     apr_time_t response_time;
 } disk_cache_info_t;
 
+typedef struct {
+    apr_pool_t *pool;
+    const char *file;
+    apr_file_t *fd;
+    char *tempfile;
+    apr_file_t *tempfd;
+} disk_cache_file_t;
+
 /*
  * disk_cache_object_t
  * Pointed to by cache_object_t::vobj
  */
 typedef struct disk_cache_object {
-    const char *root;        /* the location of the cache directory */
+    const char *root;            /* the location of the cache directory */
     apr_size_t root_len;
-    char *tempfile;    /* temp file tohold the content */
     const char *prefix;
-    const char *datafile;    /* name of file where the data will go */
-    const char *hdrsfile;    /* name of file where the hdrs will go */
-    const char *hashfile;    /* Computed hash key for this URI */
-    const char *name;   /* Requested URI without vary bits - suitable for mortals. */
-    const char *key;    /* On-disk prefix; URI with Vary bits (if present) */
-    apr_file_t *fd;          /* data file */
-    apr_file_t *hfd;         /* headers file */
-    apr_file_t *tfd;         /* temporary file for data */
-    apr_off_t file_size;     /*  File size of the cached data file  */
+    disk_cache_file_t data;      /* data file structure */
+    disk_cache_file_t hdrs;      /* headers file structure */
+    disk_cache_file_t vary;      /* vary file structure */
+    const char *hashfile;        /* Computed hash key for this URI */
+    const char *name;            /* Requested URI without vary bits - suitable for mortals.
*/
+    const char *key;             /* On-disk prefix; URI with Vary bits (if present) */
+    apr_off_t file_size;         /*  File size of the cached data file  */
     disk_cache_info_t disk_info; /* Header information. */
     apr_bucket_brigade *bb;      /* Set aside brigade */
     apr_off_t offset;            /* Max size to set aside */



Mime
View raw message