ASF Bugzilla – Attachment 35906 Details for
Bug 62308
Apache crashes after graceful restart with AH02599: slotmem (failed size check)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Change SHM filenames when not reusable (v5)
slotmem_reuse-v5.patch (text/plain), 14.30 KB, created by
Yann Ylavic
on 2018-05-03 00:49:21 UTC
(
hide
)
Description:
Change SHM filenames when not reusable (v5)
Filename:
MIME Type:
Creator:
Yann Ylavic
Created:
2018-05-03 00:49:21 UTC
Size:
14.30 KB
patch
obsolete
>Index: modules/slotmem/mod_slotmem_shm.c >=================================================================== >--- modules/slotmem/mod_slotmem_shm.c (revision 1830777) >+++ modules/slotmem/mod_slotmem_shm.c (working copy) >@@ -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,40 +289,29 @@ static APR_INLINE int is_child_process(void) > #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; > >- if (is_child_process()) { >- /* No reuse/retained data from here, let pconf cleanup everything */ >- *retained_globallistmem = globallistmem = NULL; >- return APR_SUCCESS; >- } >- >- /* When in startup/pre-config's cleanup, the retained data and global pool >- * are not used yet, but the SHMs contents were untouched hence they don't >- * need to be persisted, simply unlink them. >- * Otherwise when restarting or stopping we want to flush persisted data, >- * and in the stopping/exiting case we also want to unlink the SHMs. >+ /* 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); > } > } > >@@ -327,10 +318,10 @@ static APR_INLINE int is_child_process(void) > if (is_exiting) { > *retained_globallistmem = NULL; > } >- else if (!is_startup) { >+ else { >+ /* Restarting, save list for next run */ > *retained_globallistmem = globallistmem; > } >- globallistmem = NULL; > > return APR_SUCCESS; > } >@@ -368,19 +359,19 @@ static int check_slotmem(ap_slotmem_instance_t *me > > /* 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; > } > >@@ -387,6 +378,29 @@ static int check_slotmem(ap_slotmem_instance_t *me > 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); >+ size = strlen(name) + 1; >+ offset = strlen(*fname) + 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, >@@ -404,10 +418,12 @@ static apr_status_t slotmem_create(ap_slotmem_inst > 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 */ >@@ -414,10 +430,20 @@ static apr_status_t slotmem_create(ap_slotmem_inst > 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.%i", fname, generation); >+ persist = 0; >+ > next = next->next; > if (prev) { > prev->next = next; >@@ -433,7 +459,7 @@ static apr_status_t slotmem_create(ap_slotmem_inst > } > /* 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) { >@@ -444,7 +470,7 @@ static apr_status_t slotmem_create(ap_slotmem_inst > } > } > 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; >@@ -453,17 +479,16 @@ static apr_status_t slotmem_create(ap_slotmem_inst > > /* 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) { >- /* For MPMs (e.g. winnt) that run pre/post_config() phases in >- * both the parent and children processes, SHMs created by the >- * parent exist in the children already; only attach them. >+ /* For MPMs that run pre/post_config() phases in both the parent >+ * and children processes (e.g. winnt), SHMs created by the >+ * 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); >@@ -516,10 +541,11 @@ static apr_status_t slotmem_create(ap_slotmem_inst > 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; >@@ -554,21 +580,25 @@ static apr_status_t slotmem_attach(ap_slotmem_inst > 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; >@@ -575,8 +605,8 @@ static apr_status_t slotmem_attach(ap_slotmem_inst > *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) { >@@ -587,7 +617,7 @@ static apr_status_t slotmem_attach(ap_slotmem_inst > } > > /* 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; > } >@@ -598,7 +628,8 @@ static apr_status_t slotmem_attach(ap_slotmem_inst > > /* 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; >@@ -615,8 +646,8 @@ static apr_status_t slotmem_attach(ap_slotmem_inst > *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; > } > >@@ -828,7 +859,6 @@ static const ap_slotmem_provider_t *slotmem_shm_ge > 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); >@@ -837,27 +867,26 @@ static int pre_config(apr_pool_t *pconf, apr_pool_ > ap_retained_data_create(retained_key, > sizeof *retained_globallistmem); > } >- globallistmem = *retained_globallistmem; > >- /* For the first (dry-)loading or children in MPMs which (re-)run >- * pre_config we don't need to retain slotmems, so use pconf and its >- * normal cleanups. Otherwise we use ap_pglobal to match the lifetime >- * of retained data and register our own cleanup to update them. >+ /* 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 (is_child_process()) { >- gpool = pconf; >- } >- else if (ap_state_query(AP_SQ_MAIN_STATE) != AP_SQ_MS_CREATE_PRE_CONFIG) { >+ 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 { >- is_startup = (void *)1; >+ globallistmem = NULL; > gpool = pconf; > } > >- apr_pool_cleanup_register(pconf, is_startup, cleanup_slotmem, >- apr_pool_cleanup_null); >- > return OK; > } >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 62308
:
35878
|
35881
|
35882
|
35888
|
35899
|
35900
|
35902
|
35906
|
35936
|
35940
|
35942