> On 2 Feb 2022, at 07:02, Maxim Dounin <mdou...@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 <mdou...@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
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-le...@nginx.org
