[PATCH] HTTP/2: made it possible to flush response headers (ticket #1743)

[Sergey Kandaurov]

> On 2 Feb 2022, at 07:02, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> [..]
> As discussed privately, it might be good idea to keep the code in 
> line with UDP handling in the stream module.  Here is an updated 
> patch which introduces the c->need_flush_buf flag, and uses 
> this flag instead of the c->type checking in the stream module.
> 
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1643773980 -10800
> #      Wed Feb 02 06:53:00 2022 +0300
> # Node ID 3a695d5d222262cd57c76124d7d0b3e3863a86c6
> # Parent  dd718d1cef3c2ca2754a21176be3318b00be9e62
> 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 the c->need_flush_buf
> flag is set, similarly to the existing last buffer handling.  The same
> flag is now used for UDP sockets in the stream module instead of explicit
> checking of c->type.
> 
> diff --git a/src/core/ngx_connection.h b/src/core/ngx_connection.h
> --- a/src/core/ngx_connection.h
> +++ b/src/core/ngx_connection.h
> @@ -184,6 +184,7 @@ struct ngx_connection_s {
>     unsigned            tcp_nopush:2;    /* ngx_connection_tcp_nopush_e */
> 
>     unsigned            need_last_buf:1;
> +    unsigned            need_flush_buf:1;
> 
> #if (NGX_HAVE_SENDFILE_NODISKIO || NGX_COMPAT)
>     unsigned            busy_count:2;
> diff --git a/src/event/ngx_event_connect.c b/src/event/ngx_event_connect.c
> --- a/src/event/ngx_event_connect.c
> +++ b/src/event/ngx_event_connect.c
> @@ -179,6 +179,8 @@ ngx_event_connect_peer(ngx_peer_connecti
>         c->recv = ngx_udp_recv;
>         c->send = ngx_send;
>         c->send_chain = ngx_udp_send_chain;
> +
> +        c->need_flush_buf = 1;
>     }
> 
>     c->log_error = pc->log_error;
> diff --git a/src/event/ngx_event_udp.c b/src/event/ngx_event_udp.c
> --- a/src/event/ngx_event_udp.c
> +++ b/src/event/ngx_event_udp.c
> @@ -246,6 +246,8 @@ ngx_event_recvmsg(ngx_event_t *ev)
>         c->send = ngx_udp_send;
>         c->send_chain = ngx_udp_send_chain;
> 
> +        c->need_flush_buf = 1;
> +
>         c->log = log;
>         c->pool->log = log;
>         c->listening = ls;
> 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,8 @@ ngx_http_write_filter(ngx_http_request_t
> 
>     if (size == 0
>         && !(c->buffered & NGX_LOWLEVEL_BUFFERED)
> -        && !(last && c->need_last_buf))
> +        && !(last && c->need_last_buf)
> +        && !(flush && c->need_flush_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
> @@ -665,6 +665,7 @@ ngx_http_v2_header_filter(ngx_http_reque
> 
>     fc->send_chain = ngx_http_v2_send_chain;
>     fc->need_last_buf = 1;
> +    fc->need_flush_buf = 1;
> 
>     return ngx_http_v2_filter_send(fc, stream);
> }
> @@ -1815,7 +1816,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;
>     }
> diff --git a/src/stream/ngx_stream_write_filter_module.c b/src/stream/ngx_stream_write_filter_module.c
> --- a/src/stream/ngx_stream_write_filter_module.c
> +++ b/src/stream/ngx_stream_write_filter_module.c
> @@ -235,7 +235,7 @@ ngx_stream_write_filter(ngx_stream_sessi
>     if (size == 0
>         && !(c->buffered & NGX_LOWLEVEL_BUFFERED)
>         && !(last && c->need_last_buf)
> -        && !(c->type == SOCK_DGRAM && flush))
> +        && !(flush && c->need_flush_buf))
>     {
>         if (last || flush || sync) {
>             for (cl = *out; cl; /* void */) {
> 

Thanks for the update, it looks good.

-- 
Sergey Kandaurov