------------------------------------------------------------------------ r1702450 | ylavic | 2015-09-11 15:00:53 +0200 (Fri, 11 Sep 2015) | 10 lines mod_slotmem_shm: Fix balancers and balancer members slots reuse on restart when new ones are added (Windows and OS/2 only). PR 58024. Since Windows and OS/2 don't have the unlink() but the "delete on last close" semantic for opened files being removed, we can't reuse the same SHM files names for processes of different generation. Let's append the generation number in the files names for those. This commit also axes unused Unix specifics about mutexes/semaphores. ------------------------------------------------------------------------ r1702501 | ylavic | 2015-09-11 17:30:16 +0200 (Fri, 11 Sep 2015) | 3 lines mod_slotmem_shm: follow up to r1702450. Use the generation number from the MPM (AP_MPMQ_GENERATION) since AP_SQ_CONFIG_GEN is irrelevent in non-forked MPMs children. ------------------------------------------------------------------------ r1702473 | ylavic | 2015-09-11 15:58:44 +0200 (Fri, 11 Sep 2015) | 4 lines mod_slotmem_shm: follow up to r1702450. Rename ap_slotmem_instance_t's field 'name' to 'fname' since it really stores the file path/name of the file-based SHM. ------------------------------------------------------------------------ r1702955 | ylavic | 2015-09-14 16:04:19 +0200 (Mon, 14 Sep 2015) | 1 line mod_slotmem_shm: revert frivolous s/name/fname/ from r1702473, keep the comments. ------------------------------------------------------------------------ r1703149 | ylavic | 2015-09-15 13:01:40 +0200 (Tue, 15 Sep 2015) | 6 lines mod_slotmem_shm: follow up to r1702450. r1702450 changed the behaviour of slotmem_{create,attach}() when given given an absolute (SHM file )name. Don't mangle the SHM file name in this case, it's up to the caller to provide a unique name per call when this matters. ------------------------------------------------------------------------ r1703169 | ylavic | 2015-09-15 14:24:24 +0200 (Tue, 15 Sep 2015) | 6 lines mod_slotmem_shm: follow up to r1702450. Make systems without unlink() semantic happy by destroying (detaching) the SHM before removing the base file. Windows is supposed to have the "delete on last handle closed" semantic but yet fails here when this handle is owned by the same process (go figure!). ------------------------------------------------------------------------ r1703157 | ylavic | 2015-09-15 13:40:53 +0200 (Tue, 15 Sep 2015) | 4 lines mpm_winnt: make AP_MPMQ_GENERATION available in the child process as soon as possible, so that hooks/modules can query it at config stage. This is (e.g.) needed by r1702501 for PR 58024. ------------------------------------------------------------------------ r1703200 | ylavic | 2015-09-15 16:06:17 +0200 (Tue, 15 Sep 2015) | 1 line mod_slotmem_shm: follow up to r1702450; CHANGES entry. ------------------------------------------------------------------------ Index: CHANGES =================================================================== --- CHANGES (revision 1703173) +++ CHANGES (working copy) @@ -2,6 +2,12 @@ Changes with Apache 2.4.17 + *) mod_slotmem_shm: Fix slots/SHM files names on restart for systems that + can't create new (clear) slots while previous children gracefully stopping + still use the old ones (e.g. Windows, OS2). mod_proxy_balancer failed to + restart whenever the number of configured balancers/members changed during + restart. PR 58024. [Yann Ylavic] + *) mod_rewrite: Allow cookies set by mod_rewrite to contain ':' by accepting ';' as an alternate separator. PR47241. [, Eric Covener] Index: modules/slotmem/mod_slotmem_shm.c =================================================================== --- modules/slotmem/mod_slotmem_shm.c (revision 1703173) +++ modules/slotmem/mod_slotmem_shm.c (working copy) @@ -25,24 +25,8 @@ #include "httpd.h" #include "http_main.h" -#ifdef AP_NEED_SET_MUTEX_PERMS -#include "unixd.h" -#endif +#include "ap_mpm.h" /* for ap_mpm_query() */ -#if APR_HAVE_UNISTD_H -#include /* for getpid() */ -#endif - -#if HAVE_SYS_SEM_H -#include -#if !defined(SHM_R) -#define SHM_R 0400 -#endif -#if !defined(SHM_W) -#define SHM_W 0200 -#endif -#endif - #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) #define AP_SLOTMEM_IS_CLEARINUSE(t) (t->desc.type & AP_SLOTMEM_TYPE_CLEARINUSE) @@ -58,7 +42,8 @@ typedef struct { #define AP_UNSIGNEDINT_OFFSET (APR_ALIGN_DEFAULT(sizeof(unsigned int))) struct ap_slotmem_instance_t { - char *name; /* per segment name */ + char *name; /* file based SHM path/name */ + char *pname; /* persisted file path/name */ int fbased; /* filebased? */ void *shm; /* ptr to memory segment (apr_shm_t *) */ void *base; /* data set start */ @@ -86,6 +71,31 @@ static apr_pool_t *gpool = NULL; #define DEFAULT_SLOTMEM_SUFFIX ".shm" #define DEFAULT_SLOTMEM_PERSIST_SUFFIX ".persist" +/* Unixes (and Netware) have the unlink() semantic, which allows to + * apr_file_remove() a file still in use (opened elsewhere), the inode + * remains until the last fd is closed, whereas any file created with + * the same name/path will use a new inode. + * + * On windows and OS/2 ("\SHAREMEM\..." tree), apr_file_remove() marks + * the files for deletion until the last HANDLE is closed, meanwhile the + * same file/path can't be opened/recreated. + * Thus on graceful restart (the only restart mode with mpm_winnt), the + * old file may still exist until all the children stop, while we ought + * to create a new one for our new clear SHM. Therefore, we would only + * be able to reuse (attach) the old SHM, preventing some changes to + * the config file, like the number of balancers/members, since the + * size checks (to fit the new config) would fail. Let's avoid this by + * including the generation number in the SHM filename (obviously not + * the persisted name!) + */ +#ifndef SLOTMEM_UNLINK_SEMANTIC +#if defined(WIN32) || defined(OS2) +#define SLOTMEM_UNLINK_SEMANTIC 0 +#else +#define SLOTMEM_UNLINK_SEMANTIC 1 +#endif +#endif + /* * Persist the slotmem in a file * slotmem name and file name. @@ -94,29 +104,59 @@ static apr_pool_t *gpool = NULL; * /abs_name : $abs_name * */ +static int slotmem_filenames(apr_pool_t *pool, + const char *slotname, + const char **filename, + const char **persistname) +{ + const char *fname = NULL, *pname = NULL; -static const char *slotmem_filename(apr_pool_t *pool, const char *slotmemname, - int persist) -{ - const char *fname; - if (!slotmemname || strcasecmp(slotmemname, "none") == 0) { - return NULL; + if (slotname && *slotname && strcasecmp(slotname, "none") != 0) { + if (slotname[0] != '/') { +#if !SLOTMEM_UNLINK_SEMANTIC + /* Each generation needs its own file name. */ + int generation = 0; + ap_mpm_query(AP_MPMQ_GENERATION, &generation); + fname = apr_psprintf(pool, "%s%s_%x%s", DEFAULT_SLOTMEM_PREFIX, + slotname, generation, DEFAULT_SLOTMEM_SUFFIX); +#else + /* Reuse the same file name for each generation. */ + fname = apr_pstrcat(pool, DEFAULT_SLOTMEM_PREFIX, + slotname, DEFAULT_SLOTMEM_SUFFIX, + NULL); +#endif + fname = ap_runtime_dir_relative(pool, fname); + } + else { + /* Don't mangle the file name if given an absolute path, it's + * up to the caller to provide a unique name when necessary. + */ + fname = slotname; + } + + if (persistname) { + /* Persisted file names are immutable... */ +#if !SLOTMEM_UNLINK_SEMANTIC + if (slotname[0] != '/') { + pname = apr_pstrcat(pool, DEFAULT_SLOTMEM_PREFIX, + slotname, DEFAULT_SLOTMEM_SUFFIX, + DEFAULT_SLOTMEM_PERSIST_SUFFIX, + NULL); + pname = ap_runtime_dir_relative(pool, pname); + } + else +#endif + pname = apr_pstrcat(pool, fname, + DEFAULT_SLOTMEM_PERSIST_SUFFIX, + NULL); + } } - else if (slotmemname[0] != '/') { - const char *filenm = apr_pstrcat(pool, DEFAULT_SLOTMEM_PREFIX, - slotmemname, DEFAULT_SLOTMEM_SUFFIX, - NULL); - fname = ap_runtime_dir_relative(pool, filenm); - } - else { - fname = slotmemname; - } - if (persist) { - return apr_pstrcat(pool, fname, DEFAULT_SLOTMEM_PERSIST_SUFFIX, - NULL); + *filename = fname; + if (persistname) { + *persistname = pname; } - return fname; + return (fname != NULL); } static void slotmem_clearinuse(ap_slotmem_instance_t *slot) @@ -143,11 +183,9 @@ static void store_slotmem(ap_slotmem_instance_t *s apr_file_t *fp; apr_status_t rv; apr_size_t nbytes; - const char *storename; unsigned char digest[APR_MD5_DIGESTSIZE]; + const char *storename = slotmem->pname; - storename = slotmem_filename(slotmem->gpool, slotmem->name, 1); - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02334) "storing %s", storename); @@ -179,10 +217,9 @@ static void store_slotmem(ap_slotmem_instance_t *s } } -static apr_status_t restore_slotmem(void *ptr, const char *name, apr_size_t size, - apr_pool_t *pool) +static apr_status_t restore_slotmem(void *ptr, const char *storename, + apr_size_t size, apr_pool_t *pool) { - const char *storename; apr_file_t *fp; apr_size_t nbytes = size; apr_status_t rv = APR_SUCCESS; @@ -189,8 +226,6 @@ static void store_slotmem(ap_slotmem_instance_t *s unsigned char digest[APR_MD5_DIGESTSIZE]; unsigned char digest2[APR_MD5_DIGESTSIZE]; - storename = slotmem_filename(pool, name, 1); - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(02335) "restoring %s", storename); @@ -246,15 +281,11 @@ static apr_status_t cleanup_slotmem(void *param) if (AP_SLOTMEM_IS_PERSIST(next)) { store_slotmem(next); } + apr_shm_destroy((apr_shm_t *)next->shm); if (next->fbased) { - const char *name; apr_shm_remove(next->name, next->gpool); - name = slotmem_filename(next->gpool, next->name, 0); - if (name) { - apr_file_remove(name, next->gpool); - } + apr_file_remove(next->name, next->gpool); } - apr_shm_destroy((apr_shm_t *)next->shm); next = next->next; } } @@ -295,7 +326,6 @@ static apr_status_t slotmem_create(ap_slotmem_inst unsigned int item_num, ap_slotmem_type_t type, apr_pool_t *pool) { -/* void *slotmem = NULL; */ int fbased = 1; int restored = 0; char *ptr; @@ -302,18 +332,18 @@ static apr_status_t slotmem_create(ap_slotmem_inst sharedslotdesc_t desc; ap_slotmem_instance_t *res; ap_slotmem_instance_t *next = globallistmem; - const char *fname; + const char *fname, *pname = NULL; apr_shm_t *shm; apr_size_t basesize = (item_size * item_num); apr_size_t size = AP_SLOTMEM_OFFSET + AP_UNSIGNEDINT_OFFSET + (item_num * sizeof(char)) + basesize; + int persist = (type & AP_SLOTMEM_TYPE_PERSIST) != 0; apr_status_t rv; if (gpool == NULL) { return APR_ENOSHMAVAIL; } - fname = slotmem_filename(pool, name, 0); - if (fname) { + if (slotmem_filenames(pool, name, &fname, persist ? &pname : NULL)) { /* first try to attach to existing slotmem */ if (next) { for (;;) { @@ -399,8 +429,8 @@ static apr_status_t slotmem_create(ap_slotmem_inst * TODO: Error check the below... What error makes * sense if the restore fails? Any? */ - if (type & AP_SLOTMEM_TYPE_PERSIST) { - rv = restore_slotmem(ptr, fname, dsize, pool); + if (persist) { + rv = restore_slotmem(ptr, pname, dsize, pool); if (rv == APR_SUCCESS) { restored = 1; } @@ -417,6 +447,7 @@ static apr_status_t slotmem_create(ap_slotmem_inst res = (ap_slotmem_instance_t *) apr_pcalloc(gpool, sizeof(ap_slotmem_instance_t)); res->name = apr_pstrdup(gpool, fname); + res->pname = apr_pstrdup(gpool, pname); res->fbased = fbased; res->shm = shm; res->num_free = (unsigned int *)ptr; @@ -457,8 +488,7 @@ static apr_status_t slotmem_attach(ap_slotmem_inst if (gpool == NULL) { return APR_ENOSHMAVAIL; } - fname = slotmem_filename(pool, name, 0); - if (!fname) { + if (!slotmem_filenames(pool, name, &fname, NULL)) { return APR_ENOSHMAVAIL; } Index: server/mpm/winnt/mpm_winnt.c =================================================================== --- server/mpm/winnt/mpm_winnt.c (revision 1703173) +++ server/mpm/winnt/mpm_winnt.c (working copy) @@ -360,6 +360,13 @@ static int send_handles_to_child(apr_pool_t *p, HANDLE hScore; apr_size_t BytesWritten; + if ((rv = apr_file_write_full(child_in, &my_generation, + sizeof(my_generation), NULL)) + != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, ap_server_conf, APLOGNO(02964) + "Parent: Unable to send its generation to the child"); + return -1; + } if (!DuplicateHandle(hCurrentProcess, child_ready_event, hProcess, &hDup, EVENT_MODIFY_STATE | SYNCHRONIZE, FALSE, 0)) { ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(), ap_server_conf, APLOGNO(00392) @@ -1037,6 +1044,7 @@ static void winnt_rewrite_args(process_rec *proces { HANDLE filehand; HANDLE hproc = GetCurrentProcess(); + DWORD BytesRead; /* This is the child */ my_pid = GetCurrentProcessId(); @@ -1074,6 +1082,16 @@ static void winnt_rewrite_args(process_rec *proces * already */ + /* Read this child's generation number as soon as now, + * so that further hooks can query it. + */ + if (!ReadFile(pipe, &my_generation, sizeof(my_generation), + &BytesRead, (LPOVERLAPPED) NULL) + || (BytesRead != sizeof(my_generation))) { + ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(), NULL, APLOGNO(02965) + "Child: Unable to retrieve my generation from the parent"); + exit(APEXIT_CHILDINIT); + } /* The parent is responsible for providing the * COMPLETE ARGUMENTS REQUIRED to the child. @@ -1665,8 +1683,6 @@ static void winnt_child_init(apr_pool_t *pchild, s /* Done reading from the parent, close that channel */ CloseHandle(pipe); - - my_generation = ap_scoreboard_image->global->running_generation; } else { /* Single process mode - this lock doesn't even need to exist */ Index: . =================================================================== --- . (revision 1703173) +++ . (working copy) Property changes on: . ___________________________________________________________________ Modified: svn:mergeinfo Merged /httpd/httpd/trunk:r1702450,1702473,1702501,1702955,1703149,1703157,1703169,1703200