From 077a87403f89fc0d052337f7276415bac3209757 Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Thu, 14 Mar 2024 10:49:08 -0700 Subject: [PATCH 1/4] Use anonymous shared memory segments here --- .github/workflows/build-and-test.yml | 1 - src/mod_tile.c | 26 ++++++-------------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 7baeb02a..3d76580f 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -152,7 +152,6 @@ jobs: INSTALL_PREFIX: /usr/local INSTALL_RUNSTATEDIR: /var/run LDFLAGS: -undefined dynamic_lookup - TEST_PARALLEL_LEVEL: 1 name: >- ${{ matrix.os }} (${{ matrix.build_system }}) diff --git a/src/mod_tile.c b/src/mod_tile.c index 39c3d29b..8c0e7970 100644 --- a/src/mod_tile.c +++ b/src/mod_tile.c @@ -81,8 +81,6 @@ APLOG_USE_MODULE(tile); apr_shm_t *stats_shm; apr_shm_t *delaypool_shm; -char *shmfilename; -char *shmfilename_delaypool; apr_global_mutex_t *stats_mutex; apr_global_mutex_t *delay_mutex; @@ -1673,37 +1671,25 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, return OK; } /* Kilroy was here */ - /* Create the shared memory segment */ - - /* - * Create a unique filename using our pid. This information is - * stashed in the global variable so the children inherit it. - * TODO get the location from the environment $TMPDIR or somesuch. - */ - shmfilename = apr_psprintf(pconf, "/tmp/httpd_shm.%ld", (long int)getpid()); - shmfilename_delaypool = apr_psprintf(pconf, "/tmp/httpd_shm_delay.%ld", (long int)getpid()); - - /* Now create that segment + /* Create the shared memory segment * would prefer to use scfg->configs->nelts here but that does * not seem to be set at this stage, so rely on previously set layerCount */ rs = apr_shm_create(&stats_shm, sizeof(stats_data) + layerCount * 2 * sizeof(apr_uint64_t), - (const char *)shmfilename, pconf); + NULL, pconf); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rs, s, - "Failed to create shared memory segment on file %s", - shmfilename); + "Failed to create 'stats' shared memory segment"); return HTTP_INTERNAL_SERVER_ERROR; } rs = apr_shm_create(&delaypool_shm, sizeof(delaypool), - (const char *)shmfilename_delaypool, pconf); + NULL, pconf); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rs, s, - "Failed to create shared memory segment on file %s", - shmfilename_delaypool); + "Failed to create 'delaypool' shared memory segment"); return HTTP_INTERNAL_SERVER_ERROR; } @@ -1852,7 +1838,7 @@ static void mod_tile_child_init(apr_pool_t *p, server_rec *s) if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_CRIT, rs, s, "Failed to reopen mutex on file %s", - shmfilename); + mutexfilename); /* There's really nothing else we can do here, since * This routine doesn't return a status. */ exit(1); /* Ugly, but what else? */ From e40839e4ca29a4fa37cd9380c7c0c45cdfcc3d3a Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:34:52 -0700 Subject: [PATCH 2/4] Rename delay_mutex to delaypool_mutex (to match stats) --- src/mod_tile.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/mod_tile.c b/src/mod_tile.c index 8c0e7970..de448be0 100644 --- a/src/mod_tile.c +++ b/src/mod_tile.c @@ -82,7 +82,7 @@ APLOG_USE_MODULE(tile); apr_shm_t *stats_shm; apr_shm_t *delaypool_shm; apr_global_mutex_t *stats_mutex; -apr_global_mutex_t *delay_mutex; +apr_global_mutex_t *delaypool_mutex; char *mutexfilename; int layerCount = 0; @@ -816,7 +816,7 @@ static int delay_allowed(request_rec *r, enum tileState state) return 1; } - if (get_global_lock(r, delay_mutex) == 0) { + if (get_global_lock(r, delaypool_mutex) == 0) { ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Could not acquire lock, skipping delay pool accounting\n"); return 1; }; @@ -845,11 +845,11 @@ static int delay_allowed(request_rec *r, enum tileState state) if (delay > 0) { /* If we are on the second round, we really hit an empty delaypool, timeout for a while to slow down clients */ if (j > 0) { - apr_global_mutex_unlock(delay_mutex); + apr_global_mutex_unlock(delaypool_mutex); ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Delaypool: Client %s has hit its limits, throttling (%i)\n", ip_addr, delay); sleep(CLIENT_PENALTY); - if (get_global_lock(r, delay_mutex) == 0) { + if (get_global_lock(r, delaypool_mutex) == 0) { ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Could not acquire lock, but had to delay\n"); return 0; }; @@ -895,7 +895,7 @@ static int delay_allowed(request_rec *r, enum tileState state) delay = 0; } - apr_global_mutex_unlock(delay_mutex); + apr_global_mutex_unlock(delaypool_mutex); if (delay > 0) { ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Delaypool: Client %s has hit its limits, rejecting (%i)\n", ip_addr, delay); @@ -1789,7 +1789,7 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, mutexfilename = apr_psprintf(pconf, "/tmp/httpd_mutex_delay.%ld", (long int)getpid()); - rs = apr_global_mutex_create(&delay_mutex, (const char *)mutexfilename, + rs = apr_global_mutex_create(&delaypool_mutex, (const char *)mutexfilename, APR_LOCK_DEFAULT, pconf); if (rs != APR_SUCCESS) { @@ -1800,7 +1800,7 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, } #ifdef MOD_TILE_SET_MUTEX_PERMS - rs = ap_unixd_set_global_mutex_perms(delay_mutex); + rs = ap_unixd_set_global_mutex_perms(delaypool_mutex); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_CRIT, rs, s, From 203266f42cd61ebe006055f50c76b92ee048a35c Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:36:12 -0700 Subject: [PATCH 3/4] Use P_tmpdir rather than /tmp --- src/mod_tile.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/mod_tile.c b/src/mod_tile.c index de448be0..d9411011 100644 --- a/src/mod_tile.c +++ b/src/mod_tile.c @@ -1756,8 +1756,7 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, * depending on OS and locking mechanism of choice, the file * may or may not be actually created. */ - mutexfilename = apr_psprintf(pconf, "/tmp/httpd_mutex.%ld", - (long int)getpid()); + mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex.%ld", P_tmpdir, (long int)getpid()); rs = apr_global_mutex_create(&stats_mutex, (const char *)mutexfilename, APR_LOCK_DEFAULT, pconf); @@ -1786,8 +1785,7 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, * depending on OS and locking mechanism of choice, the file * may or may not be actually created. */ - mutexfilename = apr_psprintf(pconf, "/tmp/httpd_mutex_delay.%ld", - (long int)getpid()); + mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex_delay.%ld", P_tmpdir, (long int)getpid()); rs = apr_global_mutex_create(&delaypool_mutex, (const char *)mutexfilename, APR_LOCK_DEFAULT, pconf); From 398ebc27b15026f338f29cfe87ebbc6d2b2cb3e7 Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Wed, 10 Jul 2024 15:37:52 -0700 Subject: [PATCH 4/4] Don't comingle the two mutexes mutexfilenames --- src/mod_tile.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/mod_tile.c b/src/mod_tile.c index d9411011..9234f191 100644 --- a/src/mod_tile.c +++ b/src/mod_tile.c @@ -84,7 +84,8 @@ apr_shm_t *delaypool_shm; apr_global_mutex_t *stats_mutex; apr_global_mutex_t *delaypool_mutex; -char *mutexfilename; +char *stats_mutexfilename; +char *delaypool_mutexfilename; int layerCount = 0; int global_max_zoom = 0; @@ -1756,15 +1757,15 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, * depending on OS and locking mechanism of choice, the file * may or may not be actually created. */ - mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex.%ld", P_tmpdir, (long int)getpid()); + stats_mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex_stats.%ld", P_tmpdir, (long int)getpid()); - rs = apr_global_mutex_create(&stats_mutex, (const char *)mutexfilename, + rs = apr_global_mutex_create(&stats_mutex, (const char *)stats_mutexfilename, APR_LOCK_DEFAULT, pconf); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rs, s, "Failed to create mutex on file %s", - mutexfilename); + stats_mutexfilename); return HTTP_INTERNAL_SERVER_ERROR; } @@ -1785,15 +1786,15 @@ static int mod_tile_post_config(apr_pool_t *pconf, apr_pool_t *plog, * depending on OS and locking mechanism of choice, the file * may or may not be actually created. */ - mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex_delay.%ld", P_tmpdir, (long int)getpid()); + delaypool_mutexfilename = apr_psprintf(pconf, "%s/httpd_mutex_delaypool.%ld", P_tmpdir, (long int)getpid()); - rs = apr_global_mutex_create(&delaypool_mutex, (const char *)mutexfilename, + rs = apr_global_mutex_create(&delaypool_mutex, (const char *)delaypool_mutexfilename, APR_LOCK_DEFAULT, pconf); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rs, s, "Failed to create mutex on file %s", - mutexfilename); + delaypool_mutexfilename); return HTTP_INTERNAL_SERVER_ERROR; } @@ -1830,13 +1831,13 @@ static void mod_tile_child_init(apr_pool_t *p, server_rec *s) * the mutex pointer global here. */ rs = apr_global_mutex_child_init(&stats_mutex, - (const char *)mutexfilename, + (const char *)stats_mutexfilename, p); if (rs != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_CRIT, rs, s, "Failed to reopen mutex on file %s", - mutexfilename); + stats_mutexfilename); /* There's really nothing else we can do here, since * This routine doesn't return a status. */ exit(1); /* Ugly, but what else? */