I seem to be having issues with this patch via this scenario: o Using the balancer-manager, make changes. o Do a grace restart some times. o Make more changes. o Now shutdown. o Now restart. The changes are erased...
It looks like maybe something to do with the generation number... ?? > On May 3, 2018, at 4:32 AM, [email protected] wrote: > > Author: ylavic > Date: Thu May 3 08:32:42 2018 > New Revision: 1830800 > > URL: http://svn.apache.org/viewvc?rev=1830800&view=rev > Log: > mod_slomem_shm: Handle a generation number when the slotmem size changes. > > Modifying the number of proxy balancers or balancer members on restart > could have prevented the server to load, notably on Windows. PR 62308. > > The generation number integrated in the SHM filename allows to create a > new/resized SHM while the previous is still in use by previous generation > gracefully shutting down (Windows prevents SHM/file to be removed in this > case, but even on Unix(es) an unlinked file might not be re-openable while > an inode exists). The generation number is added/incremented only if the > size requirement changed, such that unrelated restarts continue to share > SHMs between generations. > > The cleanup handling is also simplified because both the parent process and > the Windows child process need to cleanup everything on exit. This translates > to cleanup_slotmem() being always registered but in the dry load state > (AP_SQ_MS_CREATE_PRE_CONFIG), for both cases still. > > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c > > Modified: httpd/httpd/trunk/CHANGES > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1830800&r1=1830799&r2=1830800&view=diff > ============================================================================== > --- httpd/httpd/trunk/CHANGES [utf-8] (original) > +++ httpd/httpd/trunk/CHANGES [utf-8] Thu May 3 08:32:42 2018 > @@ -1,6 +1,11 @@ > -*- coding: utf-8 -*- > Changes with Apache 2.5.1 > > + *) mod_slomem_shm: Handle a generation number when the slotmem size > changes, > + modifying the number of proxy balancers or balancer members on restart > + could have prevented the server to load, notably on Windows. PR 62308. > + [Yann Ylavic] > + > *) mod_proxy_html: Fix variable interpolation and memory allocation failure > in ProxyHTMLURLMap. [Ewald Dieterich <ewald mailbox.org>] > > > Modified: httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c?rev=1830800&r1=1830799&r2=1830800&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c (original) > +++ httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c Thu May 3 08:32:42 > 2018 > @@ -26,6 +26,7 @@ > #include "httpd.h" > #include "http_main.h" > #include "http_core.h" > +#include "ap_mpm.h" > > #define AP_SLOTMEM_IS_PREGRAB(t) (t->desc->type & AP_SLOTMEM_TYPE_PREGRAB) > #define AP_SLOTMEM_IS_PERSIST(t) (t->desc->type & AP_SLOTMEM_TYPE_PERSIST) > @@ -42,7 +43,8 @@ typedef struct { > #define AP_UNSIGNEDINT_OFFSET (APR_ALIGN_DEFAULT(sizeof(unsigned int))) > > struct ap_slotmem_instance_t { > - char *name; /* file based SHM path/name */ > + char *name; /* file based SHM name (immutable) */ > + char *fname; /* file based SHM path/name */ > char *pname; /* persisted file path/name */ > int fbased; /* filebased? */ > void *shm; /* ptr to memory segment (apr_shm_t *) > */ > @@ -287,41 +289,37 @@ static APR_INLINE int is_child_process(v > #endif > } > > -static apr_status_t cleanup_slotmem(void *is_startup) > +static apr_status_t cleanup_slotmem(void *nil) > { > int is_exiting = (ap_state_query(AP_SQ_MAIN_STATE) == AP_SQ_MS_EXITING); > + int is_child = is_child_process(); > ap_slotmem_instance_t *mem; > > - /* When restarting or stopping we want to flush persisted data, and in > the > - * stopping case we also want to unlink the SHMs. On the first (dry-)load > - * though (is_startup != NULL), the retained list is not used yet and > SHMs > - * contents were untouched (so they don't need to be persisted); unlink > - * everything before the real loading (from scratch). > + /* When restarting or stopping we want to flush persisted data, and in > + * the stopping case we also want to unlink the SHMs. A MPM winnt child > + * process is always exiting here, we don't want it to care about > persisted > + * data but it still has to (try to) cleanup SHMs since it may be the > last > + * one to use them, and Windows prevent anything but the last user to > + * effectively destroy/remove an open SHM/file. > */ > for (mem = globallistmem; mem; mem = mem->next) { > - int unlink; > - if (is_startup) { > - unlink = mem->fbased; > + if (AP_SLOTMEM_IS_PERSIST(mem) && !is_child) { > + store_slotmem(mem); > } > - else { > - if (AP_SLOTMEM_IS_PERSIST(mem)) { > - store_slotmem(mem); > - } > - unlink = is_exiting; > - } > - if (unlink) { > - /* Some systems may require the descriptor to be closed before > - * unlink, thus call destroy() first. > + if (is_exiting) { > + /* Some systems require the descriptor to be closed before > + * unlinked, thus call destroy() first. > */ > apr_shm_destroy(mem->shm); > - apr_shm_remove(mem->name, mem->pool); > + apr_shm_remove(mem->fname, mem->pool); > } > } > > - if (is_startup || is_exiting) { > + if (is_exiting) { > *retained_globallistmem = NULL; > } > else { > + /* Restarting, save list for next run */ > *retained_globallistmem = globallistmem; > } > > @@ -361,25 +359,48 @@ static int check_slotmem(ap_slotmem_inst > > /* check size */ > if (apr_shm_size_get(mem->shm) != size) { > - ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599) > - "existing shared memory for %s could not be used " > + ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf, > APLOGNO(02599) > + "existing shared memory for %s could not be reused " > "(failed size check)", > - mem->name); > + mem->fname); > return 0; > } > > desc = apr_shm_baseaddr_get(mem->shm); > if (desc->size != item_size || desc->num != item_num) { > - ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02600) > - "existing shared memory for %s could not be used " > + ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf, > APLOGNO(02600) > + "existing shared memory for %s could not be reused " > "(failed contents check)", > - mem->name); > + mem->fname); > return 0; > } > > return 1; > } > > +static apr_status_t slotmem_attach_shm(apr_shm_t **shm, const char **fname, > + apr_uint32_t num, apr_pool_t *pool) > +{ > + char *name = NULL; > + apr_size_t size = 0, offset = 0; > + > + for (; num; --num) { > + if (!name) { > + name = apr_psprintf(pool, "%s.%u", *fname, APR_UINT32_MAX); > + offset = strlen(*fname) + 1; > + size = offset + strlen(name + offset) + 1; > + } > + apr_snprintf(name + offset, size - offset, "%u", num); > + > + if (apr_shm_attach(shm, name, pool) == APR_SUCCESS) { > + *fname = name; > + return APR_SUCCESS; > + } > + } > + > + return apr_shm_attach(shm, *fname, pool); > +} > + > static apr_status_t slotmem_create(ap_slotmem_instance_t **new, > const char *name, apr_size_t item_size, > unsigned int item_num, > @@ -397,20 +418,33 @@ static apr_status_t slotmem_create(ap_sl > apr_size_t size = AP_SLOTMEM_OFFSET + AP_UNSIGNEDINT_OFFSET + > (item_num * sizeof(char)) + basesize; > int persist = (type & AP_SLOTMEM_TYPE_PERSIST) != 0; > + int generation = 0; > apr_status_t rv; > apr_pool_t *p; > > *new = NULL; > + ap_mpm_query(AP_MPMQ_GENERATION, &generation); > > if (slotmem_filenames(pool, name, &fname, persist ? &pname : NULL)) { > /* first try to attach to existing slotmem */ > if (next) { > ap_slotmem_instance_t *prev = NULL; > for (;;) { > - if (strcmp(next->name, fname) == 0) { > + if (strcmp(next->name, name) == 0) { > *new = next; /* either returned here or reused finally */ > if (!check_slotmem(next, size, item_size, item_num)) { > + /* Can't reuse this SHM, a new one is needed with its > + * own filename (including generation number) because > + * the previous one may still be used by the previous > + * generation. Persisted file (if any) can't be > reused > + * either. > + */ > apr_shm_destroy(next->shm); > + apr_shm_remove(next->fname, pool); > + fname = apr_psprintf(pool, "%s.%u", fname, > + (apr_uint32_t)generation); > + persist = 0; > + > next = next->next; > if (prev) { > prev->next = next; > @@ -426,7 +460,7 @@ static apr_status_t slotmem_create(ap_sl > } > /* we already have it */ > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, > APLOGNO(02603) > - "create found %s in global list", fname); > + "create found %s in global list", > next->fname); > return APR_SUCCESS; > } > if (!next->next) { > @@ -437,7 +471,7 @@ static apr_status_t slotmem_create(ap_sl > } > } > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, > APLOGNO(02602) > - "create didn't find %s in global list", fname); > + "create didn't find %s in global list", name); > } > else { > fbased = 0; > @@ -446,8 +480,7 @@ static apr_status_t slotmem_create(ap_sl > > /* first try to attach to existing shared memory */ > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02300) > - "create %s: %"APR_SIZE_T_FMT"/%u", fname, item_size, > - item_num); > + "create %s: %"APR_SIZE_T_FMT"/%u", name, item_size, > item_num); > > { > if (fbased) { > @@ -456,7 +489,7 @@ static apr_status_t slotmem_create(ap_sl > * parent exist in the children already; attach them. > */ > if (is_child_process()) { > - rv = apr_shm_attach(&shm, fname, gpool); > + rv = slotmem_attach_shm(&shm, &fname, generation, gpool); > } > else { > apr_shm_remove(fname, pool); > @@ -509,10 +542,11 @@ static apr_status_t slotmem_create(ap_sl > res = *new; > if (res == NULL) { > res = apr_pcalloc(p, sizeof(ap_slotmem_instance_t)); > - res->name = apr_pstrdup(p, fname); > + res->name = apr_pstrdup(p, name); > res->pname = apr_pstrdup(p, pname); > *new = res; > } > + res->fname = apr_pstrdup(p, fname); > res->fbased = fbased; > res->shm = shm; > res->persist = (void *)ptr; > @@ -547,29 +581,33 @@ static apr_status_t slotmem_attach(ap_sl > ap_slotmem_instance_t *res; > ap_slotmem_instance_t *next = globallistmem; > sharedslotdesc_t *desc; > + int generation = 0; > const char *fname; > apr_shm_t *shm; > apr_status_t rv; > > + *new = NULL; > + ap_mpm_query(AP_MPMQ_GENERATION, &generation); > + > if (!slotmem_filenames(pool, name, &fname, NULL)) { > return APR_ENOSHMAVAIL; > } > > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02301) > - "attach looking for %s", fname); > + "attach looking for %s", name); > > /* first try to attach to existing slotmem */ > if (next) { > for (;;) { > - if (strcmp(next->name, fname) == 0) { > + if (strcmp(next->name, name) == 0) { > /* we already have it */ > *new = next; > *item_size = next->desc->size; > *item_num = next->desc->num; > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, > APLOGNO(02302) > - "attach found %s: %"APR_SIZE_T_FMT"/%u", fname, > - *item_size, *item_num); > + "attach found %s: %"APR_SIZE_T_FMT"/%u", > + next->fname, *item_size, *item_num); > return APR_SUCCESS; > } > if (!next->next) { > @@ -580,7 +618,7 @@ static apr_status_t slotmem_attach(ap_sl > } > > /* next try to attach to existing shared memory */ > - rv = apr_shm_attach(&shm, fname, pool); > + rv = slotmem_attach_shm(&shm, &fname, generation, pool); > if (rv != APR_SUCCESS) { > return rv; > } > @@ -591,7 +629,8 @@ static apr_status_t slotmem_attach(ap_sl > > /* For the chained slotmem stuff */ > res = apr_pcalloc(pool, sizeof(ap_slotmem_instance_t)); > - res->name = apr_pstrdup(pool, fname); > + res->name = apr_pstrdup(pool, name); > + res->fname = apr_pstrdup(pool, fname); > res->fbased = 1; > res->shm = shm; > res->persist = (void *)ptr; > @@ -608,8 +647,8 @@ static apr_status_t slotmem_attach(ap_sl > *item_num = desc->num; > ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, > APLOGNO(02303) > - "attach found %s: %"APR_SIZE_T_FMT"/%u", fname, > - *item_size, *item_num); > + "attach found %s: %"APR_SIZE_T_FMT"/%u", > + res->fname, *item_size, *item_num); > return APR_SUCCESS; > } > > @@ -821,7 +860,6 @@ static const ap_slotmem_provider_t *slot > static int pre_config(apr_pool_t *pconf, apr_pool_t *plog, > apr_pool_t *ptemp) > { > - void *is_startup = NULL; > const char *retained_key = "mod_slotmem_shm"; > > retained_globallistmem = ap_retained_data_get(retained_key); > @@ -830,30 +868,23 @@ static int pre_config(apr_pool_t *pconf, > ap_retained_data_create(retained_key, > sizeof *retained_globallistmem); > } > - globallistmem = *retained_globallistmem; > > - if (!is_child_process()) { > - /* For the first (dry-)load, create SHMs on pconf and let them be > - * cleaned up before the real loading. Otherwise SHMs shall have the > - * same lifetime as the retained list (ap_pglobal). The cleanup is to > - * update the retained list on restart, or to unlink the SHMs on stop > - * or after the first (dry-)load (is_startup != NULL). > - */ > - if (ap_state_query(AP_SQ_MAIN_STATE) != AP_SQ_MS_CREATE_PRE_CONFIG) { > - gpool = ap_pglobal; > - } > - else { > - is_startup = (void *)1; > - gpool = pconf; > - } > - > - apr_pool_cleanup_register(pconf, is_startup, cleanup_slotmem, > + /* For the first (dry-)load, create SHMs on pconf and let them be > + * cleaned up with it before the real loading. In any other case, SHMs > + * shall have the same lifetime as the retained list (ap_pglobal), so > + * the cleanup is to update the retained list on restart, and to unlink > + * the SHMs on stop. This also works for MPM winnt child process which > + * should rebuild the list first (i.e. attach SHMs, see slotmem_create), > + * and try to cleanup/remove SHMs before exiting (see cleanup_slotmem). > + */ > + if (ap_state_query(AP_SQ_MAIN_STATE) != AP_SQ_MS_CREATE_PRE_CONFIG) { > + apr_pool_cleanup_register(pconf, NULL, cleanup_slotmem, > apr_pool_cleanup_null); > + globallistmem = *retained_globallistmem; > + gpool = ap_pglobal; > } > else { > - /* For MPMs which (re-)run pre_config we don't need to retain > slotmems > - * in children, so use pconf and let it detach SHMs when children > exit. > - */ > + globallistmem = NULL; > gpool = pconf; > } > > >
