You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2018/05/11 02:26:45 UTC
Re: svn commit: r1831218 - /httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> Author: ylavic
> Date: Wed May 9 01:23:59 2018
> New Revision: 1831218
>
> URL: http://svn.apache.org/viewvc?rev=1831218&view=rev
> Log:
> mod_proxy_balancer: follow up to r1830800.
[]
>
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1831218&r1=1831217&r2=1831218&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Wed May 9 01:23:59 2018
> @@ -1885,7 +1885,7 @@ static void balancer_child_init(apr_pool
> proxy_server_conf *conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
> apr_status_t rv;
>
> - if (conf->balancers->nelts) {
> + if (conf->balancers->nelts && !conf->bslot) {
> apr_size_t size;
> unsigned int num;
> storage->attach(&(conf->bslot), conf->id, &size, &num, p);
Hmm, storage is a provider and we always called ->attach() previously.
Possibly we'd rather fix the issue reported by Stefan the other way around:
1/ Revert this r1831218 (with a "feebacks welcome" comment)
- if (conf->balancers->nelts && !conf->bslot) {
+ if (conf->balancers->nelts) {
apr_size_t size;
unsigned int num;
- storage->attach(&(conf->bslot), conf->id, &size, &num, p);
+ /* In 2.4.x we rely on the provider to return either the same
+ * in/out &bslot, a valid new one, or NULL for failure/exit().
+ * For 2.6+/3.x we possibly could consider most errors to be real
+ * failures, and e.g. NOTFOUND and ENOSHM* would allow to continue
+ * with existing conf->bslot (even when returned one is NULL).
+ * Thus handle the slotmem reuse it here where we know it's valid
+ * both for fork()ed post_config()s and MPM winnt-like ones (run in
+ * child process too). The provider tells what it attached or not,
+ * and if not whether it's some system or own any error to fail
+ * with, or we can continue with the config one.
+ */
+ rv = storage->attach(&conf->bslot, conf->id, &size, &num, p);
if (!conf->bslot) {
- ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s,
APLOGNO(01205) "slotmem_attach failed");
+ ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
APLOGNO(01205) "slotmem_attach failed");
exit(1); /* Ugly, but what else? */
}
+ (void)rv;
}
2/ Don't mange conf->bslot in mod_slotmem_shm's attach(), added in r1830800
Index: modules/slotmem/mod_slotmem_shm.c
===================================================================
--- modules/slotmem/mod_slotmem_shm.c (revision 1831085)
+++ modules/slotmem/mod_slotmem_shm.c (working copy)
@@ -586,7 +586,6 @@ static apr_status_t slotmem_attach(ap_slotmem_inst
apr_shm_t *shm;
apr_status_t rv;
- *new = NULL;
ap_mpm_query(AP_MPMQ_GENERATION, &generation);
if (!slotmem_filenames(pool, name, &fname, NULL)) {
_
WDYT?