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);
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel