git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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?