Skip to content

Commit fa667af

Browse files
zandbeltzandbelt
committed
init: refactor pconf pool memory allocation handling, part 1
should result in less memory allocation over graceful restarts Signed-off-by: Hans Zandbelt <[email protected]>
1 parent a266848 commit fa667af

File tree

19 files changed

+110
-92
lines changed

19 files changed

+110
-92
lines changed

‎ChangeLog‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
11/22/2025
2+
- init: refactor pconf pool memory allocation handling, part 1
3+
should result in less memory allocation over graceful restarts
4+
15
11/19/2025
26
- request: set the OIDC_ERROR variables when PAR is configured but not enabled by the Provider
37

‎src/cache/cache.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@
5959
#define OIDC_CACHE_KEY_SIZE_MAX 512
6060

6161
typedef void *(*oidc_cache_cfg_create)(apr_pool_t *pool);
62-
typedef int (*oidc_cache_post_config_function)(server_rec *s);
62+
typedef int (*oidc_cache_post_config_function)(apr_pool_t *pool, server_rec *s);
6363
typedef int (*oidc_cache_child_init_function)(apr_pool_t *p, server_rec *s);
6464
typedef apr_byte_t (*oidc_cache_get_function)(request_rec *r, const char *section, const char *key, char **value);
6565
typedef apr_byte_t (*oidc_cache_set_function)(request_rec *r, const char *section, const char *key, const char *value,
6666
apr_time_t expiry);
67-
typedef int (*oidc_cache_destroy_function)(server_rec *s);
67+
typedef int (*oidc_cache_destroy_function)(apr_pool_t *pool, server_rec *s);
6868

6969
typedef struct oidc_cache_t {
7070
const char *name;
@@ -86,7 +86,7 @@ typedef struct oidc_cache_mutex_t {
8686

8787
oidc_cache_mutex_t *oidc_cache_mutex_create(apr_pool_t *pool, apr_byte_t global);
8888
char *oidc_cache_status2str(apr_pool_t *p, apr_status_t statcode);
89-
apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type);
89+
apr_byte_t oidc_cache_mutex_post_config(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m, const char *type);
9090
apr_status_t oidc_cache_mutex_child_init(apr_pool_t *p, server_rec *s, oidc_cache_mutex_t *m);
9191
apr_byte_t oidc_cache_mutex_lock(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m);
9292
apr_byte_t oidc_cache_mutex_unlock(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m);

‎src/cache/common.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,21 +80,22 @@ char *oidc_cache_status2str(apr_pool_t *p, apr_status_t statcode) {
8080
return apr_pstrdup(p, buf);
8181
}
8282

83-
static apr_byte_t oidc_cache_mutex_global_create(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
83+
static apr_byte_t oidc_cache_mutex_global_create(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m,
84+
const char *type) {
8485

8586
apr_status_t rv = APR_SUCCESS;
8687
const char *dir;
8788

8889
/* construct the mutex filename */
89-
rv = apr_temp_dir_get(&dir, s->process->pool);
90+
rv = apr_temp_dir_get(&dir, pool);
9091
if (rv != APR_SUCCESS) {
9192
oidc_serror(s, "apr_temp_dir_get failed: could not find a temp dir: %s",
92-
oidc_cache_status2str(s->process->pool, rv));
93+
oidc_cache_status2str(pool, rv));
9394
return FALSE;
9495
}
9596

9697
m->mutex_filename =
97-
apr_psprintf(s->process->pool, "%s/mod_auth_openidc_%s_mutex.%ld.%pp", dir, type, (long int)getpid(), s);
98+
apr_psprintf(pool, "%s/mod_auth_openidc_%s_mutex.%ld.%pp", dir, type, (long int)getpid(), s);
9899

99100
/* set the lock type */
100101
apr_lockmech_e mech =
@@ -108,11 +109,11 @@ static apr_byte_t oidc_cache_mutex_global_create(server_rec *s, oidc_cache_mutex
108109
;
109110

110111
/* create the mutex lock */
111-
rv = apr_global_mutex_create(&m->gmutex, (const char *)m->mutex_filename, mech, s->process->pool);
112+
rv = apr_global_mutex_create(&m->gmutex, (const char *)m->mutex_filename, mech, pool);
112113

113114
if (rv != APR_SUCCESS) {
114115
oidc_serror(s, "apr_global_mutex_create failed to create mutex (%d) on file %s: %s (%d)", mech,
115-
m->mutex_filename, oidc_cache_status2str(s->process->pool, rv), rv);
116+
m->mutex_filename, oidc_cache_status2str(pool, rv), rv);
116117
return FALSE;
117118
}
118119

@@ -121,7 +122,7 @@ static apr_byte_t oidc_cache_mutex_global_create(server_rec *s, oidc_cache_mutex
121122
rv = ap_unixd_set_global_mutex_perms(m->gmutex);
122123
if (rv != APR_SUCCESS) {
123124
oidc_serror(s, "unixd_set_global_mutex_perms failed; could not set permissions: %s (%d)",
124-
oidc_cache_status2str(s->process->pool, rv), rv);
125+
oidc_cache_status2str(pool, rv), rv);
125126
return FALSE;
126127
}
127128
#endif
@@ -131,17 +132,16 @@ static apr_byte_t oidc_cache_mutex_global_create(server_rec *s, oidc_cache_mutex
131132
return TRUE;
132133
}
133134

134-
apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
135+
apr_byte_t oidc_cache_mutex_post_config(apr_pool_t *pool, server_rec *s, oidc_cache_mutex_t *m, const char *type) {
135136

136137
apr_status_t rv = APR_SUCCESS;
137138

138139
if (m->is_global)
139-
return oidc_cache_mutex_global_create(s, m, type);
140+
return oidc_cache_mutex_global_create(s->process->pool, s, m, type);
140141

141142
rv = apr_thread_mutex_create(&m->tmutex, APR_THREAD_MUTEX_DEFAULT, s->process->pool);
142143
if (rv != APR_SUCCESS) {
143-
oidc_serror(s, "apr_thread_mutex_create failed: %s (%d)", oidc_cache_status2str(s->process->pool, rv),
144-
rv);
144+
oidc_serror(s, "apr_thread_mutex_create failed: %s (%d)", oidc_cache_status2str(pool, rv), rv);
145145
return FALSE;
146146
}
147147

@@ -162,7 +162,7 @@ apr_status_t oidc_cache_mutex_child_init(apr_pool_t *p, server_rec *s, oidc_cach
162162
apr_status_t rv = APR_SUCCESS;
163163

164164
if (m->is_global) {
165-
rv = apr_global_mutex_child_init(&m->gmutex, (const char *)m->mutex_filename, p);
165+
rv = apr_global_mutex_child_init(&m->gmutex, (const char *)m->mutex_filename, s->process->pool);
166166
if (rv != APR_SUCCESS) {
167167
oidc_serror(s,
168168
"apr_global_mutex_child_init failed to reopen mutex on "

‎src/cache/file.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ typedef struct {
6363
#define OIDC_CACHE_FILE_PREFIX "mod-auth-openidc-"
6464

6565
/* post config routine */
66-
int oidc_cache_file_post_config(server_rec *s) {
66+
int oidc_cache_file_post_config(apr_pool_t *pool, server_rec *s) {
6767
apr_status_t rv = APR_SUCCESS;
6868
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
6969
if (cfg->cache.file_dir == NULL) {
7070
/* by default we'll use the OS specified /tmp dir for cache files */
71-
rv = apr_temp_dir_get((const char **)&cfg->cache.file_dir, s->process->pool);
71+
rv = apr_temp_dir_get((const char **)&cfg->cache.file_dir, pool);
7272
if (rv != APR_SUCCESS) {
7373
oidc_serror(s, "apr_temp_dir_get failed: could not find a temp dir: %s",
74-
oidc_cache_status2str(s->process->pool, rv));
74+
oidc_cache_status2str(pool, rv));
7575
return HTTP_INTERNAL_SERVER_ERROR;
7676
}
7777
}

‎src/cache/memcache.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,19 @@ static void *oidc_cache_memcache_cfg_create(apr_pool_t *pool) {
6969
/*
7070
* initialize the memcache struct to a number of memcache servers
7171
*/
72-
static int oidc_cache_memcache_post_config(server_rec *s) {
72+
static int oidc_cache_memcache_post_config(apr_pool_t *pool, server_rec *s) {
7373
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
7474

7575
if (cfg->cache.cfg != NULL)
7676
return APR_SUCCESS;
77-
oidc_cache_cfg_memcache_t *context = oidc_cache_memcache_cfg_create(s->process->pool);
77+
oidc_cache_cfg_memcache_t *context = oidc_cache_memcache_cfg_create(pool);
7878
cfg->cache.cfg = context;
7979

8080
apr_status_t rv = APR_SUCCESS;
8181
apr_uint16_t nservers = 0;
8282
char *split;
8383
char *tok;
84-
apr_pool_t *p = s->process->pool;
84+
apr_pool_t *p = pool;
8585
APR_OPTIONAL_FN_TYPE(http2_get_num_workers) * get_h2_num_workers;
8686
int max_threads = 0;
8787
int minw = 0;

‎src/cache/redis.c‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ static oidc_cache_cfg_redis_t *oidc_cache_redis_cfg_create(apr_pool_t *pool) {
7070
return context;
7171
}
7272

73-
int oidc_cache_redis_post_config(server_rec *s, oidc_cfg_t *cfg, const char *name) {
74-
oidc_cache_cfg_redis_t *context = oidc_cache_redis_cfg_create(s->process->pool);
73+
int oidc_cache_redis_post_config(apr_pool_t *pool, server_rec *s, oidc_cfg_t *cfg, const char *name) {
74+
oidc_cache_cfg_redis_t *context = oidc_cache_redis_cfg_create(pool);
7575
cfg->cache.cfg = context;
7676

7777
/* parse the host:post tuple from the configuration */
@@ -82,10 +82,10 @@ int oidc_cache_redis_post_config(server_rec *s, oidc_cfg_t *cfg, const char *nam
8282
}
8383

8484
if (cfg->cache.redis_username != NULL) {
85-
context->username = apr_pstrdup(s->process->pool, cfg->cache.redis_username);
85+
context->username = apr_pstrdup(pool, cfg->cache.redis_username);
8686
}
8787
if (cfg->cache.redis_password != NULL) {
88-
context->passwd = apr_pstrdup(s->process->pool, cfg->cache.redis_password);
88+
context->passwd = apr_pstrdup(pool, cfg->cache.redis_password);
8989
}
9090

9191
if (oidc_cfg_cache_redis_database_get(cfg) != OIDC_CONFIG_POS_INT_UNSET)
@@ -100,7 +100,7 @@ int oidc_cache_redis_post_config(server_rec *s, oidc_cfg_t *cfg, const char *nam
100100
if (oidc_cfg_cache_redis_timeout_get(cfg) != OIDC_CONFIG_POS_INT_UNSET)
101101
context->timeout.tv_sec = oidc_cfg_cache_redis_timeout_get(cfg);
102102

103-
if (oidc_cache_mutex_post_config(s, context->mutex, name) == FALSE)
103+
if (oidc_cache_mutex_post_config(pool, s, context->mutex, name) == FALSE)
104104
return HTTP_INTERNAL_SERVER_ERROR;
105105

106106
return OK;
@@ -122,15 +122,15 @@ apr_status_t oidc_cache_redis_disconnect(oidc_cache_cfg_redis_t *context) {
122122
/*
123123
* initialize the Redis struct the specified Redis server
124124
*/
125-
static int oidc_cache_redis_post_config_impl(server_rec *s) {
125+
static int oidc_cache_redis_post_config_impl(apr_pool_t *pool, server_rec *s) {
126126
apr_status_t rv = APR_SUCCESS;
127127
oidc_cache_cfg_redis_t *context = NULL;
128128
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
129129

130130
if (cfg->cache.cfg != NULL)
131131
return OK;
132132

133-
if (oidc_cache_redis_post_config(s, cfg, "redis") != OK)
133+
if (oidc_cache_redis_post_config(pool, s, cfg, "redis") != OK)
134134
return HTTP_INTERNAL_SERVER_ERROR;
135135

136136
context = (oidc_cache_cfg_redis_t *)cfg->cache.cfg;
@@ -143,8 +143,7 @@ static int oidc_cache_redis_post_config_impl(server_rec *s) {
143143
}
144144

145145
char *scope_id;
146-
rv = apr_parse_addr_port(&context->host_str, &scope_id, &context->port, cfg->cache.redis_server,
147-
s->process->pool);
146+
rv = apr_parse_addr_port(&context->host_str, &scope_id, &context->port, cfg->cache.redis_server, pool);
148147
if (rv != APR_SUCCESS) {
149148
oidc_serror(s, "failed to parse cache server: '%s'", cfg->cache.redis_server);
150149
return HTTP_INTERNAL_SERVER_ERROR;
@@ -524,14 +523,14 @@ apr_byte_t oidc_cache_redis_set(request_rec *r, const char *section, const char
524523
return rv;
525524
}
526525

527-
static int oidc_cache_redis_destroy_impl(server_rec *s) {
526+
static int oidc_cache_redis_destroy_impl(apr_pool_t *pool, server_rec *s) {
528527
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
529528
oidc_cache_cfg_redis_t *context = (oidc_cache_cfg_redis_t *)cfg->cache.cfg;
530529

531530
if (context != NULL) {
532-
oidc_cache_mutex_lock(s->process->pool, s, context->mutex);
531+
oidc_cache_mutex_lock(pool, s, context->mutex);
533532
context->disconnect(context);
534-
oidc_cache_mutex_unlock(s->process->pool, s, context->mutex);
533+
oidc_cache_mutex_unlock(pool, s, context->mutex);
535534
if (oidc_cache_mutex_destroy(s, context->mutex) != TRUE) {
536535
oidc_serror(s, "oidc_cache_mutex_destroy on refresh mutex failed");
537536
}

‎src/cache/redis.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ typedef struct oidc_cache_cfg_redis_t {
7272
oidc_cache_redis_disconnect_function_t disconnect;
7373
} oidc_cache_cfg_redis_t;
7474

75-
int oidc_cache_redis_post_config(server_rec *s, oidc_cfg_t *cfg, const char *name);
75+
int oidc_cache_redis_post_config(apr_pool_t *pool, server_rec *s, oidc_cfg_t *cfg, const char *name);
7676
int oidc_cache_redis_child_init(apr_pool_t *p, server_rec *s);
7777
redisReply *oidc_cache_redis_command(request_rec *r, oidc_cache_cfg_redis_t *context, char **errstr, const char *format,
7878
va_list ap);

‎src/cache/shm.c‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,18 +85,17 @@ static void *oidc_cache_shm_cfg_create(apr_pool_t *pool) {
8585
/*
8686
* initialized the shared memory block in the parent process
8787
*/
88-
int oidc_cache_shm_post_config(server_rec *s) {
88+
static int oidc_cache_shm_post_config(apr_pool_t *pool, server_rec *s) {
8989
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
9090

9191
if (cfg->cache.cfg != NULL)
9292
return OK;
93-
oidc_cache_cfg_shm_t *context = oidc_cache_shm_cfg_create(s->process->pconf);
93+
oidc_cache_cfg_shm_t *context = oidc_cache_shm_cfg_create(pool);
9494
cfg->cache.cfg = context;
9595

9696
/* create the shared memory segment */
97-
apr_status_t rv =
98-
apr_shm_create(&context->shm, (apr_size_t)cfg->cache.shm_entry_size_max * cfg->cache.shm_size_max, NULL,
99-
s->process->pconf);
97+
apr_status_t rv = apr_shm_create(
98+
&context->shm, (apr_size_t)cfg->cache.shm_entry_size_max * cfg->cache.shm_size_max, NULL, s->process->pool);
10099
if (rv != APR_SUCCESS) {
101100
oidc_serror(s, "apr_shm_create failed to create shared memory segment");
102101
return HTTP_INTERNAL_SERVER_ERROR;
@@ -109,7 +108,7 @@ int oidc_cache_shm_post_config(server_rec *s) {
109108
t->access = 0;
110109
}
111110

112-
if (oidc_cache_mutex_post_config(s, context->mutex, "shm") == FALSE)
111+
if (oidc_cache_mutex_post_config(pool, s, context->mutex, "shm") == FALSE)
113112
return HTTP_INTERNAL_SERVER_ERROR;
114113

115114
oidc_sdebug(
@@ -125,7 +124,7 @@ int oidc_cache_shm_post_config(server_rec *s) {
125124
/*
126125
* initialize the shared memory segment in a child process
127126
*/
128-
int oidc_cache_shm_child_init(apr_pool_t *p, server_rec *s) {
127+
static int oidc_cache_shm_child_init(apr_pool_t *p, server_rec *s) {
129128
oidc_cfg_t *cfg = ap_get_module_config(s->module_config, &auth_openidc_module);
130129
oidc_cache_cfg_shm_t *context = (oidc_cache_cfg_shm_t *)cfg->cache.cfg;
131130

@@ -319,7 +318,7 @@ static apr_byte_t oidc_cache_shm_set(request_rec *r, const char *section, const
319318
return TRUE;
320319
}
321320

322-
static int oidc_cache_shm_destroy(server_rec *s) {
321+
static int oidc_cache_shm_destroy(apr_pool_t *pool, server_rec *s) {
323322
oidc_cfg_t *cfg = (oidc_cfg_t *)ap_get_module_config(s->module_config, &auth_openidc_module);
324323
oidc_cache_cfg_shm_t *context = (oidc_cache_cfg_shm_t *)cfg->cache.cfg;
325324
apr_status_t rv = APR_SUCCESS;
@@ -328,11 +327,11 @@ static int oidc_cache_shm_destroy(server_rec *s) {
328327
context ? context->is_parent : -1);
329328

330329
if (context && (context->is_parent == TRUE) && (context->shm) && (context->mutex)) {
331-
oidc_cache_mutex_lock(s->process->pool, s, context->mutex);
330+
oidc_cache_mutex_lock(pool, s, context->mutex);
332331
rv = apr_shm_destroy(context->shm);
333332
oidc_sdebug(s, "apr_shm_destroy returned: %d", rv);
334333
context->shm = NULL;
335-
oidc_cache_mutex_unlock(s->process->pool, s, context->mutex);
334+
oidc_cache_mutex_unlock(pool, s, context->mutex);
336335
}
337336

338337
if (context && (context->mutex)) {

0 commit comments

Comments
 (0)