Thanks for reply. When ngx_ext_rename_file() return NGX_ERROR, it should has called ngx_delete_file(). So I think maybe also should change "ext.delete_file = 1;" to "ext.delete_file = 0;".
At 2014-07-08 00:22:23,"Maxim Dounin" <[email protected]> wrote: >Hello! > >On Mon, Jul 07, 2014 at 05:19:45PM +0800, flygoast wrote: > >> # HG changeset patch >> # User FengGu <[email protected]> >> # Date 1404723967 -28800 >> # Mon Jul 07 17:06:07 2014 +0800 >> # Node ID d1d597fbf6d8e08059a308d8900e5f90def5377a >> # Parent a680bf4dddd5c4b106419e3dfb0264815c401275 >> Upstream: delete temp file when error in ngx_http_upstream_store(). >> >> >> Previously, didn't process the case when ngx_http_map_uri_to_path() returned >> NULL. And when error occured, didn't delete the temp file. >> >> >> diff -r a680bf4dddd5 -r d1d597fbf6d8 src/http/ngx_http_upstream.c >> --- a/src/http/ngx_http_upstream.cSat Jul 05 23:29:47 2014 +0400 >> +++ b/src/http/ngx_http_upstream.cMon Jul 07 17:06:07 2014 +0800 >> @@ -3375,14 +3375,16 @@ >> >> if (u->conf->store_lengths == NULL) { >> >> - ngx_http_map_uri_to_path(r, &path, &root, 0); >> + if (ngx_http_map_uri_to_path(r, &path, &root, 0) == NULL) { >> + goto error; >> + } >> >> } else { >> if (ngx_http_script_run(r, &path, u->conf->store_lengths->elts, 0, >> u->conf->store_values->elts) >> == NULL) >> { >> - return; >> + goto error; >> } >> } >> >> @@ -3393,6 +3395,16 @@ >> tf->file.name.data, path.data); >> >> (void) ngx_ext_rename_file(&tf->file.name, &path, &ext); >> + >> + return; >> + >> +error: >> + >> + if (ngx_delete_file(tf->file.name.data) == NGX_FILE_ERROR) { >> + ngx_log_error(NGX_LOG_CRIT, r->connection->log, ngx_errno, >> + ngx_delete_file_n " \"%s\" failed", >> + tf->file.name.data); >> + } >> } >> > >Thanks for noting this. What do you think about the following patch >instead? > >There is a code in ngx_http_upstream_finalize_request() which >normally removes temporary files after errors with proxy_store, >but in this particular case it won't be triggered due to u->store >set to 0 even in case of errors. With this patch, u->store is set >to 0 only in if there are no errors, and thus temporary files are >properly removed in case of errors in ngx_http_upstream_store(). > >diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c >--- a/src/http/ngx_http_upstream.c >+++ b/src/http/ngx_http_upstream.c >@@ -3253,7 +3253,6 @@ ngx_http_upstream_process_request(ngx_ht > || u->headers_in.content_length_n == tf->offset)) > { > ngx_http_upstream_store(r, u); >- u->store = 0; > } > } > } >@@ -3375,7 +3374,9 @@ ngx_http_upstream_store(ngx_http_request > > if (u->conf->store_lengths == NULL) { > >- ngx_http_map_uri_to_path(r, &path, &root, 0); >+ if (ngx_http_map_uri_to_path(r, &path, &root, 0) == NULL) { >+ return; >+ } > > } else { > if (ngx_http_script_run(r, &path, u->conf->store_lengths->elts, 0, >@@ -3392,7 +3393,11 @@ ngx_http_upstream_store(ngx_http_request > "upstream stores \"%s\" to \"%s\"", > tf->file.name.data, path.data); > >- (void) ngx_ext_rename_file(&tf->file.name, &path, &ext); >+ if (ngx_ext_rename_file(&tf->file.name, &path, &ext) != NGX_OK) { >+ return; >+ } >+ >+ u->store = 0; > } > > >-- >Maxim Dounin >http://nginx.org/ > >_______________________________________________ >nginx-devel mailing list >[email protected] >http://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
