[PATCH]HTTP/2 connection not properly closing during graceful shutdown

[Roman Arutyunyan]

Hi,

On Thu, Jul 11, 2024 at 08:51:54PM +0400, Sergey Kandaurov wrote:
> 
> > On 11 Jul 2024, at 13:33, Roman Arutyunyan <arut at nginx.com> wrote:
> > 
> > Hi,
> > 
> > On Mon, May 06, 2024 at 11:14:24AM +0800, kasei at kasei.im wrote:
> >> Hello,
> >> 
> >> Thanks for your confirmation and explanations.
> >> 
> >> The following is a modified patch. In my test it would send GOAWAY(on
> >> stream id 0), same as the previous one, but call
> >> ngx_http_v2_finalize_connection instead of set c->close.
> >> 
> >> # HG changeset patch
> >> # User Kasei Wang <kasei at kasei.im>
> >> # Date 1714965008 -28800
> >> #      Mon May 06 11:10:08 2024 +0800
> >> # Branch help
> >> # Node ID 70b6b6b69e6fd3a0d03de004acf45bad16b03a9c
> >> # Parent  8618e4d900cc71082fbe7dc72af087937d64faf5
> >> HTTP/2: close http2 connections initialized during graceful shutdown.
> > 
> > I'd remove 'http2' to fulfill the 67 character restriction.
> > 
> >> In some rare cases, a HTTP/2 connections can be initialized during a
> >> graceful shutdown. Now close such an connection to avoid unexcepted
> >> delays in the graceful shutdown.
> >> 
> >> diff -r 8618e4d900cc -r 70b6b6b69e6f src/http/v2/ngx_http_v2.c
> >> --- a/src/http/v2/ngx_http_v2.c Tue Apr 16 18:27:50 2024 +0400
> >> +++ b/src/http/v2/ngx_http_v2.c Mon May 06 11:10:08 2024 +0800
> >> @@ -304,6 +304,11 @@
> >>     c->idle = 1;
> >>     ngx_reusable_connection(c, 0);
> >> 
> >> +    if (ngx_exiting) {
> >> +        ngx_http_v2_finalize_connection(h2c, NGX_HTTP_V2_NO_ERROR);
> >> +        return;
> >> +    }
> > 
> > I'd move this up a few lines.
> 
> I think the block can be moved between setting c->data,
> as this is required for lingering close processing,
> and setting read and write handlers, as they're useless
> because connection is switched to lingering close mode
> as part of finalization.
> 
> > 
> >> +
> >>     if (c->buffer) {
> >>         p = c->buffer->pos;
> >>         end = c->buffer->last;
> > 
> > Otherwise looks fine, thanks.  And sorry for the delay.
> > 
> 
> -- 
> Sergey Kandaurov
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

Committed with minor changes:

https://hg.nginx.org/nginx/rev/d1b8568f3042

--
Roman Arutyunyan