On 3/18/22 10:52 AM, icing@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
> 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