Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov
September 22, 2023 07:24AM
On Thu, Sep 21, 2023 at 07:40:32PM +0400, Roman Arutyunyan wrote:
> On Tue, Sep 19, 2023 at 01:59:27PM +0400, Sergey Kandaurov wrote:
> >
> > > On 14 Sep 2023, at 14:17, Roman Arutyunyan <arut@nginx.com> wrote:
> > >
> > > # HG changeset patch
> > > # User Roman Arutyunyan <arut@nginx.com>
> > > # Date 1694613709 -14400
> > > # Wed Sep 13 18:01:49 2023 +0400
> > > # Node ID 51166a8f35ba880415ddc2bf2745012a8d4cea34
> > > # Parent 6d3ca6f8db357a1db267978f730875e51e87c608
> > > QUIC: call shutdown() callback only after handshake completion.
> > >
> > > Previously the callback could be called while QUIC handshake was in progress
> > > and, what's more important, before the init() callback. Now it's postponed
> > > after init().
> > >
> > > This change is a preparation to postponing HTTP/3 session creation to init().
> > >
> > > diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> > > --- a/src/event/quic/ngx_event_quic.c
> > > +++ b/src/event/quic/ngx_event_quic.c
> > > @@ -427,7 +427,7 @@ ngx_quic_input_handler(ngx_event_t *rev)
> > > return;
> > > }
> > >
> > > - if (!qc->closing && qc->conf->shutdown) {
> > > + if (!qc->closing && qc->streams.initialized && qc->conf->shutdown) {
> > > qc->conf->shutdown(c);
> >
> > Adding condition here will now prevent doing anything on graceful shutdown,
> > input handler will just return, connection will stuck for handshake_timeout.
> > I'd rather move it above, to handle similar to closing reusable connections:
>
> Nothing will be done if handshake is in progress, in which case the shutdown()
> handler will be called after init(), see the change below.
>
> > if (!ngx_exiting || !qc->streams.initialized) {
> >
> >
> > > }
> > >
> > > diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
> > > --- a/src/event/quic/ngx_event_quic_streams.c
> > > +++ b/src/event/quic/ngx_event_quic_streams.c
> > > @@ -620,6 +620,10 @@ ngx_quic_do_init_streams(ngx_connection_
> > > }
> > > }
> > >
> > > + if (ngx_exiting && qc->conf->shutdown) {
> > > + qc->conf->shutdown(c);
> >
> > how this can be reached?
> >
> > > + }
> > > +
> > > for (q = ngx_queue_head(&qc->streams.uninitialized);
> > > q != ngx_queue_sentinel(&qc->streams.uninitialized);
> > > q = ngx_queue_next(q))
>
> While the above change tries to be as graceful as possible, there can be
> another solution that's less complex. We can just terminate the connection
> with handshake in progress instead of waiting for its completion and sending
> GOAWAY.
>
> --
> Roman Arutyunyan

> # HG changeset patch
> # User Roman Arutyunyan <arut@nginx.com>
> # Date 1695310358 -14400
> # Thu Sep 21 19:32:38 2023 +0400
> # Node ID ed9c0b01d341ac0cc13ab55b06d5a2cc3572d3fa
> # Parent 2e657cae3de01aa55079cc2e4d43215aeeccb324
> QUIC: do not call shutdown() when handshake is in progress.
>
> Instead, when worker is shutting down and handshake is not yet completed,
> connection is terminated immediately.
>
> Previously the callback could be called while QUIC handshake was in progress
> and, what's more important, before the init() callback. Now it's postponed
> after init().
>
> This change is a preparation to postponing HTTP/3 session creation to init().
>
> diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> --- a/src/event/quic/ngx_event_quic.c
> +++ b/src/event/quic/ngx_event_quic.c
> @@ -421,7 +421,7 @@ ngx_quic_input_handler(ngx_event_t *rev)
> if (c->close) {
> c->close = 0;
>
> - if (!ngx_exiting) {
> + if (!ngx_exiting || !qc->streams.initialized) {
> qc->error = NGX_QUIC_ERR_NO_ERROR;
> qc->error_reason = "graceful shutdown";
> ngx_quic_close_connection(c, NGX_ERROR);

Although this approach may look more abrupt to close connection for
seemingly no reason, I think it is more safe, in terms of how long
a client can control the lifetime of a shutting down worker process.

Looks good for me.
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 3] QUIC module compatibility issues

Roman Arutyunyan 366 September 11, 2023 07:32AM

[PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 83 September 11, 2023 07:32AM

Re: [PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Sergey Kandaurov 85 September 12, 2023 09:06AM

Re: [PATCH 1 of 3] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 86 September 13, 2023 11:56AM

[PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Roman Arutyunyan 87 September 11, 2023 07:32AM

Re: [PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Sergey Kandaurov 81 September 12, 2023 09:06AM

Re: [PATCH 2 of 3] HTTP/3: eliminated v3_session field from ngx_http_connection_t

Roman Arutyunyan 79 September 13, 2023 12:04PM

[PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 84 September 11, 2023 07:32AM

Re: [PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Sergey Kandaurov 89 September 12, 2023 09:06AM

Re: [PATCH 3 of 3] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 81 September 13, 2023 12:08PM

[PATCH 0 of 6] QUIC module compatibility issues

Roman Arutyunyan 92 September 14, 2023 06:18AM

[PATCH 1 of 6] QUIC: "handshake_timeout" configuration parameter

Roman Arutyunyan 107 September 14, 2023 06:18AM

[PATCH 2 of 6] HTTP/3: moved variable initialization

Roman Arutyunyan 94 September 14, 2023 06:18AM

[PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Roman Arutyunyan 87 September 14, 2023 06:18AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 87 September 19, 2023 06:00AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 91 September 19, 2023 06:34AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Roman Arutyunyan 89 September 21, 2023 11:42AM

Re: [PATCH 3 of 6] QUIC: call shutdown() callback only after handshake completion

Sergey Kandaurov 92 September 22, 2023 07:24AM

[PATCH 4 of 6] HTTP/3: postponed session creation to init() callback

Roman Arutyunyan 86 September 14, 2023 06:18AM

[PATCH 5 of 6] QUIC: simplified setting close timer when closing connection

Roman Arutyunyan 89 September 14, 2023 06:18AM

[PATCH 6 of 6] Modules compatibility: added QUIC to signature (ticket #2539)

Roman Arutyunyan 77 September 14, 2023 06:18AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 121
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready