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

[Sergey Kandaurov]

> On 19 Sep 2023, at 13:59, Sergey Kandaurov <pluknet at nginx.com> 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:
> 
>        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?

More specifically, the check in ngx_quic_input_handler()
seems to take care for all possible cases, it won't reach there.

> 
>> +    }
>> +
>>    for (q = ngx_queue_head(&qc->streams.uninitialized);
>>         q != ngx_queue_sentinel(&qc->streams.uninitialized);
>>         q = ngx_queue_next(q))
> 

Other patches look good.

-- 
Sergey Kandaurov