Skip to content

Commit

Permalink
mod_proxy: Add ap_proxy_worker_get_name() and deprecate ap_proxy_work…
Browse files Browse the repository at this point in the history 
…er_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
  • Loading branch information
@ylavic
ylavic committed Sep 21, 2023
1 parent d11d0b8 commit 29fb603
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 39 deletions.
3 changes: 2 additions & 1 deletion include/ap_mmn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,14 +720,15 @@
* 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" */

#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
Expand Down
10 changes: 5 additions & 5 deletions modules/proxy/mod_proxy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
18 changes: 14 additions & 4 deletions modules/proxy/mod_proxy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -764,14 +765,23 @@ 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
Expand Down
12 changes: 6 additions & 6 deletions modules/proxy/mod_proxy_balancer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
46 changes: 23 additions & 23 deletions modules/proxy/proxy_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -2318,15 +2318,15 @@ 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);
/* 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));
ap_proxy_worker_get_name(worker));
/* Now init local worker data */
#if APR_HAS_THREADS
if (worker->tmutex == NULL) {
Expand Down Expand Up @@ -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;
}
}
Expand Down

0 comments on commit 29fb603

Please sign in to comment.