# ./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");