bugfix: ngx.flush() buffer could be recycled while still queued

[setnicka]

Problem

ngx_http_lua_get_flush_chain() takes the flush buffer from ctx->free_bufs and leaves the module tag on it.

A flush buffer holds no data, so ngx_buf_special() is what makes its zero size acceptable to ngx_http_write_filter(), which may therefore keep it queued in r->out. But ngx_chain_update_chains() treats ngx_buf_size() == 0 as consumed, so it returns the buffer to ctx->free_bufs while the writer still references it. The next ngx_http_lua_chain_get_free_buf() memzeros it and sets temporary, so it is no longer special and the writer finds a zero size non-special buffer. This results in:

[alert] ... zero size buf in writer t:1 r:0 f:0 <start> <pos>-<last> 0000000000000000 0-0 while sending to client, ...

It returns NGX_ERROR, so the request is terminated, and the response truncated. I also think that two chain links can possibly end up pointing at the same buffer, in which case its contents may be sent twice.

The tag matters: an untagged flush buffer is skipped by ngx_chain_update_chains() and never recycled, so only the tagged one is affected.

How I ran into it

I used nginx as a caching reverse proxy, with Lua in the rewrite phase issuing cosocket requests and response bodies coming from the upstream API. Output was still queued in r->out (slow clients, and clients abandoning transfers with proxy_ignore_client_abort on), while ctx->busy_bufs was empty. So the flush buffer was the head the ngx_chain_update_chains() walks.

No explicit ngx.flush() is needed: ngx_http_lua_flush_pending_output() creates the flush chain exactly in that
ctx->busy_bufs == NULL case. The symptoms were truncated responses and alerts appearing a few times per hour per worker.

Fix

Allocate the chain link and the buffer directly and leave the buffer untagged, so ngx_chain_update_chains() drops the chain link without recycling the buffer. Cost is one ngx_buf_t per flush, reclaimed with the request pool.

Testing

No regression test. Triggering this needs ctx->free_bufs non-empty (so the buffer is tagged), ctx->busy_bufs empty, output still queued, and a later reuse. In synthetic tests, I was not able to reliably reproduce this reliably without stress testing with thousands of requests. The flush buffer is freshly allocated and therefore untagged, which is the unaffected state. The failure was confirmed on production traffic using debug logging that logged buffers returned to ctx->free_bufs while still linked in r->out.

I had to adapt the 1st test from 056-flush.t because of the new allocation.

Related

#1028 reports the same defect with an independent analysis; it was closed without a fix.

The same pattern probably exists for the last_buf / last_in_chain / flush markers in ngx_http_lua_body_filter_param_set(); left unchanged until confirmed.

---

I hereby granted the copyright of the changes in this pull request to the authors of this lua-nginx-module project.