On 09/16/2010 02:05 AM, [email protected] wrote: > 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/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); Is this correct? Do we want to call file_cache_temp_cleanup when we get forked? > + > + 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 Regards Rüdiger
