> -----Original Message----- > From: Graham Leggett > Sent: Donnerstag, 16. September 2010 11:54 > To: [email protected] > Subject: Re: 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 > > On 16 Sep 2010, at 9:13 AM, Ruediger Pluem wrote: > > >> +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? > > I don't follow, when you we fork during normal request handling? > > This function creates the structure that keeps track of each of the > three files we write, along with the tempfiles used to construct the > data. The cleanup guarantees that tempfiles are deleted should the > request end unexpectedly, and that we don't leave temp files lying > around as we previously did.
Hm. What happens in a threaded MPM, where we a doing a caching operation and another thread causes a fork (e.g. because of mod_cgi (I know should not be used with threaded MPM's), or some 3rd party module that does a fork). Wouldn't that cause our cache file to be deleted in the middle of the caching? My comment is only about the 4th parameter to apr_pool_cleanup_register set to file_cache_temp_cleanup, I am fine with setting the 3rd to file_cache_temp_cleanup and fully follow that purpose. Shouldn't we set the 4th paramter just to apr_pool_cleanup_null? Regards Rüdiger
