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" <mdounin@mdounin.ru> wrote:

>Hello!
>
>On Mon, Jul 07, 2014 at 05:19:45PM +0800, flygoast wrote:
>
>> # HG changeset patch
>> # User FengGu <flygoast@126.com>
>> # 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
>nginx-devel@nginx.org
>http://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel