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

[Roman Arutyunyan]

On Tue, Sep 19, 2023 at 01:59:27PM +0400, Sergey Kandaurov wrote:
> 
> > On 14 Sep 2023, at 14:17, Roman Arutyunyan <arut at nginx.com> wrote:
> > 
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at 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
-------------- next part --------------
# HG changeset patch
# User Roman Arutyunyan <arut at 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);