On 3/18/22 10:52 AM, ic...@apache.org wrote:
> Author: icing
> Date: Fri Mar 18 09:52:52 2022
> New Revision: 1899032
> 
> URL: http://svn.apache.org/viewvc?rev=1899032&view=rev
> Log:
>   *) core: adding a new hook and method to the API:
>      create_secondary_connection and ap_create_secondary_connection()
>      to setup connections related to a "master" one, as used in
>      the HTTP/2 protocol implementation.
> 
>   *) mod_http2: using the new API calls to get rid of knowledge
>      about how the core handles conn_rec specifics.
>      Improvements in pollset stream handling to use less sets.
>      Using atomic read/writes instead of volatiles now.
>      Keeping a reserve of "transit" pools and bucket_allocs for
>      use on secondary connections to avoid repeated setup/teardowns.
> 
> 
> Added:
>     httpd/httpd/trunk/changes-entries/core_secondary_conn.txt
> Modified:
>     httpd/httpd/trunk/   (props changed)
>     httpd/httpd/trunk/build/   (props changed)
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/include/http_connection.h
>     httpd/httpd/trunk/modules/http2/h2_c2.c
>     httpd/httpd/trunk/modules/http2/h2_c2.h
>     httpd/httpd/trunk/modules/http2/h2_conn_ctx.c
>     httpd/httpd/trunk/modules/http2/h2_conn_ctx.h
>     httpd/httpd/trunk/modules/http2/h2_mplx.c
>     httpd/httpd/trunk/modules/http2/h2_mplx.h
>     httpd/httpd/trunk/modules/http2/h2_stream.c
>     httpd/httpd/trunk/modules/http2/h2_workers.c
>     httpd/httpd/trunk/modules/http2/h2_workers.h
>     httpd/httpd/trunk/server/connection.c
>     httpd/httpd/trunk/server/core.c
>     httpd/httpd/trunk/test/modules/http2/test_711_load_post_cgi.py
> 


> Modified: httpd/httpd/trunk/server/core.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1899032&r1=1899031&r2=1899032&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Fri Mar 18 09:52:52 2022

> @@ -5517,6 +5519,54 @@ static conn_rec *core_create_conn(apr_po
>      return c;
>  }
>  
> +static conn_rec *core_create_secondary_conn(apr_pool_t *ptrans,
> +                                            conn_rec *master,
> +                                            apr_bucket_alloc_t *alloc)
> +{
> +    apr_pool_t *pool;
> +    conn_config_t *conn_config;
> +    conn_rec *c = (conn_rec *) apr_pmemdup(ptrans, master, sizeof(*c));

Why don't we allocate the structure from the subpool below?

> +
> +    /* Got a connection structure, so initialize what fields we can
> +     * (the rest are zeroed out by pcalloc).
> +     */

I don't get the comment above. We copy the existing master conn_rec. We
don't apr_pcalloc here.


> +    apr_pool_create(&pool, ptrans);

Why do we need a subpool here? In c2_transit_create we already create
a subpool with a dedicated allocator.
Is there a usecase for a subpool without a dedicated allocator?
If yes does it make sense to allow passing a dedicated allocator to the hook
to create a subpool using it (just like in c2_transit_create) and if NULL is
supplied only create a subpool like now?


> +    apr_pool_tag(pool, "secondary_conn");
> +
> +    /* we copied everything, now replace what is different */
> +    c->master = master;
> +    c->pool = pool;
> +    c->bucket_alloc = alloc;
> +    c->conn_config = ap_create_conn_config(pool);
> +    c->notes = apr_table_make(pool, 5);
> +    c->slaves = apr_array_make(pool, 20, sizeof(conn_slave_rec *));
> +    c->requests = apr_array_make(pool, 20, sizeof(request_rec *));
> +    c->input_filters = NULL;
> +    c->output_filters = NULL;
> +    c->filter_conn_ctx = NULL;
> +
> +    /* prevent mpm_event from making wrong assumptions about this connection,
> +     * like e.g. using its socket for an async read check. */
> +    c->clogging_input_filters = 1;
> +
> +    c->log = NULL;
> +    c->aborted = 0;
> +    c->keepalives = 0;
> +
> +    /* FIXME: mpms (and maybe other) parts think that there is always
> +     * a socket for a connection. We cannot use the master socket for
> +     * secondary connections, as this gets modified (closed?) when
> +     * the secondary connection terminates.
> +     * There seem to be some checks for c->master necessary in other
> +     * places.
> +     */
> +    conn_config = apr_pcalloc(pool, sizeof(*conn_config));
> +    conn_config->socket = dummy_socket;
> +    ap_set_core_module_config(c->conn_config, conn_config);
> +
> +    return c;
> +}
> +
>  static int core_pre_connection(conn_rec *c, void *csd)
>  {
>      conn_config_t *conn_config;


Regards

RĂ¼diger

Reply via email to