Blob Blame History Raw
# ./pullrev.sh 1872790

http://svn.apache.org/viewvc?view=revision&revision=1872790

- Adjusted to remove (pointless-for-RHEL) additions of #if APR_HAS_THREADS.
- merged a previous change to connection_destructor in mod_proxy_balancer.c
- s/hostname_ex/hostname since 2.4.6 doesn't have hostname_ex

--- httpd-2.4.6/modules/proxy/mod_proxy_balancer.c.r1872790
+++ httpd-2.4.6/modules/proxy/mod_proxy_balancer.c
@@ -1229,12 +1229,23 @@
                     bsel->wupdated = bsel->s->wupdated = nworker->s->updated = apr_time_now();
                     /* by default, all new workers are disabled */
                     ap_proxy_set_wstatus('D', 1, nworker);
+                } else {
+                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10163)
+                                  "%s: failed to add worker %s",
+                                  bsel->s->name, val);
+                    PROXY_GLOBAL_UNLOCK(bsel);
+                    return HTTP_BAD_REQUEST;
                 }
                 if ((rv = PROXY_GLOBAL_UNLOCK(bsel)) != APR_SUCCESS) {
                     ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01203)
                                   "%s: Unlock failed for adding worker",
                                   bsel->s->name);
                 }
+            } else {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10164)
+                                  "%s: failed to add worker %s",
+                                  bsel->s->name, val);
+                return HTTP_BAD_REQUEST;
             }
 
         }
--- httpd-2.4.6/modules/proxy/mod_proxy_ftp.c.r1872790
+++ httpd-2.4.6/modules/proxy/mod_proxy_ftp.c
@@ -972,7 +972,7 @@
     conn_rec *origin, *data = NULL;
     apr_status_t err = APR_SUCCESS;
     apr_status_t uerr = APR_SUCCESS;
-    apr_bucket_brigade *bb = apr_brigade_create(p, c->bucket_alloc);
+    apr_bucket_brigade *bb;
     char *buf, *connectname;
     apr_port_t connectport;
     char buffer[MAX_STRING_LEN];
@@ -1112,13 +1112,15 @@
 
     if (worker->s->is_address_reusable) {
         if (!worker->cp->addr) {
+#if APR_HAS_THREADS
             if ((err = PROXY_THREAD_LOCK(worker->balancer)) != APR_SUCCESS) {
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, err, r, APLOGNO(01037) "lock");
                 return HTTP_INTERNAL_SERVER_ERROR;
             }
+#endif
         }
-        connect_addr = worker->cp->addr;
-        address_pool = worker->cp->pool;
+        connect_addr = AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr);
+        address_pool = worker->cp->dns_pool;
     }
     else
         address_pool = r->pool;
@@ -1206,6 +1208,7 @@
      * correct directory...
      */
 
+    bb = apr_brigade_create(p, c->bucket_alloc);
 
     /* possible results: */
     /* 120 Service ready in nnn minutes. */
--- httpd-2.4.6/modules/proxy/mod_proxy.h.r1872790
+++ httpd-2.4.6/modules/proxy/mod_proxy.h
@@ -260,12 +260,15 @@
 
 /* Connection pool */
 struct proxy_conn_pool {
-    apr_pool_t     *pool;   /* The pool used in constructor and destructor calls */
-    apr_sockaddr_t *addr;   /* Preparsed remote address info */
-    apr_reslist_t  *res;    /* Connection resource list */
-    proxy_conn_rec *conn;   /* Single connection for prefork mpm */
+    apr_pool_t     *pool;     /* The pool used in constructor and destructor calls */
+    apr_sockaddr_t *addr;     /* Preparsed remote address info */
+    apr_reslist_t  *res;      /* Connection resource list */
+    proxy_conn_rec *conn;     /* Single connection for prefork mpm */
+    apr_pool_t     *dns_pool; /* The pool used for worker scoped DNS resolutions */
 };
 
+#define AP_VOLATILIZE_T(T, x) (*(T volatile *)&(x))
+
 /* Keep below in sync with proxy_util.c! */
 /* worker status bits */
 #define PROXY_WORKER_INITIALIZED    0x0001
--- httpd-2.4.6/modules/proxy/proxy_util.c.r1872790
+++ httpd-2.4.6/modules/proxy/proxy_util.c
@@ -1318,16 +1324,14 @@
 
 static apr_status_t conn_pool_cleanup(void *theworker)
 {
-    proxy_worker *worker = (proxy_worker *)theworker;
-    if (worker->cp->res) {
-        worker->cp->pool = NULL;
-    }
+    ((proxy_worker *)theworker)->cp = NULL;
     return APR_SUCCESS;
 }
 
 static void init_conn_pool(apr_pool_t *p, proxy_worker *worker)
 {
     apr_pool_t *pool;
+    apr_pool_t *dns_pool;
     proxy_conn_pool *cp;
 
     /*
@@ -1339,11 +1343,20 @@
     apr_pool_create(&pool, p);
     apr_pool_tag(pool, "proxy_worker_cp");
     /*
+     * Create a subpool of the connection pool for worker
+     * scoped DNS resolutions. This is needed to avoid race
+     * conditions in using the connection pool by multiple
+     * threads during ramp up.
+     */
+    apr_pool_create(&dns_pool, pool);
+    apr_pool_tag(dns_pool, "proxy_worker_dns");
+    /*
      * Alloc from the same pool as worker.
      * proxy_conn_pool is permanently attached to the worker.
      */
     cp = (proxy_conn_pool *)apr_pcalloc(p, sizeof(proxy_conn_pool));
     cp->pool = pool;
+    cp->dns_pool = dns_pool;
     worker->cp = cp;
 }
 
@@ -1359,14 +1372,6 @@
     proxy_conn_rec *conn = (proxy_conn_rec *)theconn;
     proxy_worker *worker = conn->worker;
 
-    /*
-     * If the connection pool is NULL the worker
-     * cleanup has been run. Just return.
-     */
-    if (!worker->cp) {
-        return APR_SUCCESS;
-    }
-
     if (conn->r) {
         apr_pool_destroy(conn->r->pool);
         conn->r = NULL;
@@ -1487,10 +1492,11 @@
 static apr_status_t connection_destructor(void *resource, void *params,
                                           apr_pool_t *pool)
 {
-    proxy_conn_rec *conn = (proxy_conn_rec *)resource;
+    proxy_worker *worker = params;
 
     /* Destroy the pool only if not called from reslist_destroy */
-    if (conn->worker->cp->pool) {
+    if (worker->cp) {
+        proxy_conn_rec *conn = resource;
         apr_pool_destroy(conn->pool);
     }
 
@@ -1880,67 +1886,73 @@
                      ap_proxy_worker_name(p, worker));
     }
     else {
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00927)
-                     "initializing worker %s local",
-                     ap_proxy_worker_name(p, worker));
         apr_global_mutex_lock(proxy_mutex);
-        /* Now init local worker data */
-        if (worker->tmutex == NULL) {
-            rv = apr_thread_mutex_create(&(worker->tmutex), APR_THREAD_MUTEX_DEFAULT, p);
-            if (rv != APR_SUCCESS) {
-                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00928)
-                             "can not create worker thread mutex");
-                apr_global_mutex_unlock(proxy_mutex);
-                return rv;
+        /* Check again after we got the lock if we are still uninitialized */
+        if (!(AP_VOLATILIZE_T(unsigned int, worker->local_status) & PROXY_WORKER_INITIALIZED)) {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00927)
+                         "initializing worker %s local",
+                         ap_proxy_worker_name(p, worker));
+            /* Now init local worker data */
+#if APR_HAS_THREADS
+            if (worker->tmutex == NULL) {
+                rv = apr_thread_mutex_create(&(worker->tmutex), APR_THREAD_MUTEX_DEFAULT, p);
+                if (rv != APR_SUCCESS) {
+                    ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO(00928)
+                                 "can not create worker thread mutex");
+                    apr_global_mutex_unlock(proxy_mutex);
+                    return rv;
+                }
             }
-        }
-        if (worker->cp == NULL)
-            init_conn_pool(p, worker);
-        if (worker->cp == NULL) {
-            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00929)
-                         "can not create connection pool");
-            apr_global_mutex_unlock(proxy_mutex);
-            return APR_EGENERAL;
-        }
-
-        if (worker->s->hmax) {
-            rv = apr_reslist_create(&(worker->cp->res),
-                                    worker->s->min, worker->s->smax,
-                                    worker->s->hmax, worker->s->ttl,
-                                    connection_constructor, connection_destructor,
-                                    worker, worker->cp->pool);
-
-            apr_pool_cleanup_register(worker->cp->pool, (void *)worker,
-                                      conn_pool_cleanup,
-                                      apr_pool_cleanup_null);
-
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00930)
-                "initialized pool in child %" APR_PID_T_FMT " for (%s) min=%d max=%d smax=%d",
-                 getpid(), worker->s->hostname, worker->s->min,
-                 worker->s->hmax, worker->s->smax);
-
-            /* Set the acquire timeout */
-            if (rv == APR_SUCCESS && worker->s->acquire_set) {
-                apr_reslist_timeout_set(worker->cp->res, worker->s->acquire);
+#endif
+            if (worker->cp == NULL)
+                init_conn_pool(p, worker);
+            if (worker->cp == NULL) {
+                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00929)
+                             "can not create connection pool");
+                apr_global_mutex_unlock(proxy_mutex);
+                return APR_EGENERAL;
             }
 
-        }
-        else {
-            void *conn;
+            if (worker->s->hmax) {
+                rv = apr_reslist_create(&(worker->cp->res),
+                                        worker->s->min, worker->s->smax,
+                                        worker->s->hmax, worker->s->ttl,
+                                        connection_constructor, connection_destructor,
+                                        worker, worker->cp->pool);
+
+                apr_pool_pre_cleanup_register(worker->cp->pool, worker,
+                                              conn_pool_cleanup);
+
+                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00930)
+                    "initialized pool in child %" APR_PID_T_FMT " for (%s) min=%d max=%d smax=%d",
+                     getpid(), worker->s->hostname, worker->s->min,
+                     worker->s->hmax, worker->s->smax);
+
+                /* Set the acquire timeout */
+                if (rv == APR_SUCCESS && worker->s->acquire_set) {
+                    apr_reslist_timeout_set(worker->cp->res, worker->s->acquire);
+                }
 
-            rv = connection_constructor(&conn, worker, worker->cp->pool);
-            worker->cp->conn = conn;
+            }
+            else {
+                void *conn;
+
+                rv = connection_constructor(&conn, worker, worker->cp->pool);
+                worker->cp->conn = conn;
 
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00931)
-                 "initialized single connection worker in child %" APR_PID_T_FMT " for (%s)",
-                 getpid(), worker->s->hostname);
+                ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, s, APLOGNO(00931)
+                     "initialized single connection worker in child %" APR_PID_T_FMT " for (%s)",
+                     getpid(), worker->s->hostname);
+            }
+            if (rv == APR_SUCCESS) {
+                worker->local_status |= (PROXY_WORKER_INITIALIZED);
+            }
         }
         apr_global_mutex_unlock(proxy_mutex);
 
     }
     if (rv == APR_SUCCESS) {
         worker->s->status |= (PROXY_WORKER_INITIALIZED);
-        worker->local_status |= (PROXY_WORKER_INITIALIZED);
     }
     return rv;
 }
@@ -2183,13 +2195,13 @@
     else {
         /* create the new connection if the previous was destroyed */
         if (!worker->cp->conn) {
-            connection_constructor((void **)conn, worker, worker->cp->pool);
+            rv = connection_constructor((void **)conn, worker, worker->cp->pool);
         }
         else {
             *conn = worker->cp->conn;
             worker->cp->conn = NULL;
+            rv = APR_SUCCESS;
         }
-        rv = APR_SUCCESS;
     }
 
     if (rv != APR_SUCCESS) {
@@ -2374,15 +2386,25 @@
                 }
 
                 /*
-                 * Worker can have the single constant backend adress.
-                 * The single DNS lookup is used once per worker.
-                 * If dynamic change is needed then set the addr to NULL
-                 * inside dynamic config to force the lookup.
+                 * Recheck addr after we got the lock. This may have changed
+                 * while waiting for the lock.
                  */
-                err = apr_sockaddr_info_get(&(worker->cp->addr),
-                                            conn->hostname, APR_UNSPEC,
-                                            conn->port, 0,
-                                            worker->cp->pool);
+                if (!AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr)) {
+
+                    apr_sockaddr_t *addr;
+
+                    /*
+                     * Worker can have the single constant backend address.
+                     * The single DNS lookup is used once per worker.
+                     * If dynamic change is needed then set the addr to NULL
+                     * inside dynamic config to force the lookup.
+                     */
+                    err = apr_sockaddr_info_get(&addr,
+                                                conn->hostname, APR_UNSPEC,
+                                                conn->port, 0,
+                                                worker->cp->dns_pool);
+                    worker->cp->addr = addr;
+                }
                 conn->addr = worker->cp->addr;
                 if ((uerr = PROXY_THREAD_UNLOCK(worker)) != APR_SUCCESS) {
                     ap_log_rerror(APLOG_MARK, APLOG_ERR, uerr, r, APLOGNO(00946) "unlock");