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 0804d3bff7110be9d49c80ed22b1eb10580db019..ec4444e7b415deb476b689920235a9ca9b95a8a9 100644 (file)
--- 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 6f34ca5f145009e4fcebbe12e29edc952265496d..c84c48bee14ba4f339c626c78e714ddfdc02e045 100644 (file)
--- 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 3e9ed23697c617b946bedbb1de5b61fb5d1bc48c..6138911b9f5ae8fe9caccf251373f3910529f3a6 100644 (file)
--- 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 c2257fa1d2df248c1ad47c4389a194c49bbfe77b..aca55cc31db49934d208e9caab0c80719709b577 100644 (file)
--- 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 dd28d3170c5fd71ffb9b2cc8191857d1331bd818..8200c8f1ca8a11728c9fc08c85feecc815b6e293 100644 (file)
--- 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 331ba1619e53e95547a7d2db1b434168c7a66a29..5f98db4f2a079ce387011c3ccd6ad0de8b672503 100644 (file)
--- 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,