> On 27 Jan 2022, at 04:24, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1643225036 -10800
> # Wed Jan 26 22:23:56 2022 +0300
> # Node ID f76e0cf8e525996563e5f0092fa48a4fee873e93
> # Parent 56ead48cfe885e8b89b30017459bf621b21d95f5
> HTTP/2: made it possible to flush response headers (ticket #1743).
>
> Response headers can be buffered in the SSL buffer. But stream's fake
> connection buffered flag did not reflect this, so any attempts to flush
> the buffer without sending additional data were stopped by the write filter.
>
> It does not seem to be possible to reflect this in fc->buffered though, as
> we never known if main connection's c->buffered corresponds to the particular
> stream or not. As such, fc->buffered might prevent request finalization
> due to sending data on some other stream.
>
> Fix is to implement handling of flush buffers when c->need_last_buf is set,
> similarly to the existing last buffer handling.
>
> diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c
> --- a/src/http/ngx_http_write_filter_module.c
> +++ b/src/http/ngx_http_write_filter_module.c
> @@ -227,7 +227,7 @@ ngx_http_write_filter(ngx_http_request_t
>
> if (size == 0
> && !(c->buffered & NGX_LOWLEVEL_BUFFERED)
> - && !(last && c->need_last_buf))
> + && !((last || flush) && c->need_last_buf))
> {
> if (last || flush || sync) {
> for (cl = r->out; cl; /* void */) {
> diff --git a/src/http/v2/ngx_http_v2_filter_module.c b/src/http/v2/ngx_http_v2_filter_module.c
> --- a/src/http/v2/ngx_http_v2_filter_module.c
> +++ b/src/http/v2/ngx_http_v2_filter_module.c
> @@ -1815,7 +1815,11 @@ ngx_http_v2_waiting_queue(ngx_http_v2_co
> static ngx_inline ngx_int_t
> ngx_http_v2_filter_send(ngx_connection_t *fc, ngx_http_v2_stream_t *stream)
> {
> - if (stream->queued == 0) {
> + ngx_connection_t *c;
> +
> + c = stream->connection->connection;
> +
> + if (stream->queued == 0 && !c->buffered) {
> fc->buffered &= ~NGX_HTTP_V2_BUFFERED;
> return NGX_OK;
> }
>

Nitpicking:
Shouldn't NGX_HTTP_V2_BUFFERED still be cleared regardless?

if (stream->queued == 0) {
fc->buffered &= ~NGX_HTTP_V2_BUFFERED;

if (!c->buffered) {
return NGX_OK;
}
}

It doesn't seem to change anything, at a glance.
However, this would save a contract that
(fc->buffered & NGX_HTTP_V2_BUFFERED) is influenced by stream->queued.

Otherwise, looks good.

--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org