Bojan Smojver
Wed, 23 Jul 2008 00:58:54 -0700
On Wed, 2008-07-23 at 16:21 +1000, Bojan Smojver wrote: > As Joe predicted, it even works when pool pointer is not reset. Just some minor cleanups. -- Bojan
Index: misc/apr_reslist.c
===================================================================
--- misc/apr_reslist.c (revision 679009)
+++ misc/apr_reslist.c (working copy)
@@ -14,8 +14,6 @@
* limitations under the License.
*/
-#include <assert.h>
-
#include "apu.h"
#include "apr_reslist.h"
#include "apr_errno.h"
@@ -23,6 +21,7 @@
#include "apr_thread_mutex.h"
#include "apr_thread_cond.h"
#include "apr_ring.h"
+#include "apr_hash.h"
#if APR_HAS_THREADS
@@ -33,6 +32,8 @@
apr_time_t freed;
void *opaque;
APR_RING_ENTRY(apr_res_t) link;
+ apr_pool_t *pool; /* resource specific pool, passed to con/de-structor */
+ apr_reslist_t *list;
};
typedef struct apr_res_t apr_res_t;
@@ -43,7 +44,7 @@
typedef struct apr_resring_t apr_resring_t;
struct apr_reslist_t {
- apr_pool_t *pool; /* the pool used in constructor and destructor calls */
+ apr_pool_t *pool; /* the pool used for the list */
int ntotal; /* total number of resources managed by this list */
int nidle; /* number of available resources */
int min; /* desired minimum number of available resources */
@@ -55,7 +56,7 @@
apr_reslist_destructor destructor;
void *params; /* opaque data passed to constructor and destructor calls */
apr_resring_t avail_list;
- apr_resring_t free_list;
+ apr_hash_t *in_use;
apr_thread_mutex_t *listlock;
apr_thread_cond_t *avail;
};
@@ -69,6 +70,7 @@
apr_res_t *res;
res = APR_RING_FIRST(&reslist->avail_list);
APR_RING_REMOVE(res, link);
+ apr_hash_set(reslist->in_use, &res->opaque, sizeof(res->opaque), res);
reslist->nidle--;
return res;
}
@@ -78,38 +80,21 @@
* it was added to the list.
* Assumes: that the reslist is locked.
*/
-static void push_resource(apr_reslist_t *reslist, apr_res_t *resource)
+static void push_resource(apr_res_t *res)
{
- APR_RING_INSERT_HEAD(&reslist->avail_list, resource, apr_res_t, link);
- resource->freed = apr_time_now();
- reslist->nidle++;
+ APR_RING_INSERT_HEAD(&res->list->avail_list, res, apr_res_t, link);
+ apr_hash_set(res->list->in_use, &res->opaque, sizeof(res->opaque), NULL);
+ res->freed = apr_time_now();
+ res->list->nidle++;
}
-/**
- * Get an resource container from the free list or create a new one.
- */
-static apr_res_t *get_container(apr_reslist_t *reslist)
+static apr_status_t res_clean(void *data)
{
- apr_res_t *res;
-
- if (!APR_RING_EMPTY(&reslist->free_list, apr_res_t, link)) {
- res = APR_RING_FIRST(&reslist->free_list);
- APR_RING_REMOVE(res, link);
- }
- else
- res = apr_pcalloc(reslist->pool, sizeof(*res));
- return res;
+ apr_res_t *res = data;
+ return res->list->destructor(res->opaque, res->list->params, res->pool);
}
/**
- * Free up a resource container by placing it on the free list.
- */
-static void free_container(apr_reslist_t *reslist, apr_res_t *container)
-{
- APR_RING_INSERT_TAIL(&reslist->free_list, container, apr_res_t, link);
-}
-
-/**
* Create a new resource and return it.
* Assumes: that the reslist is locked.
*/
@@ -117,11 +102,25 @@
{
apr_status_t rv;
apr_res_t *res;
+ apr_pool_t *p;
- res = get_container(reslist);
+ if ((rv = apr_pool_create(&p, reslist->pool)) != APR_SUCCESS) {
+ return rv;
+ }
- rv = reslist->constructor(&res->opaque, reslist->params, reslist->pool);
+ res = apr_pcalloc(p, sizeof(*res));
+ rv = reslist->constructor(&res->opaque, reslist->params, p);
+
+ if (rv != APR_SUCCESS) {
+ apr_pool_destroy(p);
+ return rv;
+ }
+
+ res->pool = p;
+ res->list = reslist;
+ apr_pool_cleanup_register(p, res, res_clean, apr_pool_cleanup_null);
+
*ret_res = res;
return rv;
}
@@ -132,39 +131,10 @@
*/
static apr_status_t destroy_resource(apr_reslist_t *reslist, apr_res_t *res)
{
- return reslist->destructor(res->opaque, reslist->params, reslist->pool);
+ apr_pool_destroy(res->pool);
+ return APR_SUCCESS;
}
-static apr_status_t reslist_cleanup(void *data_)
-{
- apr_status_t rv = APR_SUCCESS;
- apr_reslist_t *rl = data_;
- apr_res_t *res;
-
- apr_thread_mutex_lock(rl->listlock);
-
- while (rl->nidle > 0) {
- apr_status_t rv1;
- res = pop_resource(rl);
- rl->ntotal--;
- rv1 = destroy_resource(rl, res);
- if (rv1 != APR_SUCCESS) {
- rv = rv1; /* loses info in the unlikely event of
- * multiple *different* failures */
- }
- free_container(rl, res);
- }
-
- assert(rl->nidle == 0);
- assert(rl->ntotal == 0);
-
- apr_thread_mutex_unlock(rl->listlock);
- apr_thread_mutex_destroy(rl->listlock);
- apr_thread_cond_destroy(rl->avail);
-
- return rv;
-}
-
/**
* Perform routine maintenance on the resource list. This call
* may instantiate new resources or expire old resources.
@@ -183,12 +153,11 @@
/* Create the resource */
rv = create_resource(reslist, &res);
if (rv != APR_SUCCESS) {
- free_container(reslist, res);
apr_thread_mutex_unlock(reslist->listlock);
return rv;
}
/* Add it to the list */
- push_resource(reslist, res);
+ push_resource(res);
/* Update our counters */
reslist->ntotal++;
/* If someone is waiting on that guy, wake them up. */
@@ -221,7 +190,6 @@
reslist->nidle--;
reslist->ntotal--;
rv = destroy_resource(reslist, res);
- free_container(reslist, res);
if (rv != APR_SUCCESS) {
apr_thread_mutex_unlock(reslist->listlock);
return rv;
@@ -242,6 +210,7 @@
{
apr_status_t rv;
apr_reslist_t *rl;
+ apr_pool_t *np;
/* Do some sanity checks so we don't thrash around in the
* maintenance routine later. */
@@ -249,8 +218,12 @@
return APR_EINVAL;
}
- rl = apr_pcalloc(pool, sizeof(*rl));
- rl->pool = pool;
+ if ((rv = apr_pool_create (&np, pool)) != APR_SUCCESS) {
+ return rv;
+ }
+
+ rl = apr_pcalloc(np, sizeof(*rl));
+ rl->pool = np;
rl->min = min;
rl->smax = smax;
rl->hmax = hmax;
@@ -260,14 +233,14 @@
rl->params = params;
APR_RING_INIT(&rl->avail_list, apr_res_t, link);
- APR_RING_INIT(&rl->free_list, apr_res_t, link);
- rv = apr_thread_mutex_create(&rl->listlock, APR_THREAD_MUTEX_DEFAULT,
- pool);
+ rl->in_use = apr_hash_make(np);
+
+ rv = apr_thread_mutex_create(&rl->listlock, APR_THREAD_MUTEX_DEFAULT, np);
if (rv != APR_SUCCESS) {
return rv;
}
- rv = apr_thread_cond_create(&rl->avail, pool);
+ rv = apr_thread_cond_create(&rl->avail, np);
if (rv != APR_SUCCESS) {
return rv;
}
@@ -277,9 +250,6 @@
return rv;
}
- apr_pool_cleanup_register(rl->pool, rl, reslist_cleanup,
- apr_pool_cleanup_null);
-
*reslist = rl;
return APR_SUCCESS;
@@ -287,7 +257,8 @@
APU_DECLARE(apr_status_t) apr_reslist_destroy(apr_reslist_t *reslist)
{
- return apr_pool_cleanup_run(reslist->pool, reslist, reslist_cleanup);
+ apr_pool_destroy(reslist->pool);
+ return APR_SUCCESS;
}
APU_DECLARE(apr_status_t) apr_reslist_acquire(apr_reslist_t *reslist,
@@ -308,7 +279,6 @@
/* this res is expired - kill it */
reslist->ntotal--;
rv = destroy_resource(reslist, res);
- free_container(reslist, res);
if (rv != APR_SUCCESS) {
apr_thread_mutex_unlock(reslist->listlock);
return rv; /* FIXME: this might cause unnecessary fails */
@@ -316,7 +286,6 @@
continue;
}
*resource = res->opaque;
- free_container(reslist, res);
apr_thread_mutex_unlock(reslist->listlock);
return APR_SUCCESS;
}
@@ -339,7 +308,6 @@
if (reslist->nidle > 0) {
res = pop_resource(reslist);
*resource = res->opaque;
- free_container(reslist, res);
apr_thread_mutex_unlock(reslist->listlock);
return APR_SUCCESS;
}
@@ -351,8 +319,9 @@
if (rv == APR_SUCCESS) {
reslist->ntotal++;
*resource = res->opaque;
+ apr_hash_set(reslist->in_use,
+ &res->opaque, sizeof(res->opaque), res);
}
- free_container(reslist, res);
apr_thread_mutex_unlock(reslist->listlock);
return rv;
}
@@ -364,9 +333,8 @@
apr_res_t *res;
apr_thread_mutex_lock(reslist->listlock);
- res = get_container(reslist);
- res->opaque = resource;
- push_resource(reslist, res);
+ res = apr_hash_get(reslist->in_use, &resource, sizeof(resource));
+ push_resource(res);
apr_thread_cond_signal(reslist->avail);
apr_thread_mutex_unlock(reslist->listlock);