Make the configuration store use a serf allocator instead of pools for its
data to allow clearing out values without a (very slow) memory leak.
* config_store.c
(serf__config_hdr_t): Remove pool.
(config_free_cb_t): New typedef.
(config_entry_t): Add free callback.
(create_config_hdr): Use allocator.
(add_or_replace_entry): Allow storing how to free value.
(add_or_replace_entry): Handle free support.
(config_set_object): New helper function, extracted from
serf__config_set_object.
(cleanup_hdr,
cleanup_store): New function.
(serf__config_store_init): Create allocator. Hook explicit cleanup.
(serf__config_store_create_ctx_config,
serf__config_store_create_conn_config,
serf__config_store_create_client_config,
serf__config_store_create_listener_config): Remove out_pool, which is only
partially used as result pool. Use the context pool directly.
(serf_config_set_stringc): Use config_set_object and declare how to free.
(serf_config_set_stringf): Add scratch_pool argument. Update caller.
(serf_config_set_object): Move some code to config_set_object.
* context.c
(serf_context_create_ex): Update caller.
* incoming.c
(serf_incoming_create2,
serf_listener_create): Update caller.
* outgoing.c
(serf_connection_create2): Update caller.
* pump.c
(serf_pump__store_ipaddresses_in_config): Update caller.
* serf.h
(serf_config_set_stringf): Add argument.
* serf_private.h
(serf_config_t): Store allocator instead of conn_pool.
(serf__config_store_t): Store allocator. Update comment.
(serf__config_store_create_conn_config,
serf__config_store_create_client_config,
serf__config_store_create_listener_config,
serf__config_store_create_ctx_config): Remove argument.
git-svn-id: https://svn.apache.org/repos/asf/serf/trunk@1717012 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/config_store.c b/config_store.c
index 801e112..48f03ad 100644
--- a/config_store.c
+++ b/config_store.c
@@ -27,25 +27,28 @@
/* Use a linked list to store the config values, as we'll only store a couple
of values per context. */
struct serf__config_hdr_t {
- apr_pool_t *pool;
struct config_entry_t *first;
};
+typedef void (*config_free_cb_t)(serf_bucket_alloc_t *alloc, void *value);
+
typedef struct config_entry_t {
apr_uint32_t key;
void *value;
struct config_entry_t *next;
+ config_free_cb_t free_cb;
} config_entry_t;
-static serf__config_hdr_t *create_config_hdr(apr_pool_t *pool)
+static serf__config_hdr_t *create_config_hdr(serf_bucket_alloc_t *allocator)
{
- serf__config_hdr_t *hdr = apr_pcalloc(pool, sizeof(serf__config_hdr_t));
- hdr->pool = pool;
+ serf__config_hdr_t *hdr = serf_bucket_mem_calloc(allocator, sizeof(*hdr));
return hdr;
}
static apr_status_t
-add_or_replace_entry(serf__config_hdr_t *hdr, serf_config_key_t key, void *value)
+add_or_replace_entry(serf_config_t *config,
+ serf__config_hdr_t *hdr, serf_config_key_t key, void *value,
+ config_free_cb_t free_cb)
{
config_entry_t *iter = hdr->first;
config_entry_t *last = iter;
@@ -63,12 +66,18 @@
if (found) {
iter->key = key;
+ if (iter->free_cb)
+ iter->free_cb(config->allocator, iter->value);
iter->value = value;
+ iter->free_cb = free_cb;
} else {
/* Not found, create a new entry and append it to the list. */
- config_entry_t *entry = apr_palloc(hdr->pool, sizeof(config_entry_t));
+ config_entry_t *entry = serf_bucket_mem_alloc(config->allocator,
+ sizeof(*entry));
+
entry->key = key;
entry->value = value;
+ entry->free_cb = free_cb;
entry->next = NULL;
if (last)
@@ -80,16 +89,103 @@
return APR_SUCCESS;
}
+static apr_status_t config_set_object(serf_config_t *config,
+ serf_config_key_t key,
+ void *value,
+ config_free_cb_t free_cb)
+{
+ serf__config_hdr_t *target;
+
+ /* Set the value in the hash table of the selected category */
+ if (key & SERF_CONFIG_PER_CONTEXT) {
+ target = config->per_context;
+ }
+ else if (key & SERF_CONFIG_PER_HOST) {
+ target = config->per_host;
+ }
+ else {
+ target = config->per_conn;
+ }
+
+ if (!target) {
+ /* Config object doesn't manage keys in this category */
+ return APR_EINVAL;
+ }
+
+ return add_or_replace_entry(config, target, key, value, free_cb);
+}
+
+static void cleanup_hdr(serf__config_store_t *store, serf__config_hdr_t *hdr)
+{
+ config_entry_t *e = hdr->first;
+
+ serf_bucket_mem_free(store->allocator, hdr);
+
+ while (e) {
+ config_entry_t *next = e->next;
+
+ if (e->free_cb)
+ e->free_cb(store->allocator, e->value);
+
+ serf_bucket_mem_free(store->allocator, e);
+ e = next;
+ }
+}
+
+static apr_status_t cleanup_store(void *baton)
+{
+ serf__config_store_t *store = baton;
+ apr_hash_index_t *hi;
+
+ for (hi = apr_hash_first(store->pool, store->global_per_host);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *key;
+ void *val;
+
+ apr_hash_this(hi, &key, NULL, &val);
+
+ serf_bucket_mem_free(store->allocator, (void *)key);
+
+ cleanup_hdr(store, val);
+ }
+
+ for (hi = apr_hash_first(store->pool, store->global_per_conn);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *key;
+ void *val;
+
+ apr_hash_this(hi, &key, NULL, &val);
+
+ serf_bucket_mem_free(store->allocator, (void *)key);
+
+ cleanup_hdr(store, val);
+ }
+
+ cleanup_hdr(store, store->global_per_context);
+
+ return APR_SUCCESS;
+}
+
/*** Config Store ***/
apr_status_t serf__config_store_init(serf_context_t *ctx)
{
apr_pool_t *pool = ctx->pool;
+ serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(pool,
+ NULL, NULL);
ctx->config_store.pool = pool;
- ctx->config_store.global_per_context = create_config_hdr(pool);
+ ctx->config_store.allocator = alloc;
+ ctx->config_store.global_per_context = create_config_hdr(alloc);
ctx->config_store.global_per_host = apr_hash_make(pool);
ctx->config_store.global_per_conn = apr_hash_make(pool);
+ apr_pool_cleanup_register(pool, &ctx->config_store, cleanup_store,
+ apr_pool_cleanup_null);
+
return APR_SUCCESS;
}
@@ -125,15 +221,14 @@
return apr_psprintf(pool, "%pp", listener);
}
-
apr_status_t serf__config_store_create_ctx_config(serf_context_t *ctx,
- serf_config_t **config,
- apr_pool_t *out_pool)
+ serf_config_t **config)
{
serf__config_store_t *config_store = &ctx->config_store;
- serf_config_t *cfg = apr_pcalloc(out_pool, sizeof(serf_config_t));
- cfg->ctx_pool = ctx->pool;
+ serf_config_t *cfg = apr_pcalloc(ctx->pool, sizeof(serf_config_t));
+ cfg->ctx_pool = config_store->pool;
+ cfg->allocator = config_store->allocator;
cfg->per_context = config_store->global_per_context;
*config = cfg;
@@ -141,8 +236,7 @@
}
apr_status_t serf__config_store_create_conn_config(serf_connection_t *conn,
- serf_config_t **config,
- apr_pool_t *out_pool)
+ serf_config_t **config)
{
serf__config_store_t *config_store = &conn->ctx->config_store;
const char *host_key, *conn_key;
@@ -150,13 +244,12 @@
apr_pool_t *tmp_pool;
apr_status_t status;
- serf_config_t *cfg = apr_pcalloc(out_pool, sizeof(serf_config_t));
- cfg->ctx_pool = conn->ctx->pool;
+ serf_config_t *cfg = apr_pcalloc(conn->pool, sizeof(serf_config_t));
+ cfg->ctx_pool = config_store->pool;
+ cfg->allocator = config_store->allocator;
cfg->per_context = config_store->global_per_context;
- cfg->conn_pool = conn->pool;
-
- if ((status = apr_pool_create(&tmp_pool, out_pool)) != APR_SUCCESS)
+ if ((status = apr_pool_create(&tmp_pool, cfg->ctx_pool)) != APR_SUCCESS)
return status;
/* Find the config values for this connection, create empty structure
@@ -165,10 +258,10 @@
per_conn = apr_hash_get(config_store->global_per_conn, conn_key,
APR_HASH_KEY_STRING);
if (!per_conn) {
- per_conn = create_config_hdr(conn->pool);
+ per_conn = create_config_hdr(cfg->allocator);
apr_hash_set(config_store->global_per_conn,
- apr_pstrdup(conn->pool, conn_key),
- APR_HASH_KEY_STRING, per_conn);
+ serf_bstrdup(cfg->allocator, conn_key),
+ APR_HASH_KEY_STRING, per_conn);
}
cfg->per_conn = per_conn;
@@ -179,10 +272,10 @@
host_key,
APR_HASH_KEY_STRING);
if (!per_host) {
- per_host = create_config_hdr(config_store->pool);
+ per_host = create_config_hdr(cfg->allocator);
apr_hash_set(config_store->global_per_host,
- apr_pstrdup(config_store->pool, host_key),
- APR_HASH_KEY_STRING, per_host);
+ serf_bstrdup(cfg->allocator, host_key),
+ APR_HASH_KEY_STRING, per_host);
}
cfg->per_host = per_host;
@@ -194,8 +287,7 @@
}
apr_status_t serf__config_store_create_client_config(serf_incoming_t *client,
- serf_config_t **config,
- apr_pool_t *out_pool)
+ serf_config_t **config)
{
serf__config_store_t *config_store = &client->ctx->config_store;
const char *client_key;
@@ -203,14 +295,12 @@
apr_pool_t *tmp_pool;
apr_status_t status;
- serf_config_t *cfg = apr_pcalloc(out_pool, sizeof(serf_config_t));
- cfg->ctx_pool = client->ctx->pool;
+ serf_config_t *cfg = apr_pcalloc(client->pool, sizeof(serf_config_t));
+ cfg->ctx_pool = config_store->pool;
+ cfg->allocator = config_store->allocator;
cfg->per_context = config_store->global_per_context;
-
- cfg->conn_pool = client->pool;
-
- if ((status = apr_pool_create(&tmp_pool, out_pool)) != APR_SUCCESS)
+ if ((status = apr_pool_create(&tmp_pool, client->pool)) != APR_SUCCESS)
return status;
/* Find the config values for this connection, create empty structure
@@ -219,10 +309,10 @@
per_conn = apr_hash_get(config_store->global_per_conn, client_key,
APR_HASH_KEY_STRING);
if (!per_conn) {
- per_conn = create_config_hdr(client->pool);
+ per_conn = create_config_hdr(cfg->allocator);
apr_hash_set(config_store->global_per_conn,
- apr_pstrdup(client->pool, client_key),
- APR_HASH_KEY_STRING, per_conn);
+ serf_bstrdup(cfg->allocator, client_key),
+ APR_HASH_KEY_STRING, per_conn);
}
cfg->per_conn = per_conn;
cfg->per_host = NULL;
@@ -235,8 +325,7 @@
}
apr_status_t serf__config_store_create_listener_config(serf_listener_t *listener,
- serf_config_t **config,
- apr_pool_t *out_pool)
+ serf_config_t **config)
{
serf__config_store_t *config_store = &listener->ctx->config_store;
const char *client_key;
@@ -244,13 +333,12 @@
apr_pool_t *tmp_pool;
apr_status_t status;
- serf_config_t *cfg = apr_pcalloc(out_pool, sizeof(serf_config_t));
- cfg->ctx_pool = listener->ctx->pool;
+ serf_config_t *cfg = apr_pcalloc(listener->pool, sizeof(serf_config_t));
+ cfg->ctx_pool = config_store->pool;
+ cfg->allocator = config_store->allocator;
cfg->per_context = config_store->global_per_context;
- cfg->conn_pool = listener->pool;
-
- if ((status = apr_pool_create(&tmp_pool, out_pool)) != APR_SUCCESS)
+ if ((status = apr_pool_create(&tmp_pool, listener->pool)) != APR_SUCCESS)
return status;
/* Find the config values for this connection, create empty structure
@@ -259,9 +347,9 @@
per_conn = apr_hash_get(config_store->global_per_conn, client_key,
APR_HASH_KEY_STRING);
if (!per_conn) {
- per_conn = create_config_hdr(listener->pool);
+ per_conn = create_config_hdr(cfg->allocator);
apr_hash_set(config_store->global_per_conn,
- apr_pstrdup(listener->pool, client_key),
+ serf_bstrdup(cfg->allocator, client_key),
APR_HASH_KEY_STRING, per_conn);
}
cfg->per_conn = per_conn;
@@ -310,66 +398,34 @@
serf_config_key_t key,
const char *value)
{
- const char *cvalue;
- apr_pool_t *pool;
+ char *cvalue;
- if (key & SERF_CONFIG_PER_CONTEXT ||
- key & SERF_CONFIG_PER_HOST) {
- pool = config->ctx_pool;
- } else {
- pool = config->conn_pool;
- }
+ cvalue = serf_bstrdup(config->allocator, value);
- cvalue = apr_pstrdup(pool, value);
-
- return serf_config_set_string(config, key, cvalue);
+ return config_set_object(config, key, cvalue,
+ serf_bucket_mem_free);
}
apr_status_t serf_config_set_stringf(serf_config_t *config,
serf_config_key_t key,
+ apr_pool_t *scratch_pool,
const char *fmt, ...)
{
- apr_pool_t *pool;
va_list argp;
char *cvalue;
- if (key & SERF_CONFIG_PER_CONTEXT)
- pool = config->ctx_pool;
- else if (key & SERF_CONFIG_PER_HOST)
- pool = config->ctx_pool;
- else
- pool = config->conn_pool;
-
va_start(argp, fmt);
- cvalue = apr_pvsprintf(pool, fmt, argp);
+ cvalue = apr_pvsprintf(scratch_pool, fmt, argp);
va_end(argp);
- return serf_config_set_string(config, key, cvalue);
+ return serf_config_set_stringc(config, key, cvalue);
}
apr_status_t serf_config_set_object(serf_config_t *config,
serf_config_key_t key,
void *value)
{
- serf__config_hdr_t *target;
-
- /* Set the value in the hash table of the selected category */
- if (key & SERF_CONFIG_PER_CONTEXT) {
- target = config->per_context;
- }
- else if (key & SERF_CONFIG_PER_HOST) {
- target = config->per_host;
- }
- else {
- target = config->per_conn;
- }
-
- if (!target) {
- /* Config object doesn't manage keys in this category */
- return APR_EINVAL;
- }
-
- return add_or_replace_entry(target, key, value);
+ return config_set_object(config, key, value, NULL);
}
apr_status_t serf_config_get_string(serf_config_t *config,
diff --git a/context.c b/context.c
index 86c256b..f471fed 100644
--- a/context.c
+++ b/context.c
@@ -195,7 +195,7 @@
/* Assume returned status is APR_SUCCESS */
serf__config_store_init(ctx);
- serf__config_store_create_ctx_config(ctx, &ctx->config, ctx->pool);
+ serf__config_store_create_ctx_config(ctx, &ctx->config);
serf__log_init(ctx);
diff --git a/incoming.c b/incoming.c
index 3323629..36679bd 100644
--- a/incoming.c
+++ b/incoming.c
@@ -577,7 +577,7 @@
ic->closed_baton = closed_baton;
/* Store the connection specific info in the configuration store */
- status = serf__config_store_create_client_config(ic, &config, client_pool);
+ status = serf__config_store_create_client_config(ic, &config);
if (status)
return status;
@@ -689,7 +689,7 @@
}
/* Store the connection specific info in the configuration store */
- rv = serf__config_store_create_listener_config(l, &config, l->pool);
+ rv = serf__config_store_create_listener_config(l, &config);
if (rv)
return rv;
diff --git a/outgoing.c b/outgoing.c
index 3e2ab84..371f1d0 100644
--- a/outgoing.c
+++ b/outgoing.c
@@ -1319,7 +1319,7 @@
}
/* Store the connection specific info in the configuration store */
- status = serf__config_store_create_conn_config(c, &config, pool);
+ status = serf__config_store_create_conn_config(c, &config);
if (status)
return status;
c->config = config;
diff --git a/pump.c b/pump.c
index 300968d..aa08bd2 100644
--- a/pump.c
+++ b/pump.c
@@ -192,13 +192,13 @@
char buf[48];
if (!apr_sockaddr_ip_getbuf(buf, sizeof(buf), sa))
serf_config_set_stringf(pump->config, SERF_CONFIG_CONN_LOCALIP,
- "%s:%d", buf, (int)sa->port);
+ pump->pool, "%s:%d", buf, (int)sa->port);
}
if (apr_socket_addr_get(&sa, APR_REMOTE, pump->skt) == APR_SUCCESS) {
char buf[48];
if (!apr_sockaddr_ip_getbuf(buf, sizeof(buf), sa))
serf_config_set_stringf(pump->config, SERF_CONFIG_CONN_REMOTEIP,
- "%s:%d", buf, (int)sa->port);
+ pump->pool, "%s:%d", buf, (int)sa->port);
}
}
diff --git a/serf.h b/serf.h
index e58f538..362eb12 100644
--- a/serf.h
+++ b/serf.h
@@ -1351,6 +1351,7 @@
*/
apr_status_t serf_config_set_stringf(serf_config_t *config,
serf_config_key_t key,
+ apr_pool_t *scratch_pool,
const char *fmt, ...);
/* Set a value of generic type for configuration item CATEGORY+KEY.
diff --git a/serf_private.h b/serf_private.h
index 43acc0c..5e179c5 100644
--- a/serf_private.h
+++ b/serf_private.h
@@ -290,11 +290,8 @@
typedef struct serf__config_hdr_t serf__config_hdr_t;
struct serf_config_t {
- /* Pool for per-connection configuration values */
- apr_pool_t *conn_pool;
- /* Pool for per-host and per-context configuration values */
apr_pool_t *ctx_pool;
-
+ serf_bucket_alloc_t *allocator;
/* Configuration key/value pairs per context */
serf__config_hdr_t *per_context;
@@ -306,19 +303,20 @@
typedef struct serf__config_store_t {
apr_pool_t *pool;
+ serf_bucket_alloc_t *allocator;
/* Configuration key/value pairs per context */
serf__config_hdr_t *global_per_context;
- /* Configuration per host, dual-layered:
+ /* Configuration per host:
Key: hostname:port
- Value: hash table of per host key/value pairs
+ Value: serf__config_hdr_t *
*/
apr_hash_t *global_per_host;
- /* Configuration per connection, dual-layered:
- Key: string(connection ptr) (?)
- Value: hash table of per host key/value pairs
+ /* Configuration per connection:
+ Key: string(connection ptr as string)
+ Value: serf__config_hdr_t *
*/
apr_hash_t *global_per_conn;
@@ -340,23 +338,19 @@
lifecycle cannot extend beyond that of the serf context!
*/
apr_status_t serf__config_store_create_conn_config(serf_connection_t *conn,
- serf_config_t **config,
- apr_pool_t *out_pool);
+ serf_config_t **config);
/* Same thing, but for incoming connections */
apr_status_t serf__config_store_create_client_config(serf_incoming_t *client,
- serf_config_t **config,
- apr_pool_t *out_pool);
+ serf_config_t **config);
/* Same thing, but for listeners */
apr_status_t serf__config_store_create_listener_config(serf_listener_t *listener,
- serf_config_t **config,
- apr_pool_t *out_pool);
+ serf_config_t **config);
/* Same thing, but for the context itself */
apr_status_t serf__config_store_create_ctx_config(serf_context_t *ctx,
- serf_config_t **config,
- apr_pool_t *out_pool);
+ serf_config_t **config);
/* Cleans up all connection specific configuration values */
