mod_proxy: Add ap_proxy_worker_get_name() and deprecate ap_proxy_worker_name().

The latter requires a pool and returns a non constant string although it may
return worker shared data.

By computing the worker "UDS" name at init time we can return a constant name
in any case with no need for a pool, that's the new ap_proxy_worker_get_name().



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912461 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 50 insertions, 39 deletions

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 9028490429..839ef0550d 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -720,6 +720,7 @@
  * 20211221.14 (2.5.1-dev) Add request_rec->final_resp_passed bit
  * 20211221.15 (2.5.1-dev) Add ap_get_pollfd_from_conn()
  * 20211221.16 (2.5.1-dev) Add ap_proxy_determine_address()
+ * 20211221.17 (2.5.1-dev) Add ap_proxy_worker_get_name()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -727,7 +728,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20211221
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 16             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 17             /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c
index bddbb838c0..fb15be958b 100644
--- a/modules/proxy/mod_proxy.c
+++ b/modules/proxy/mod_proxy.c
@@ -2241,14 +2241,14 @@ static const char *
             reuse = 1;
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01145)
                          "Sharing worker '%s' instead of creating new worker '%s'",
-                         ap_proxy_worker_name(cmd->pool, worker), new->real);
+                         ap_proxy_worker_get_name(worker), new->real);
         }
 
         for (i = 0; i < arr->nelts; i++) {
             if (reuse) {
                 ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(01146)
                              "Ignoring parameter '%s=%s' for worker '%s' because of worker sharing",
-                             elts[i].key, elts[i].val, ap_proxy_worker_name(cmd->pool, worker));
+                             elts[i].key, elts[i].val, ap_proxy_worker_get_name(worker));
             } else {
                 const char *err = set_worker_param(cmd->pool, s, worker, elts[i].key,
                                                    elts[i].val);
@@ -2793,13 +2793,13 @@ static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg)
             return apr_pstrcat(cmd->temp_pool, "BalancerMember ", err, NULL);
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01148)
                      "Defined worker '%s' for balancer '%s'",
-                     ap_proxy_worker_name(cmd->pool, worker), balancer->s->name);
+                     ap_proxy_worker_get_name(worker), balancer->s->name);
         PROXY_COPY_CONF_PARAMS(worker, conf);
     } else {
         reuse = 1;
         ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01149)
                      "Sharing worker '%s' instead of creating new worker '%s'",
-                     ap_proxy_worker_name(cmd->pool, worker), name);
+                     ap_proxy_worker_get_name(worker), name);
     }
     if (!worker->section_config) {
         worker->section_config = balancer->section_config;
@@ -2811,7 +2811,7 @@ static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg)
         if (reuse) {
             ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(01150)
                          "Ignoring parameter '%s=%s' for worker '%s' because of worker sharing",
-                         elts[i].key, elts[i].val, ap_proxy_worker_name(cmd->pool, worker));
+                         elts[i].key, elts[i].val, ap_proxy_worker_get_name(worker));
         } else {
             err = set_worker_param(cmd->pool, cmd->server, worker, elts[i].key,
                                    elts[i].val);
diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
index 2e032f589c..2cd2e06246 100644
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -513,6 +513,7 @@ struct proxy_worker {
     void            *context;   /* general purpose storage */
     ap_conf_vector_t *section_config; /* <Proxy>-section wherein defined */
     struct proxy_address *volatile address; /* current worker address (if reusable) */
+    const char      *uds_name;  /* "unix:/uds/path|worker-URL" */
 };
 
 /* default to health check every 30 seconds */
@@ -764,15 +765,24 @@ typedef __declspec(dllimport) const char *
 /* Connection pool API */
 /**
  * Return the user-land, UDS aware worker name
- * @param p        memory pool used for displaying worker name
+ * @param unused   memory pool unused
  * @param worker   the worker
- * @return         name
+ * @return         the name
+ * @note Even though the returned name is non constant char*, the string
+ *       it points to is shared and should *not* be modified by the caller!
+ * @deprecated Replaced by ap_proxy_worker_get_name()
  */
-
-PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
+PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *unused,
                                            proxy_worker *worker);
 
 /**
+ * Return the user-land, UDS aware worker name
+ * @param worker   the worker
+ * @return         the name
+ */
+PROXY_DECLARE(const char *) ap_proxy_worker_get_name(const proxy_worker *worker);
+
+/**
  * Return whether a worker upgrade configuration matches Upgrade header
  * @param p       memory pool used for displaying worker name
  * @param worker  the worker
diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index 97993afc52..1cc5463ac8 100644
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -156,7 +156,7 @@ static void init_balancer_members(apr_pool_t *p, server_rec *s,
         proxy_worker *worker = *workers;
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01158)
                      "Looking at %s -> %s initialized?", balancer->s->name,
-                     ap_proxy_worker_name(p, worker));
+                     ap_proxy_worker_get_name(worker));
         worker_is_initialized = PROXY_WORKER_IS_INITIALIZED(worker);
         if (!worker_is_initialized) {
             ap_proxy_initialize_worker(worker, s, p);
@@ -688,7 +688,7 @@ static int proxy_balancer_post_request(proxy_worker *worker,
                               "%s: Forcing worker (%s) into error state "
                               "due to status code %d matching 'failonstatus' "
                               "balancer parameter",
-                              balancer->s->name, ap_proxy_worker_name(r->pool, worker),
+                              balancer->s->name, ap_proxy_worker_get_name(worker),
                               val);
                 worker->s->status |= PROXY_WORKER_IN_ERROR;
                 worker->s->error_time = apr_time_now();
@@ -703,7 +703,7 @@ static int proxy_balancer_post_request(proxy_worker *worker,
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02460)
                       "%s: Forcing worker (%s) into error state "
                       "due to timeout and 'failontimeout' parameter being set",
-                       balancer->s->name, ap_proxy_worker_name(r->pool, worker));
+                       balancer->s->name, ap_proxy_worker_get_name(worker));
         worker->s->status |= PROXY_WORKER_IN_ERROR;
         worker->s->error_time = apr_time_now();
 
@@ -1486,7 +1486,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
                 worker = *workers;
                 /* Start proxy_worker */
                 ap_rputs("        <httpd:worker>\n", r);
-                ap_rvputs(r, "          <httpd:name>", ap_proxy_worker_name(r->pool, worker),
+                ap_rvputs(r, "          <httpd:name>", ap_proxy_worker_get_name(worker),
                           "</httpd:name>\n", NULL);
                 ap_rvputs(r, "          <httpd:scheme>", worker->s->scheme,
                           "</httpd:scheme>\n", NULL);
@@ -1727,7 +1727,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
                           ap_escape_uri(r->pool, worker->s->name),
                           "&amp;nonce=", balancer->s->nonce,
                           "\">", NULL);
-                ap_rvputs(r, (*worker->s->uds_path ? "<i>" : ""), ap_proxy_worker_name(r->pool, worker),
+                ap_rvputs(r, (*worker->s->uds_path ? "<i>" : ""), ap_proxy_worker_get_name(worker),
                           (*worker->s->uds_path ? "</i>" : ""), "</a></td>", NULL);
                 ap_rvputs(r, "<td>", ap_escape_html(r->pool, worker->s->route),
                           NULL);
@@ -1764,7 +1764,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
         }
         if (wsel && bsel) {
             ap_rputs("<h3>Edit worker settings for ", r);
-            ap_rvputs(r, (*wsel->s->uds_path?"<i>":""), ap_proxy_worker_name(r->pool, wsel), (*wsel->s->uds_path?"</i>":""), "</h3>\n", NULL);
+            ap_rvputs(r, (*wsel->s->uds_path?"<i>":""), ap_proxy_worker_get_name(wsel), (*wsel->s->uds_path?"</i>":""), "</h3>\n", NULL);
             ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action=\"", r);
             ap_rvputs(r, ap_escape_uri(r->pool, action), "\">\n", NULL);
             ap_rputs("<table><tr><td>Load factor:</td><td><input name='w_lf' id='w_lf' type=text ", r);
diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index fd6d2df865..4533e260cc 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -1652,7 +1652,7 @@ static void connection_cleanup(void *theconn)
         ap_log_perror(APLOG_MARK, APLOG_ERR, 0, conn->pool, APLOGNO(00923)
                       "Pooled connection 0x%pp for worker %s has been"
                       " already returned to the connection pool.", conn,
-                      ap_proxy_worker_name(conn->pool, worker));
+                      ap_proxy_worker_get_name(worker));
         return;
     }
 
@@ -1766,14 +1766,17 @@ static apr_status_t connection_destructor(void *resource, void *params,
  * WORKER related...
  */
 
-PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
+PROXY_DECLARE(const char *) ap_proxy_worker_get_name(const proxy_worker *worker)
+{
+    return worker->uds_name ? worker->uds_name : worker->s->name;
+}
+
+/* Deprecated/legacy */
+PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *unused,
                                            proxy_worker *worker)
 {
-    if (!(*worker->s->uds_path) || !p) {
-        /* just in case */
-        return worker->s->name;
-    }
-    return apr_pstrcat(p, "unix:", worker->s->uds_path, "|", worker->s->name, NULL);
+    (void)unused;
+    return (char *)ap_proxy_worker_get_name(worker);
 }
 
 PROXY_DECLARE(int) ap_proxy_worker_can_upgrade(apr_pool_t *p,
@@ -2118,6 +2121,12 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
         "worker hostname (%s) too long; truncated for legacy modules that do not use "
         "proxy_worker_shared->hostname_ex: %s", uri.hostname, wshared->hostname);
     }
+    if (sockpath) {
+        if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
+            return apr_psprintf(p, "worker uds path (%s) too long", sockpath);
+        }
+        (*worker)->uds_name = apr_pstrcat(p, "unix:", sockpath, "|", ptr, NULL);
+    }
     wshared->port = (uri.port) ? uri.port : port_of_scheme;
     wshared->flush_packets = flush_off;
     wshared->flush_wait = PROXY_FLUSH_WAIT;
@@ -2156,15 +2165,6 @@ PROXY_DECLARE(char *) ap_proxy_define_worker_ex(apr_pool_t *p,
             wshared->disablereuse = 1;
         }
     }
-    if (sockpath) {
-        if (PROXY_STRNCPY(wshared->uds_path, sockpath) != APR_SUCCESS) {
-            return apr_psprintf(p, "worker uds path (%s) too long", sockpath);
-        }
-
-    }
-    else {
-        *wshared->uds_path = '\0';
-    }
     if (!balancer) {
         wshared->status |= PROXY_WORKER_IGNORE_ERRORS;
     }
@@ -2232,7 +2232,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_share_worker(proxy_worker *worker, proxy_wo
         apr_pool_tag(pool, "proxy_worker_name");
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02338)
                      "%s shm[%d] (0x%pp) for worker: %s", action, i, (void *)shm,
-                     ap_proxy_worker_name(pool, worker));
+                     ap_proxy_worker_get_name(worker));
         if (pool) {
             apr_pool_destroy(pool);
         }
@@ -2250,12 +2250,12 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser
         /* The worker is already initialized */
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00924)
                      "worker %s shared already initialized",
-                     ap_proxy_worker_name(p, worker));
+                     ap_proxy_worker_get_name(worker));
     }
     else {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00925)
                      "initializing worker %s shared",
-                     ap_proxy_worker_name(p, worker));
+                     ap_proxy_worker_get_name(worker));
         /* Set default parameters */
         if (!worker->s->retry_set) {
             worker->s->retry = apr_time_from_sec(PROXY_WORKER_DEFAULT_RETRY);
@@ -2274,7 +2274,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser
             if (worker->s->disablereuse_set) {
                 ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(10400)
                              "enablereuse/disablereuse ignored for worker %s",
-                             ap_proxy_worker_name(p, worker));
+                             ap_proxy_worker_get_name(worker));
             }
             worker->s->disablereuse = 1;
         }
@@ -2318,7 +2318,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser
     if (worker->local_status & PROXY_WORKER_INITIALIZED) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00926)
                      "worker %s local already initialized",
-                     ap_proxy_worker_name(p, worker));
+                     ap_proxy_worker_get_name(worker));
     }
     else {
         apr_global_mutex_lock(proxy_mutex);
@@ -2326,7 +2326,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser
         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));
+                         ap_proxy_worker_get_name(worker));
             /* Now init local worker data */
 #if APR_HAS_THREADS
             if (worker->tmutex == NULL) {
@@ -4257,7 +4257,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer(proxy_balancer *b, server_rec
                 found = 1;
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02402)
                              "re-grabbing shm[%d] (0x%pp) for worker: %s", i, (void *)shm,
-                             ap_proxy_worker_name(conf->pool, worker));
+                             ap_proxy_worker_get_name(worker));
                 break;
             }
         }