mod_proxy: Allocate and pnitialize the workers and balancers on pconf.

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

diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c
index fb15be958bfb21b4ee2dd3c2ba8efaffe59597f2..7e682437b66edeae0c5601bec8b038adfc3ffc39 100644 (file)
--- a/modules/proxy/mod_proxy.c
+++ b/modules/proxy/mod_proxy.c
@@ -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,
@@ -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;
@@ -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 */
@@ -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;
     }
 }
 

diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index 1cc5463ac8cd2ab6d349e9619ded9bfde13fde1d..c42d0d6c393c554320558099a109a7f9d9643412 100644 (file)
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -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;
@@ -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;
@@ -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;
     }
 
 }