From: Yann Ylavic <ylavic@apache.org>
Date: Wed, 12 May 2021 11:21:09 +0000 (+0000)
Subject: Merge r1888266 from trunk:
X-Git-Tag: 2.4.48~15
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0e382d189d5136e95878ac7559f47b015708ecdb;p=thirdparty%2Fapache%2Fhttpd.git

Merge r1888266 from trunk:

mod_socache_shmcb: be safe from socache_shmcb_destroy() late call.  PR 59798.

ssl_init_Module() in post_config early registers ssl_init_ModuleKill(), which
will then run after all the next cleanups registered later in post_config, thus
any shm_cleanup() registered from ssl_scache_init::socache_shmcb_init().
This can cause a double SHM cleanup when apr_shm_destroy() is called from
ssl_init_ModuleKill::ssl_scache_kill() as pconf is cleared.

Fix this in mod_socache_shmcb by registering a socache_shmcb_cleanup() after
the SHM is created, and by letting socache_shmcb_destroy() run the cleanup,
such that shm_cleanup() is always and ever called only once.

Ideally apr_shm_create() would be consistent accross platforms to register its
shm_cleanup() on the pool but that's not the case for now (I'm on it), so httpd
has to call apr_shm_destroy() explicitely from several places (we'll be able to
remove ssl_scache_kill() and other similar cleanups once the minimal APR
version required by httpd is fixed..).

We could also fix this by registering ssl_init_ModuleKill() late(r) in
ssl_init_Module(), though the more robust mod_socache_shmcb the better for
all the modules..


Reviewed by: ylavic, icing, rpluem


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1889798 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/modules/cache/mod_socache_shmcb.c b/modules/cache/mod_socache_shmcb.c
index 9bbe8a8922a..4727961ff45 100644
--- a/modules/cache/mod_socache_shmcb.c
+++ b/modules/cache/mod_socache_shmcb.c
@@ -105,6 +105,7 @@ typedef struct {
 } SHMCBIndex;
 
 struct ap_socache_instance_t {
+    apr_pool_t *pool;
     const char *data_file;
     apr_size_t shm_size;
     apr_shm_t *shm;
@@ -295,6 +296,7 @@ static const char *socache_shmcb_create(ap_socache_instance_t **context,
 
     /* Allocate the context. */
     *context = ctx = apr_pcalloc(p, sizeof *ctx);
+    ctx->pool = p;
 
     ctx->shm_size  = 1024*512; /* 512KB */
 
@@ -340,6 +342,16 @@ static const char *socache_shmcb_create(ap_socache_instance_t **context,
     return NULL;
 }
 
+static apr_status_t socache_shmcb_cleanup(void *arg)
+{
+    ap_socache_instance_t *ctx = arg;
+    if (ctx->shm) {
+        apr_shm_destroy(ctx->shm);
+        ctx->shm = NULL;
+    }
+    return APR_SUCCESS;
+}
+
 static apr_status_t socache_shmcb_init(ap_socache_instance_t *ctx,
                                        const char *namespace,
                                        const struct ap_socache_hints *hints,
@@ -370,6 +382,7 @@ static apr_status_t socache_shmcb_init(ap_socache_instance_t *ctx,
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(00818)
                          "Could not use anonymous shm for '%s' cache",
                          namespace);
+            ctx->shm = NULL;
             return APR_EINVAL;
         }
 
@@ -384,8 +397,11 @@ static apr_status_t socache_shmcb_init(ap_socache_instance_t *ctx,
         ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO(00819)
                      "Could not allocate shared memory segment for shmcb "
                      "socache");
+        ctx->shm = NULL;
         return rv;
     }
+    apr_pool_cleanup_register(ctx->pool, ctx, socache_shmcb_cleanup,
+                              apr_pool_cleanup_null); 
 
     shm_segment = apr_shm_baseaddr_get(ctx->shm);
     shm_segsize = apr_shm_size_get(ctx->shm);
@@ -473,9 +489,8 @@ static apr_status_t socache_shmcb_init(ap_socache_instance_t *ctx,
 
 static void socache_shmcb_destroy(ap_socache_instance_t *ctx, server_rec *s)
 {
-    if (ctx && ctx->shm) {
-        apr_shm_destroy(ctx->shm);
-        ctx->shm = NULL;
+    if (ctx) {
+        apr_pool_cleanup_run(ctx->pool, ctx, socache_shmcb_cleanup); 
     }
 }