code: refactor/rename cleanup handling

Signed-off-by: Hans Zandbelt <[email protected]>

Author: zandbelt

src/cache/common.c

@@ -178,20 +178,25 @@ static apr_byte_t oidc_cache_mutex_global_create(apr_pool_t *pool, server_rec *s
  * initialize a server- or process-wide mutex
  */
 apr_byte_t oidc_cache_mutex_post_config(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m, const char *type) {
-
+	apr_byte_t rc = TRUE;
 	apr_status_t rv = APR_SUCCESS;
 
-	if (m->is_global)
-		return oidc_cache_mutex_global_create(pool, s, m, type);
+	if (m->is_global) {
+		rc = oidc_cache_mutex_global_create(pool, s, m, type);
+		goto end;
+	}
 
 	// NB: see note above at apr_global_mutex_create on the use of s->process->pool
 	rv = apr_thread_mutex_create(&m->tmutex, APR_THREAD_MUTEX_DEFAULT, s->process->pool);
 	if (rv != APR_SUCCESS) {
 		oidc_serror(s, "apr_thread_mutex_create failed: %s (%d)", oidc_cache_status2str(pool, rv), rv);
-		return FALSE;
+		rc = FALSE;
+		goto end;
 	}
 
-	return TRUE;
+end:
+
+	return rc;
 }
 
 /*

src/cfg/cfg.c

@@ -693,14 +693,10 @@ typedef struct oidc_cfg_cleanup_ctx_t {
 /*
  * destroy a server config record and its members
  */
-static apr_status_t oidc_cfg_server_destroy(void *data) {
-	oidc_cfg_cleanup_ctx_t *ctx = (oidc_cfg_cleanup_ctx_t *)data;
-	oidc_cfg_t *cfg = ctx->cfg;
-	if (cfg->cache.impl && cfg->cache.impl->destroy) {
-		cfg->cache.impl->destroy(ctx->pool, ctx->svr);
-		cfg->cache.impl->destroy = NULL;
-		cfg->cache.impl = NULL;
-	}
+apr_byte_t oidc_cfg_server_destroy(apr_pool_t *pool, server_rec *s, oidc_cfg_t *cfg) {
+	if ((cfg->cache.impl) && (cfg->cache.impl->destroy))
+		cfg->cache.impl->destroy(pool, s);
+	cfg->cache.impl = NULL;
 	oidc_cfg_provider_destroy(cfg->provider);
 	cfg->provider = NULL;
 	oidc_cfg_oauth_destroy(cfg->oauth);
@@ -709,18 +705,25 @@ static apr_status_t oidc_cfg_server_destroy(void *data) {
 	cfg->public_keys = NULL;
 	oidc_jwk_list_destroy(cfg->private_keys);
 	cfg->private_keys = NULL;
-	return APR_SUCCESS;
+	return TRUE;
+}
+
+static apr_status_t oidc_cfg_server_cleanup(void *data) {
+	oidc_cfg_cleanup_ctx_t *ctx = (oidc_cfg_cleanup_ctx_t *)data;
+	oidc_cfg_t *cfg = ctx->cfg;
+	return oidc_cfg_server_destroy(ctx->pool, ctx->svr, cfg) ? APR_SUCCESS : APR_EGENERAL;
 }
 
-static oidc_cfg_t *oidc_cfg_server_alloc(apr_pool_t *pool, server_rec *svr) {
+static oidc_cfg_t *oidc_cfg_server_alloc(apr_pool_t *pool, server_rec *s) {
 	oidc_cfg_t *c = apr_pcalloc(pool, sizeof(oidc_cfg_t));
 	oidc_cfg_cleanup_ctx_t *ctx = apr_pcalloc(pool, sizeof(oidc_cfg_cleanup_ctx_t));
 	ctx->cfg = c;
-	// pool used at descrution time
+	// pconf  pool used at destruction time
 	ctx->pool = pool;
-	ctx->svr = svr;
-	// need to register a cleanup handler to the config pool to handle graceful restarts without memory leaks
-	apr_pool_cleanup_register(pool, ctx, oidc_cfg_server_destroy, oidc_cfg_server_destroy);
+	ctx->svr = s;
+	// need to register a cleanup handler to the config pool to handle graceful restarts without memory increasing
+	// memory consumption
+	apr_pool_cleanup_register(pool, ctx, oidc_cfg_server_cleanup, apr_pool_cleanup_null);
 	return c;
 }
 
@@ -1040,7 +1043,7 @@ void oidc_cfg_child_init(apr_pool_t *pool, oidc_cfg_t *cfg, server_rec *s) {
 	}
 }
 
-void oidc_cfg_cleanup_child(oidc_cfg_t *cfg, server_rec *s) {
+void oidc_cfg_process_cleanup(oidc_cfg_t *cfg, server_rec *s) {
 	if (_oidc_refresh_mutex != NULL) {
 		if (oidc_cache_mutex_destroy(s, _oidc_refresh_mutex) != TRUE) {
 			oidc_serror(s, "oidc_cache_mutex_destroy on refresh mutex failed");

src/cfg/cfg.h

@@ -188,11 +188,12 @@ int oidc_cfg_merged_get(oidc_cfg_t *cfg);
 
 void oidc_pre_config_init();
 
-void *oidc_cfg_server_create(apr_pool_t *pool, server_rec *svr);
+void *oidc_cfg_server_create(apr_pool_t *pool, server_rec *s);
 void *oidc_cfg_server_merge(apr_pool_t *pool, void *BASE, void *ADD);
+apr_byte_t oidc_cfg_server_destroy(apr_pool_t *pool, server_rec *s, oidc_cfg_t *cfg);
 int oidc_cfg_post_config(apr_pool_t *pool, oidc_cfg_t *cfg, server_rec *s);
 void oidc_cfg_child_init(apr_pool_t *pool, oidc_cfg_t *cfg, server_rec *s);
-void oidc_cfg_cleanup_child(oidc_cfg_t *cfg, server_rec *s);
+void oidc_cfg_process_cleanup(oidc_cfg_t *cfg, server_rec *s);
 const char *oidc_cfg_string_list_add(apr_pool_t *pool, apr_array_header_t **list, const char *arg);
 const char *oidc_cfg_endpoint_auth_set(apr_pool_t *pool, oidc_cfg_t *cfg, const char *arg, char **auth, char **alg);
 

src/mod_auth_openidc.c

@@ -1648,26 +1648,17 @@ static void oidc_ssl_id_callback(CRYPTO_THREADID *id) {
 #endif /* defined(OPENSSL_THREADS) && APR_HAS_THREADS */
 
 /*
- * cleanup resources allocated in a child process
+ * cleanup resources allocated in a process
  */
-static apr_status_t oidc_cleanup_child(void *data) {
+static apr_status_t oidc_process_cleanup(void *data) {
+
 	server_rec *sp = (server_rec *)data;
 	while (sp != NULL) {
 		oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(sp->module_config, &auth_openidc_module);
-		oidc_cfg_cleanup_child(cfg, sp);
+		oidc_cfg_process_cleanup(cfg, sp);
 		sp = sp->next;
 	}
 
-	return APR_SUCCESS;
-}
-
-/*
- * cleanup resources allocated in a parent process
- */
-static apr_status_t oidc_cleanup_parent(void *data) {
-
-	oidc_cleanup_child(data);
-
 #if ((OPENSSL_VERSION_NUMBER < 0x10100000) && defined(OPENSSL_THREADS) && APR_HAS_THREADS)
 	if (CRYPTO_get_locking_callback() == oidc_ssl_locking_callback)
 		CRYPTO_set_locking_callback(NULL);
@@ -1754,7 +1745,7 @@ static int oidc_post_config(apr_pool_t *pool, apr_pool_t *p1, apr_pool_t *p2, se
 
 #endif /* (OPENSSL_VERSION_NUMBER < 0x10100000) && defined (OPENSSL_THREADS) && APR_HAS_THREADS */
 
-	apr_pool_cleanup_register(pool, s, oidc_cleanup_parent, apr_pool_cleanup_null);
+	apr_pool_cleanup_register(pool, s, oidc_process_cleanup, apr_pool_cleanup_null);
 
 	server_rec *sp = s;
 	while (sp != NULL) {
@@ -1822,11 +1813,6 @@ static void oidc_child_init(apr_pool_t *p, server_rec *s) {
 		oidc_cfg_child_init(p, cfg, sp);
 		sp = sp->next;
 	}
-	/*
-	 * NB: don't pass oidc_cleanup_child as the child cleanup routine parameter
-	 *     because that does not actually get called upon child cleanup...
-	 */
-	apr_pool_cleanup_register(p, s, oidc_cleanup_child, apr_pool_cleanup_null);
 }
 
 static const char oidcFilterName[] = "oidc_filter_in_filter";