From: Mladen Turk Date: Tue, 14 Dec 2004 15:45:19 +0000 (+0000) Subject: Make proxy address cache thread safe and available only X-Git-Tag: 2.1.3~262 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=939710c063c347cf240fa6ccc91229423fb688c5;p=thirdparty%2Fapache%2Fhttpd.git Make proxy address cache thread safe and available only to pooled workers. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@111827 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index 0804d3bff71..ec4444e7b41 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -1778,6 +1778,8 @@ static void child_init(apr_pool_t *p, server_rec *s) ap_proxy_initialize_worker(conf->forward, s); /* Do not disable worker in case of errors */ conf->forward->s->status |= PROXY_WORKER_IGNORE_ERRORS; + /* Disable address cache for generic forward worker */ + conf->forward->is_address_reusable = 0; } if (!reverse) { reverse = ap_proxy_create_worker(p); @@ -1788,6 +1790,8 @@ static void child_init(apr_pool_t *p, server_rec *s) ap_proxy_initialize_worker(reverse, s); /* Do not disable worker in case of errors */ reverse->s->status |= PROXY_WORKER_IGNORE_ERRORS; + /* Disable address cache for generic reverse worker */ + reverse->is_address_reusable = 0; } conf->reverse = reverse; s = s->next; diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 6f34ca5f145..c84c48bee14 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -278,6 +278,10 @@ struct proxy_worker { proxy_conn_pool *cp; /* Connection pool to use */ proxy_worker_stat *s; /* Shared data */ void *opaque; /* per scheme worker data */ + int is_address_reusable; +#if APR_HAS_THREADS + apr_thread_mutex_t *mutex; /* Thread lock for updating address cache */ +#endif }; struct proxy_balancer { @@ -299,6 +303,14 @@ struct proxy_balancer { #endif }; +#if APR_HAS_THREADS +#define PROXY_THREAD_LOCK(x) apr_thread_mutex_lock((x)->mutex) +#define PROXY_THREAD_UNLOCK(x) apr_thread_mutex_unlock((x)->mutex) +#else +#define PROXY_THREAD_LOCK(x) APR_SUCCESS +#define PROXY_THREAD_UNLOCK(x) APR_SUCCESS +#endif + /* hooks */ /* Create a set of PROXY_DECLARE(type), PROXY_DECLARE_NONSTD(type) and diff --git a/modules/proxy/proxy_balancer.c b/modules/proxy/proxy_balancer.c index 3e9ed23697c..6138911b9f5 100644 --- a/modules/proxy/proxy_balancer.c +++ b/modules/proxy/proxy_balancer.c @@ -23,14 +23,6 @@ module AP_MODULE_DECLARE_DATA proxy_balancer_module; -#if APR_HAS_THREADS -#define PROXY_BALANCER_LOCK(b) apr_thread_mutex_lock((b)->mutex) -#define PROXY_BALANCER_UNLOCK(b) apr_thread_mutex_unlock((b)->mutex) -#else -#define PROXY_BALANCER_LOCK(b) APR_SUCCESS -#define PROXY_BALANCER_UNLOCK(b) APR_SUCCESS -#endif - static int init_balancer_members(proxy_server_conf *conf, server_rec *s, proxy_balancer *balancer) { @@ -261,7 +253,7 @@ static proxy_worker *find_best_worker(proxy_balancer *balancer, proxy_worker *worker = (proxy_worker *)balancer->workers->elts; proxy_worker *candidate = NULL; - if (PROXY_BALANCER_LOCK(balancer) != APR_SUCCESS) + if (PROXY_THREAD_LOCK(balancer) != APR_SUCCESS) return NULL; /* First try to see if we have available candidate */ @@ -289,11 +281,11 @@ static proxy_worker *find_best_worker(proxy_balancer *balancer, if (candidate) { candidate->s->lbstatus -= total_factor; candidate->s->elected++; - PROXY_BALANCER_UNLOCK(balancer); + PROXY_THREAD_UNLOCK(balancer); return candidate; } else { - PROXY_BALANCER_UNLOCK(balancer); + PROXY_THREAD_UNLOCK(balancer); /* All the workers are in error state or disabled. * If the balancer has a timeout sleep for a while * and try again to find the worker. The chances are @@ -374,7 +366,7 @@ static int proxy_balancer_pre_request(proxy_worker **worker, /* Lock the LoadBalancer * XXX: perhaps we need the process lock here */ - if ((rv = PROXY_BALANCER_LOCK(*balancer)) != APR_SUCCESS) { + if ((rv = PROXY_THREAD_LOCK(*balancer)) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, "proxy: BALANCER: lock"); return DECLINED; @@ -407,11 +399,11 @@ static int proxy_balancer_pre_request(proxy_worker **worker, ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, "proxy: BALANCER: (%s). All workers are in error state for route (%s)", (*balancer)->name, route); - PROXY_BALANCER_UNLOCK(*balancer); + PROXY_THREAD_UNLOCK(*balancer); return HTTP_SERVICE_UNAVAILABLE; } - PROXY_BALANCER_UNLOCK(*balancer); + PROXY_THREAD_UNLOCK(*balancer); if (!*worker) { runtime = find_best_worker(*balancer, r); if (!runtime) { @@ -449,7 +441,7 @@ static int proxy_balancer_post_request(proxy_worker *worker, { apr_status_t rv; - if ((rv = PROXY_BALANCER_LOCK(balancer)) != APR_SUCCESS) { + if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, "proxy: BALANCER: lock"); return HTTP_INTERNAL_SERVER_ERROR; @@ -462,7 +454,7 @@ static int proxy_balancer_post_request(proxy_worker *worker, * track on that. */ - PROXY_BALANCER_UNLOCK(balancer); + PROXY_THREAD_UNLOCK(balancer); ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "proxy_balancer_post_request for (%s)", balancer->name); diff --git a/modules/proxy/proxy_ftp.c b/modules/proxy/proxy_ftp.c index c2257fa1d2d..aca55cc31db 100644 --- a/modules/proxy/proxy_ftp.c +++ b/modules/proxy/proxy_ftp.c @@ -759,7 +759,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, conn_rec *c = r->connection; proxy_conn_rec *backend; apr_socket_t *sock, *local_sock, *data_sock = NULL; - apr_sockaddr_t *connect_addr; + apr_sockaddr_t *connect_addr = NULL; apr_status_t rv; conn_rec *origin, *data = NULL; apr_status_t err = APR_SUCCESS; @@ -786,6 +786,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, int connect = 0, use_port = 0; char dates[APR_RFC822_DATE_LEN]; int status; + apr_pool_t *address_pool; /* is this for us? */ if (proxyhost) { @@ -893,12 +894,30 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "proxy: FTP: connecting %s to %s:%d", url, connectname, connectport); + if (worker->is_address_reusable) { + if (!worker->cp->addr) { + if ((err = PROXY_THREAD_LOCK(worker)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, err, r->server, + "proxy: FTP: lock"); + return HTTP_INTERNAL_SERVER_ERROR; + } + } + connect_addr = worker->cp->addr; + address_pool = worker->cp->pool; + } + else + address_pool = r->pool; + /* do a DNS lookup for the destination host */ - if (!worker->cp->addr) - err = apr_sockaddr_info_get(&(worker->cp->addr), + if (!connect_addr) + err = apr_sockaddr_info_get(&(connect_addr), connectname, APR_UNSPEC, connectport, 0, - worker->cp->pool); + address_pool); + if (worker->is_address_reusable && !worker->cp->addr) { + worker->cp->addr = connect_addr; + PROXY_THREAD_UNLOCK(worker); + } /* * get all the possible IP addresses for the destname and loop through * them until we get a successful connection @@ -910,7 +929,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, } /* check if ProxyBlock directive on this host */ - if (OK != ap_proxy_checkproxyblock(r, conf, worker->cp->addr)) { + if (OK != ap_proxy_checkproxyblock(r, conf, connect_addr)) { return ap_proxyerror(r, HTTP_FORBIDDEN, "Connect to remote machine blocked"); } @@ -927,7 +946,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, return status; } /* TODO: see if ftp could use determine_connection */ - backend->addr = worker->cp->addr; + backend->addr = connect_addr; ap_set_module_config(c->conn_config, &proxy_ftp_module, backend); } @@ -942,7 +961,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, if (ap_proxy_connect_backend("FTP", backend, worker, r->server)) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "proxy: FTP: an error occurred creating a new connection to %pI (%s)", - worker->cp->addr, connectname); + connect_addr, connectname); proxy_ftp_cleanup(r, backend); return HTTP_SERVICE_UNAVAILABLE; } @@ -957,7 +976,6 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, /* Use old naming */ origin = backend->connection; - connect_addr = worker->cp->addr; sock = backend->sock; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c index dd28d3170c5..8200c8f1ca8 100644 --- a/modules/proxy/proxy_http.c +++ b/modules/proxy/proxy_http.c @@ -493,7 +493,7 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r, if (status != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: request failed to %pI (%s)", - conn->worker->cp->addr, conn->hostname); + conn->addr, conn->hostname); return status; } } @@ -573,7 +573,7 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r, if (status != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: pass request data failed to %pI (%s)", - conn->worker->cp->addr, conn->hostname); + conn->addr, conn->hostname); return status; } @@ -619,7 +619,7 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r, if (status != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: pass request data failed to %pI (%s)", - conn->worker->cp->addr, conn->hostname); + conn->addr, conn->hostname); return status; } @@ -631,7 +631,7 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r, if (status != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: pass request data failed to %pI (%s)", - conn->worker->cp->addr, conn->hostname); + conn->addr, conn->hostname); return status; } apr_brigade_length(body_brigade, 0, &transfered); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 331ba1619e5..5f98db4f2a0 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1146,6 +1146,13 @@ PROXY_DECLARE(const char *) ap_proxy_add_worker(proxy_worker **worker, /* Increase the total worker count */ proxy_lb_workers++; init_conn_pool(p, *worker); +#if APR_HAS_THREADS + if (apr_thread_mutex_create(&((*worker)->mutex), + APR_THREAD_MUTEX_DEFAULT, p) != APR_SUCCESS) { + /* XXX: Do we need to log something here */ + return "can not create thread mutex"; + } +#endif return NULL; } @@ -1427,6 +1434,8 @@ PROXY_DECLARE(void) ap_proxy_initialize_worker_share(proxy_server_conf *conf, /* Set default parameters */ if (!worker->retry) worker->retry = apr_time_from_sec(PROXY_WORKER_DEFAULT_RETRY); + /* By default address is reusable */ + worker->is_address_reusable = 1; } @@ -1640,14 +1649,20 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, } } /* TODO: add address cache for forward proxies */ - conn->addr = worker->cp->addr; - if (r->proxyreq == PROXYREQ_PROXY) { + if (r->proxyreq == PROXYREQ_PROXY || r->proxyreq == PROXYREQ_REVERSE || + !worker->is_address_reusable) { err = apr_sockaddr_info_get(&(conn->addr), conn->hostname, APR_UNSPEC, conn->port, 0, conn->pool); } else if (!worker->cp->addr) { + if ((err = PROXY_THREAD_LOCK(worker)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, err, r->server, + "proxy: lock"); + return HTTP_INTERNAL_SERVER_ERROR; + } + /* 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 @@ -1658,6 +1673,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, conn->port, 0, worker->cp->pool); conn->addr = worker->cp->addr; + PROXY_THREAD_UNLOCK(worker); } if (err != APR_SUCCESS) { return ap_proxyerror(r, HTTP_BAD_GATEWAY, @@ -1675,7 +1691,6 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, server_port); } } - /* check if ProxyBlock directive on this host */ if (OK != ap_proxy_checkproxyblock(r, conf, conn->addr)) { return ap_proxyerror(r, HTTP_FORBIDDEN,