Skip to content

Commit

Permalink
mod_proxy: Allocate and pnitialize the workers and balancers on pconf.
Browse files Browse the repository at this point in the history 
On ungraceful restart, pchild might be destroyed without waiting for the MPM
threads, just before exit()ing but still there is a window where threads may
be using its data still.

Avoid possible exit path crashes by basing the workers/balancers on pconf,
which is not destroyed in children processes.

While at it, avoid the duplication of the generic "forward" worker for each
server(_rec), there can be a single instance like the generic "reverse"
worker.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912463 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@ylavic
ylavic committed Sep 21, 2023
1 parent 08cde3e commit 8eb0f9e
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 35 deletions.
54 changes: 29 additions & 25 deletions modules/proxy/mod_proxy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3368,6 +3368,8 @@ static int proxy_status_hook(request_rec *r, int flags)

static void child_init(apr_pool_t *p, server_rec *s)
{
proxy_server_conf *main_conf;
proxy_worker *forward = NULL;
proxy_worker *reverse = NULL;

apr_status_t rv = apr_global_mutex_child_init(&proxy_mutex,
Expand All @@ -3379,8 +3381,8 @@ static void child_init(apr_pool_t *p, server_rec *s)
exit(1); /* Ugly, but what else? */
}

/* TODO */
while (s) {
main_conf = ap_get_module_config(s->module_config, &proxy_module);
for (; s; s = s->next) {
void *sconf = s->module_config;
proxy_server_conf *conf;
proxy_worker *worker;
Expand All @@ -3393,32 +3395,36 @@ static void child_init(apr_pool_t *p, server_rec *s)
*/
worker = (proxy_worker *)conf->workers->elts;
for (i = 0; i < conf->workers->nelts; i++, worker++) {
ap_proxy_initialize_worker(worker, s, p);
ap_proxy_initialize_worker(worker, s, conf->pool);
}

/* Create and initialize forward worker if defined */
if (conf->req_set && conf->req) {
proxy_worker *forward;
ap_proxy_define_worker(conf->pool, &forward, NULL, NULL,
if (conf->req_set && conf->req && !forward) {
ap_proxy_define_worker(main_conf->pool, &forward, NULL, NULL,
"http://www.apache.org", 0);
conf->forward = forward;
PROXY_STRNCPY(conf->forward->s->name, "proxy:forward");
PROXY_STRNCPY(conf->forward->s->hostname, "*"); /* for compatibility */
PROXY_STRNCPY(conf->forward->s->hostname_ex, "*");
PROXY_STRNCPY(conf->forward->s->scheme, "*");
conf->forward->hash.def = conf->forward->s->hash.def =
ap_proxy_hashfunc(conf->forward->s->name, PROXY_HASHFUNC_DEFAULT);
conf->forward->hash.fnv = conf->forward->s->hash.fnv =
ap_proxy_hashfunc(conf->forward->s->name, PROXY_HASHFUNC_FNV);
PROXY_STRNCPY(forward->s->name, "proxy:forward");
PROXY_STRNCPY(forward->s->hostname, "*"); /* for compatibility */
PROXY_STRNCPY(forward->s->hostname_ex, "*");
PROXY_STRNCPY(forward->s->scheme, "*");
forward->hash.def = forward->s->hash.def =
ap_proxy_hashfunc(forward->s->name, PROXY_HASHFUNC_DEFAULT);
forward->hash.fnv = forward->s->hash.fnv =
ap_proxy_hashfunc(forward->s->name, PROXY_HASHFUNC_FNV);
/* Do not disable worker in case of errors */
conf->forward->s->status |= PROXY_WORKER_IGNORE_ERRORS;
forward->s->status |= PROXY_WORKER_IGNORE_ERRORS;
/* Mark as the "generic" worker */
conf->forward->s->status |= PROXY_WORKER_GENERIC;
ap_proxy_initialize_worker(conf->forward, s, p);
/* Disable address cache for generic forward worker */
conf->forward->s->is_address_reusable = 0;
forward->s->status |= PROXY_WORKER_GENERIC;
/* Disable connection and address reuse for generic workers */
forward->s->is_address_reusable = 0;
ap_proxy_initialize_worker(forward, s, main_conf->pool);
}
if (conf->req_set && conf->req) {
conf->forward = forward;
}

/* Create and initialize the generic reserse worker once only */
if (!reverse) {
ap_proxy_define_worker(conf->pool, &reverse, NULL, NULL,
ap_proxy_define_worker(main_conf->pool, &reverse, NULL, NULL,
"http://www.apache.org", 0);
PROXY_STRNCPY(reverse->s->name, "proxy:reverse");
PROXY_STRNCPY(reverse->s->hostname, "*"); /* for compatibility */
Expand All @@ -3432,13 +3438,11 @@ static void child_init(apr_pool_t *p, server_rec *s)
reverse->s->status |= PROXY_WORKER_IGNORE_ERRORS;
/* Mark as the "generic" worker */
reverse->s->status |= PROXY_WORKER_GENERIC;
conf->reverse = reverse;
ap_proxy_initialize_worker(conf->reverse, s, p);
/* Disable address cache for generic reverse worker */
/* Disable connection and address reuse for generic workers */
reverse->s->is_address_reusable = 0;
ap_proxy_initialize_worker(reverse, s, main_conf->pool);
}
conf->reverse = reverse;
s = s->next;
}
}

Expand Down
17 changes: 7 additions & 10 deletions modules/proxy/mod_proxy_balancer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,11 @@ static int proxy_balancer_canon(request_rec *r, char *url)
return OK;
}

static void init_balancer_members(apr_pool_t *p, server_rec *s,
proxy_balancer *balancer)
static void init_balancer_members(proxy_balancer *balancer,
server_rec *s, apr_pool_t *p)
{
int i;
proxy_worker **workers;

workers = (proxy_worker **)balancer->workers->elts;
proxy_worker **workers = (proxy_worker **)balancer->workers->elts;

for (i = 0; i < balancer->workers->nelts; i++) {
int worker_is_initialized;
Expand Down Expand Up @@ -2024,7 +2022,7 @@ static int balancer_handler(request_rec *r)

static void balancer_child_init(apr_pool_t *p, server_rec *s)
{
while (s) {
for (; s; s = s->next) {
proxy_balancer *balancer;
int i;
void *sconf = s->module_config;
Expand Down Expand Up @@ -2055,17 +2053,16 @@ static void balancer_child_init(apr_pool_t *p, server_rec *s)

balancer = (proxy_balancer *)conf->balancers->elts;
for (i = 0; i < conf->balancers->nelts; i++, balancer++) {
rv = ap_proxy_initialize_balancer(balancer, s, p);

rv = ap_proxy_initialize_balancer(balancer, s, conf->pool);
if (rv != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s, APLOGNO(01206)
"Failed to init balancer %s in child",
balancer->s->name);
exit(1); /* Ugly, but what else? */
}
init_balancer_members(p, s, balancer);

init_balancer_members(balancer, s, conf->pool);
}
s = s->next;
}

}
Expand Down

0 comments on commit 8eb0f9e

Please sign in to comment.