> On 19 Sep 2023, at 13:59, Sergey Kandaurov <pluknet@nginx.com> 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:
>
> 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
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel