From ccfb8dba2accd8b23aebf2813a47b3b36bf6bdf6 Mon Sep 17 00:00:00 2001 From: Jim Jagielski Date: Tue, 9 Jul 2013 12:35:23 +0000 Subject: [PATCH] Merge r1480627, r1482859, r1483190, r1484343, r1500437 from trunk: Mod_proxy used the global pool w/o mutex. fix. mod_proxy: Use a global mutex for handling workers. add in child_init which is needed Add in logno's conf->mutex is not used... Also, ensure that pool use is protected Submitted by: jim, minfrin, jim, jim, jim Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1501223 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 4 ++++ STATUS | 8 -------- modules/proxy/mod_proxy.c | 33 +++++++++++++++++++++++++++--- modules/proxy/mod_proxy.h | 2 +- modules/proxy/mod_proxy_balancer.c | 2 +- modules/proxy/proxy_util.c | 12 ++++++++++- 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index 0ee1ba1156..a8a08c5bc0 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,10 @@ Changes with Apache 2.4.5 + *) mod_proxy: Fix seg-faults when using the global pool on threaded + MPMs [Thomas Eckert , Graham Leggett, + Jim Jagielski] + *) mod_deflate: Remove assumptions as to when an EOS bucket might arrive. Gracefully step aside if the body size is zero. [Graham Leggett] diff --git a/STATUS b/STATUS index ca6fcc88c2..16676086ca 100644 --- a/STATUS +++ b/STATUS @@ -91,14 +91,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_proxy: Fix pool usage by protecting w/ a mutex - trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1480627 - http://svn.apache.org/viewvc?view=revision&revision=1482859 - http://svn.apache.org/viewvc?view=revision&revision=1483190 - http://svn.apache.org/viewvc?view=revision&revision=1484343 - http://svn.apache.org/viewvc?view=revision&revision=1500437 - 2.4.x patch: trunk works, modulo CHANGES - +1: jim, minfrin, sf PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index c6459728ee..0ee2ff35e1 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -36,6 +36,9 @@ APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup, #define MAX(x,y) ((x) >= (y) ? (x) : (y)) #endif +static const char * const proxy_id = "proxy"; +apr_global_mutex_t *proxy_mutex = NULL; + /* * A Web proxy module. Stages: * @@ -1193,7 +1196,7 @@ static void * create_proxy_config(apr_pool_t *p, server_rec *s) ps->badopt_set = 0; ps->source_address = NULL; ps->source_address_set = 0; - ps->pool = p; + apr_pool_create_ex(&ps->pool, p, NULL, NULL); return ps; } @@ -1255,7 +1258,7 @@ static void * merge_proxy_config(apr_pool_t *p, void *basev, void *overridesv) ps->proxy_status_set = overrides->proxy_status_set || base->proxy_status_set; ps->source_address = (overrides->source_address_set == 0) ? base->source_address : overrides->source_address; ps->source_address_set = overrides->source_address_set || base->source_address_set; - ps->pool = p; + ps->pool = base->pool; return ps; } static const char *set_source_address(cmd_parms *parms, void *dummy, @@ -2402,6 +2405,13 @@ PROXY_DECLARE(const char *) ap_proxy_ssl_val(apr_pool_t *p, server_rec *s, static int proxy_post_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *s) { + apr_status_t rv = ap_global_mutex_create(&proxy_mutex, NULL, + proxy_id, NULL, s, pconf, 0); + if (rv != APR_SUCCESS) { + ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, plog, APLOGNO(02478) + "failed to create %s mutex", proxy_id); + return rv; + } proxy_ssl_enable = APR_RETRIEVE_OPTIONAL_FN(ssl_proxy_enable); proxy_ssl_disable = APR_RETRIEVE_OPTIONAL_FN(ssl_engine_disable); @@ -2504,6 +2514,15 @@ static void child_init(apr_pool_t *p, server_rec *s) { proxy_worker *reverse = NULL; + apr_status_t rv = apr_global_mutex_child_init(&proxy_mutex, + apr_global_mutex_lockfile(proxy_mutex), + p); + if (rv != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s, APLOGNO(02479) + "could not init proxy_mutex in child"); + exit(1); /* Ugly, but what else? */ + } + /* TODO */ while (s) { void *sconf = s->module_config; @@ -2561,11 +2580,19 @@ static void child_init(apr_pool_t *p, server_rec *s) /* * This routine is called before the server processes the configuration - * files. There is no return value. + * files. */ static int proxy_pre_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp) { + apr_status_t rv = ap_mutex_register(pconf, proxy_id, NULL, + APR_LOCK_DEFAULT, 0); + if (rv != APR_SUCCESS) { + ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, plog, APLOGNO(02480) + "failed to register %s mutex", proxy_id); + return 500; /* An HTTP status would be a misnomer! */ + } + APR_OPTIONAL_HOOK(ap, status_hook, proxy_status_hook, NULL, NULL, APR_HOOK_MIDDLE); /* Reset workers count on gracefull restart */ diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index b8e95802b2..af6c2cbf47 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -164,7 +164,7 @@ typedef struct { status_full } proxy_status; /* Status display options */ apr_sockaddr_t *source_address; - apr_global_mutex_t *mutex; /* global lock (needed??) */ + apr_global_mutex_t *mutex; /* global lock, for pool, etc */ ap_slotmem_instance_t *bslot; /* balancers shm data - runtime */ ap_slotmem_provider_t *storage; diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index b1fd84eeb5..1e215126b4 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -1171,7 +1171,7 @@ static int balancer_handler(request_rec *r) (val = apr_table_get(params, "b_nwrkr"))) { char *ret; proxy_worker *nworker; - nworker = ap_proxy_get_worker(conf->pool, bsel, conf, val); + nworker = ap_proxy_get_worker(r->pool, bsel, conf, val); if (!nworker && storage->num_free_slots(bsel->wslot)) { if ((rv = PROXY_GLOBAL_LOCK(bsel)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01194) diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 961822fc2f..1a174cc5b8 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -69,6 +69,8 @@ static int lb_workers_limit = 0; const apr_strmatch_pattern PROXY_DECLARE_DATA *ap_proxy_strmatch_path; const apr_strmatch_pattern PROXY_DECLARE_DATA *ap_proxy_strmatch_domain; +extern apr_global_mutex_t *proxy_mutex; + static int proxy_match_ipaddr(struct dirconn_entry *This, request_rec *r); static int proxy_match_domainname(struct dirconn_entry *This, request_rec *r); static int proxy_match_hostname(struct dirconn_entry *This, request_rec *r); @@ -1730,12 +1732,14 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser else { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00927) "initializing worker %s local", worker->s->name); + 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; } } @@ -1744,6 +1748,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser 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; } @@ -1779,6 +1784,8 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_worker(proxy_worker *worker, ser "initialized single connection worker in child %" APR_PID_T_FMT " for (%s)", getpid(), worker->s->hostname); } + apr_global_mutex_unlock(proxy_mutex); + } if (rv == APR_SUCCESS) { worker->s->status |= (PROXY_WORKER_INITIALIZED); @@ -2766,15 +2773,18 @@ PROXY_DECLARE(apr_status_t) ap_proxy_sync_balancer(proxy_balancer *b, server_rec } if (!found) { proxy_worker **runtime; + apr_global_mutex_lock(proxy_mutex); runtime = apr_array_push(b->workers); *runtime = apr_palloc(conf->pool, sizeof(proxy_worker)); + apr_global_mutex_unlock(proxy_mutex); (*runtime)->hash = shm->hash; (*runtime)->context = NULL; (*runtime)->cp = NULL; (*runtime)->balancer = b; (*runtime)->s = shm; (*runtime)->tmutex = NULL; - if ((rv = ap_proxy_initialize_worker(*runtime, s, conf->pool)) != APR_SUCCESS) { + rv = ap_proxy_initialize_worker(*runtime, s, conf->pool); + if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(00966) "Cannot init worker"); return rv; } -- 2.50.1