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

[Sergey Kandaurov]

> On 11 Sep 2023, at 15:30, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1694431436 -14400
> #      Mon Sep 11 15:23:56 2023 +0400
> # Node ID 9ee4158b9d3fa41e647b772e707c29b3e4cb77b5
> # Parent  65574b876c4047495ac8622c6161fc87c1b75913
> HTTP/3: eliminated v3_session field from ngx_http_connection_t.
> 
> The field was under NGX_HTTP_V3 macro, which was a source of binary
> compatibility problems when nginx/module is build with/without HTTP/3 support.
> 
> Now ngx_http_v3_session_t is assigned to c->data on streams initialization,
> while ngx_http_connection_t object is referenced by http_connection field of
> the session object, similar to ngx_http_v2_connection_t and ngx_http_request_t.

I'd write explicitly (taking from the 1st patch commit log) that session
creation is now postponed, to make the description self contained, YMMV.

Note that this change will trigger segfaults on graceful shutdown
if there are connections with handshake not yet complete,
due to now postponed session creation.
In this case, http3 shutdown() callback is called, used to send
GOAWAY and proceed with connection close, but h3c is not yet.
Probably calling the callback should depend on c->ssl->handshaked,
because it has little sense if handshake isn't complete.

> 
> diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
> --- a/src/http/ngx_http_request.h
> +++ b/src/http/ngx_http_request.h
> @@ -324,10 +324,6 @@ typedef struct {
> #endif
> #endif
> 
> -#if (NGX_HTTP_V3 || NGX_COMPAT)
> -    ngx_http_v3_session_t            *v3_session;
> -#endif
> -
>     ngx_chain_t                      *busy;
>     ngx_int_t                         nbusy;
> 
> diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c
> --- a/src/http/v3/ngx_http_v3.c
> +++ b/src/http/v3/ngx_http_v3.c
> @@ -30,6 +30,8 @@ ngx_http_v3_init_session(ngx_connection_
>         goto failed;
>     }
> 
> +    h3c->http_connection = hc;
> +
>     ngx_queue_init(&h3c->blocked);
> 
>     h3c->keepalive.log = c->log;
> @@ -48,7 +50,7 @@ ngx_http_v3_init_session(ngx_connection_
>     cln->handler = ngx_http_v3_cleanup_session;
>     cln->data = h3c;
> 
> -    hc->v3_session = h3c;
> +    c->data = h3c;
> 
>     return NGX_OK;
> 
> diff --git a/src/http/v3/ngx_http_v3.h b/src/http/v3/ngx_http_v3.h
> --- a/src/http/v3/ngx_http_v3.h
> +++ b/src/http/v3/ngx_http_v3.h
> @@ -78,11 +78,12 @@
> #define NGX_HTTP_V3_ERR_DECODER_STREAM_ERROR       0x202
> 
> 
> -#define ngx_http_quic_get_connection(c)                                       \
> -    ((ngx_http_connection_t *) ((c)->quic ? (c)->quic->parent->data           \
> +#define ngx_http_v3_get_session(c)                                            \
> +    ((ngx_http_v3_session_t *) ((c)->quic ? (c)->quic->parent->data           \
>                                           : (c)->data))
> 
> -#define ngx_http_v3_get_session(c)  ngx_http_quic_get_connection(c)->v3_session
> +#define ngx_http_quic_get_connection(c)                                       \
> +    (ngx_http_v3_get_session(c)->http_connection)
> 
> #define ngx_http_v3_get_module_loc_conf(c, module)                            \
>     ngx_http_get_module_loc_conf(ngx_http_quic_get_connection(c)->conf_ctx,   \
> @@ -120,6 +121,8 @@ struct ngx_http_v3_parse_s {
> 
> 
> struct ngx_http_v3_session_s {
> +    ngx_http_connection_t        *http_connection;
> +
>     ngx_http_v3_dynamic_table_t   table;
> 
>     ngx_event_t                   keepalive;
> diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> --- a/src/http/v3/ngx_http_v3_request.c
> +++ b/src/http/v3/ngx_http_v3_request.c
> @@ -70,11 +70,6 @@ ngx_http_v3_init_stream(ngx_connection_t
>     h3scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v3_module);
> 
>     if (c->quic == NULL) {
> -        if (ngx_http_v3_init_session(c) != NGX_OK) {
> -            ngx_http_close_connection(c);
> -            return;
> -        }
> -
>         h3scf->quic.idle_timeout = clcf->keepalive_timeout;
>         ngx_quic_run(c, &h3scf->quic);
>         return;
> @@ -112,6 +107,10 @@ ngx_http_v3_init(ngx_connection_t *c)
> 
>     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 init");
> 
> +    if (ngx_http_v3_init_session(c) != NGX_OK) {
> +        return NGX_ERROR;
> +    }
> +
>     h3c = ngx_http_v3_get_session(c);
>     clcf = ngx_http_v3_get_module_loc_conf(c, ngx_http_core_module);
>     ngx_add_timer(&h3c->keepalive, clcf->keepalive_timeout);

-- 
Sergey Kandaurov