svn commit: r1888266 - /httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c

[yl...@apache.org]

Author: ylavic
Date: Thu Apr  1 09:14:34 2021
New Revision: 1888266

URL: http://svn.apache.org/viewvc?rev=1888266&view=rev
Log:
mod_socache_shmcb: be safe from socache_shmcb_destroy() late call.

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() 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..

Modified:
    httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c

Modified: httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c?rev=1888266&r1=1888265&r2=1888266&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c Thu Apr  1 09:14:34 2021
@@ -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(
 
     /* 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(
     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(a
             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(a
         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(a
 
 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); 
     }
 }