[PATCH] ngx_upstream do not cleanup file buffers in subrequest

[Maxim Dounin]

Hello!

On Tue, Mar 08, 2011 at 03:24:47PM -0500, magz wrote:

> Hello,
> 
> Is it reasonable to add following check to not clean/discard fd in case
> if it's sub request?
> 
> I developed nginx module and when it started to use tmp buffers under
> load I had following error:
> 
> [alert] 29663#0: *45789 sendfile() failed (9: Bad file descriptor) while
> sending request to upstream, client: 10.1.24.14, server: hss, request:
> "POST 
> 
> So that sub-request cleared file before main request reached it. 
> Probably this bug should not appear if  request body was discarded for
> sub request. But  in case if you need request body for sub-request you
> should be sure that sub-request does not erases main request buffers.

In official nginx, subrequests can't appear before main 
request.

On the other hand, main request may clear body as well, and it 
won't be available for subrequests as a result.  This problem may 
be easily seen with ssi module.  The other possible case when this 
problem may appear is filter finalization (e.g. as done by image 
filter module).

That is, just disabling body cleanup for subrequests is clearly 
not enough.

I've already posted the patch which removes the code in question 
at all (as a reply to some real problems seen with ssi and POSTs, 
somewhere in russian mailing list).  Probably it should be made 
controllable via some configuration directive if there are some 
real use cases which may be seriously affected by removing this 
code.  I would like to hear something from Igor about this.

BTW, just for history: the code in question was introduced in 
nginx 0.3.3 with the following log message:

    *) Bugfix: a temporary file with client request body now is removed 
       just after the response header was transferred to a client.

Maxim Dounin


> 
> 
> -- src/http/ngx_http_upstream.c_orig	2011-03-08 08:01:20.000000000
> -0800
> +++ src/http/ngx_http_upstream.c	2011-03-08 08:01:41.000000000 -0800
> @@ -2033,7 +2033,7 @@ ngx_http_upstream_send_response(ngx_http
>  
>      u->header_sent = 1;
>  
> -    if (r->request_body && r->request_body->temp_file) {
> +    if (r->request_body && r->request_body->temp_file && r == r->main)
> {
>          ngx_pool_run_cleanup_file(r->pool,
> r->request_body->temp_file->file.fd);
>          r->request_body->temp_file->file.fd = NGX_INVALID_FILE;
>      }
> 
> Posted at Nginx Forum: http://forum.nginx.org/read.php?2,181400,181400#msg-181400
> 
> 
> _______________________________________________
> nginx mailing list
> nginx at nginx.org
> http://nginx.org/mailman/listinfo/nginx