You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2008/02/11 21:02:53 UTC
Re: svn commit: r620489 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
On 02/11/2008 03:27 PM, dirkx@apache.org wrote:
> Author: dirkx
> Date: Mon Feb 11 06:27:34 2008
> New Revision: 620489
>
> URL: http://svn.apache.org/viewvc?rev=620489&view=rev
> Log:
> Return a little bit more error information when, say a disk is full or something gets write protected. Note that in some cases mod_cache.c will_also_ log a 'cache: store_headers failed' subsequently.
>
> Modified:
> httpd/httpd/trunk/modules/cache/mod_disk_cache.c
>
> 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=620489&r1=620488&r2=620489&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Mon Feb 11 06:27:34 2008
> @@ -846,13 +852,23 @@
> dobj->prefix = NULL;
> }
>
> - mkdir_structure(conf, dobj->hdrsfile, r->pool);
> + rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
> +
> + if (rv != APR_SUCCESS) {
> + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
> + "disk_cache: could not create directory path to %s",
> + dobj->hdrsfile);
> + return rv;
> + }
This non fatal here and may happen due to races with htcachelean. So I wouldn't
check the return value here and let it fail later in safe_file_rename if it
really doesn't work.
>
> rv = apr_file_mktemp(&dobj->tfd, dobj->tempfile,
> APR_CREATE | APR_WRITE | APR_BINARY | APR_EXCL,
> r->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);
> return rv;
> }
>
> @@ -877,7 +893,6 @@
> 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);
Why shouldn't we remove the tempfile in this case?
> return rv;
> }
>
> @@ -960,7 +987,13 @@
> */
> rv = apr_file_remove(dobj->hdrsfile, r->pool);
> if (rv != APR_SUCCESS) {
> - mkdir_structure(conf, dobj->hdrsfile, r->pool);
> + rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
> + if (rv != APR_SUCCESS) {
> + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
> + "disk_cache: creating directories for hdrsfile %s failed",
> + dobj->hdrsfile);
> + return rv;
> + }
This non fatal here and may happen due to races with htcachelean. So I wouldn't
check the return value here and let it fail later in safe_file_rename if it
really doesn't work.
> }
>
> rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile, r->pool);
>
Regards
Rüdiger
Re: svn commit: r620489 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
On Feb 11, 2008, at 9:02 PM, Ruediger Pluem wrote:
> This non fatal here and may happen due to races with htcachelean. So
> I wouldn't
> check the return value here and let it fail later in
> safe_file_rename if it
> really doesn't work.
Ok - I'll remove the checks for now and will give it a play -- I
introduced them as I found
them helpful exactly there when playing with intentional disk-full and
unmounting of
the cache disk* -- and finding the old behaviour very mistyfiing.
> Why shouldn't we remove the tempfile in this case?
Mistake - fixed !
Dw.
*: temp volume in memory/swapspace.