ngx_chain_get_free_buf crashes with free=0 after return next_body_filter

[Maxim Dounin]

Hello!

On Mon, Dec 03, 2012 at 10:34:22PM +0400, Ruslan Khusnullin wrote:

> On Mon, Dec 3, 2012 at 10:09 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> > Note: returning NGX_HTTP_INTERNAL_SERVER_ERROR is completely wrong
> > here, and it will result in undefined behaviour if ever returned.
> >
> > You should either return NGX_ERROR here (probably logging an alert
> > message) or just assume it should never happen unless process
> > memory is corrupted and just dereference the pointer (this way
> > you'll get proper segmentation fault instead of undefined
> > behviour).
> 
> Thank you for this, I have replaced all NGX_HTTP_INTERNAL_SERVER_ERROR
> with NGX_ERROR in body_filter and now it doesn't crash.
> 
> So is returning NGX_HTTP_*_ERROR valid only in a handler function?

Yes.

[...]

> Could you please also comment below piece of code? I've got it from
> previous coders and I'm not sure it needs to be there. It is a version
> of ngx_chain_add_copy that really copies data instead of copying
> pointers. Does it make sense? The module copies all chunks from
> upstream cache with this function and then when meeting buf->last_buf,
> it composes whole response in one memory block and modifies it. I
> guess that all the chunks remain in memory for all request time or I
> really need to make a copy of data in my context?

In general you have to make a copy if you need buffer data in 
memory longer than a single buffer invocation.  E.g. with

    output_buffers 1 4k;

you won't be able process files bigger than 4k if you'll keep 
buffers busy till request end - as there is only one buffer 
configured to read a file from disk and serve it to a client.

In most cases it's possible to do something better than just a 
dumb copy of all buffers (e.g. one may want to short-circut 
situation where the last buffer is readily available), but in a 
worst case copy is unavoidable (unless you'll change your code to 
do proper stream processing).

-- 
Maxim Dounin
http://nginx.com/support.html