From dev-return-20868-apmail-apr-dev-archive=apr.apache.org@apr.apache.org Thu Jul 24 01:04:52 2008 Return-Path: Delivered-To: apmail-apr-dev-archive@www.apache.org Received: (qmail 85219 invoked from network); 24 Jul 2008 01:04:52 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 24 Jul 2008 01:04:52 -0000 Received: (qmail 38665 invoked by uid 500); 24 Jul 2008 01:04:51 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 38629 invoked by uid 500); 24 Jul 2008 01:04:51 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Id: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 38618 invoked by uid 99); 24 Jul 2008 01:04:51 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 Jul 2008 18:04:51 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of bojan@rexursive.com designates 203.171.74.242 as permitted sender) Received: from [203.171.74.242] (HELO beauty.rexursive.com) (203.171.74.242) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 24 Jul 2008 01:03:55 +0000 Received: from [10.1.120.24] (shrek.rexursive.com [10.1.120.24]) by beauty.rexursive.com (Postfix) with ESMTP id 6D4C140330 for ; Thu, 24 Jul 2008 11:04:19 +1000 (EST) Subject: Re: Changing the order of cleanup for some core objects From: Bojan Smojver To: dev@apr.apache.org In-Reply-To: <20080723120039.GA8981@redhat.com> References: <48843999.1010804@apache.org> <20080721093425.GA13566@redhat.com> <4884671A.8070401@apache.org> <20080721110144.GC13566@redhat.com> <4884C350.3000907@pearsoncmg.com> <20080722081559.GA4724@redhat.com> <1216793532.2754.192.camel@shrek.rexursive.com> <1216794074.2754.193.camel@shrek.rexursive.com> <4886D0E9.2020803@apache.org> <20080723120039.GA8981@redhat.com> Content-Type: multipart/mixed; boundary="=-sfEUa31bktRgM31cHVBI" Date: Thu, 24 Jul 2008 11:04:19 +1000 Message-Id: <1216861459.2754.300.camel@shrek.rexursive.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 (2.22.3.1-1.fc9) X-Virus-Checked: Checked by ClamAV on apache.org --=-sfEUa31bktRgM31cHVBI Content-Type: text/plain Content-Transfer-Encoding: 7bit On Wed, 2008-07-23 at 13:00 +0100, Joe Orton wrote: > But that does make the destructor a bit redundant Just a quick an dirty. -- Bojan --=-sfEUa31bktRgM31cHVBI Content-Disposition: attachment; filename=apu-respool.patch Content-Type: text/x-patch; name=apu-respool.patch; charset=utf-8 Content-Transfer-Encoding: 7bit Index: libaprutil.dsp =================================================================== --- libaprutil.dsp (revision 679250) +++ libaprutil.dsp (working copy) @@ -384,6 +384,10 @@ # End Source File # Begin Source File +SOURCE=.\misc\apr_respool.c +# End Source File +# Begin Source File + SOURCE=.\misc\apr_rmm.c # End Source File # Begin Source File Index: aprutil.dsp =================================================================== --- aprutil.dsp (revision 679250) +++ aprutil.dsp (working copy) @@ -339,6 +339,10 @@ # End Source File # Begin Source File +SOURCE=.\misc\apr_respool.c +# End Source File +# Begin Source File + SOURCE=.\misc\apr_rmm.c # End Source File # Begin Source File Index: test/Makefile.in =================================================================== --- test/Makefile.in (revision 679250) +++ test/Makefile.in (working copy) @@ -16,8 +16,8 @@ TESTS = teststrmatch.lo testuri.lo testuuid.lo testbuckets.lo testpass.lo \ testmd4.lo testmd5.lo testldap.lo testdate.lo testdbm.lo testdbd.lo \ - testxml.lo testrmm.lo testreslist.lo testqueue.lo testxlate.lo \ - testmemcache.lo + testxml.lo testrmm.lo testreslist.lo testrespool.lo testqueue.lo \ + testxlate.lo testmemcache.lo PROGRAMS = $(STDTEST_PORTABLE) Index: test/testutillib.dsp =================================================================== --- test/testutillib.dsp (revision 679250) +++ test/testutillib.dsp (working copy) @@ -227,6 +227,10 @@ # End Source File # Begin Source File +SOURCE=.\testrespool.c +# End Source File +# Begin Source File + SOURCE=.\testrmm.c # End Source File # Begin Source File Index: test/testutildll.dsp =================================================================== --- test/testutildll.dsp (revision 679250) +++ test/testutildll.dsp (working copy) @@ -227,6 +227,10 @@ # End Source File # Begin Source File +SOURCE=.\testrespool.c +# End Source File +# Begin Source File + SOURCE=.\testrmm.c # End Source File # Begin Source File Index: test/abts_tests.h =================================================================== --- test/abts_tests.h (revision 679250) +++ test/abts_tests.h (working copy) @@ -39,7 +39,8 @@ {testrmm}, {testdbm}, {testqueue}, - {testreslist} + {testreslist}, + {testrespool} }; #endif /* APR_TEST_INCLUDES */ Index: test/testutil.h =================================================================== --- test/testutil.h (revision 679250) +++ test/testutil.h (working copy) @@ -55,6 +55,7 @@ abts_suite *testdate(abts_suite *suite); abts_suite *testmemcache(abts_suite *suite); abts_suite *testreslist(abts_suite *suite); +abts_suite *testrespool(abts_suite *suite); abts_suite *testqueue(abts_suite *suite); abts_suite *testxml(abts_suite *suite); abts_suite *testxlate(abts_suite *suite); Index: test/Makefile.win =================================================================== --- test/Makefile.win (revision 679250) +++ test/Makefile.win (working copy) @@ -59,9 +59,10 @@ $(INTDIR)\testmd4.obj $(INTDIR)\testmd5.obj \ $(INTDIR)\testldap.obj $(INTDIR)\testdbd.obj \ $(INTDIR)\testdbm.obj $(INTDIR)\testreslist.obj \ - $(INTDIR)\testxml.obj $(INTDIR)\testqueue.obj \ - $(INTDIR)\testrmm.obj $(INTDIR)\testxlate.obj \ - $(INTDIR)\testdate.obj $(INTDIR)\testmemcache.obj + $(INTDIR)\testrespool.obj $(INTDIR)\testxml.obj \ + $(INTDIR)\testqueue.obj $(INTDIR)\testrmm.obj \ + $(INTDIR)\testxlate.obj $(INTDIR)\testdate.obj \ + $(INTDIR)\testmemcache.obj CLEAN_DATA = manyfile.bin testfile.txt data\sqlite*.db Index: include/apr_respool.h =================================================================== --- include/apr_respool.h (revision 0) +++ include/apr_respool.h (revision 0) @@ -0,0 +1,134 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef APR_RESPOOL_H +#define APR_RESPOOL_H + +/** + * @file apr_respool.h + * @brief APR-UTIL Resource Pool Routines + */ + +#include "apr.h" +#include "apu.h" +#include "apr_pools.h" +#include "apr_errno.h" +#include "apr_time.h" + +#if APR_HAS_THREADS + +/** + * @defgroup APR_Util_RL Resource Pool Routines + * @ingroup APR_Util + * @{ + */ + +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + +/** Opaque resource pool object */ +typedef struct apr_respool_t apr_respool_t; + +/* Generic constructor called by resource pool when it needs to create a + * resource. + * @param resource opaque resource + * @param param flags + * @param pool Pool + * @remark Note that there is not destructor. Register pool cleanups with the + * pool passed into the constructor instead. + */ +typedef apr_status_t (*apr_respool_constructor)(void **resource, void *params, + apr_pool_t *pool); + +/** + * Create a new resource pool with the following parameters: + * @param respool An address where the pointer to the new resource + * pool will be stored. + * @param min Allowed minimum number of available resources. Zero + * creates new resources only when needed. + * @param smax Resources will be destroyed to meet this maximum + * restriction as they expire. + * @param hmax Absolute maximum limit on the number of total resources. + * @param ttl If non-zero, sets the maximum amount of time a resource + * may be available while exceeding the soft limit. + * @param con Constructor routine that is called to create a new resource. + * @param params Passed to constructor and deconstructor + * @param pool The pool from which to create a new pool for this resource + * pool. Each resource will get a sub-pool of this newly created + * pool passed into the constructor. + */ +APU_DECLARE(apr_status_t) apr_respool_create(apr_respool_t **respool, + int min, int smax, int hmax, + apr_interval_time_t ttl, + apr_respool_constructor con, + void *params, + apr_pool_t *pool); + +/** + * Destroy the given resource pool and all resources controlled by + * this pool. + * @param respool The respool to destroy + */ +APU_DECLARE(apr_status_t) apr_respool_destroy(apr_respool_t *respool); + +/** + * Retrieve a resource from the pool, creating a new one if necessary. + * If we have met our maximum number of resources, we will block + * until one becomes available. + */ +APU_DECLARE(apr_status_t) apr_respool_acquire(apr_respool_t *respool, + void **resource); + +/** + * Return a resource back to the pool of available resources. + */ +APU_DECLARE(apr_status_t) apr_respool_release(apr_respool_t *respool, + void *resource); + +/** + * Set the timeout the acquire will wait for a free resource + * when the maximum number of resources is exceeded. + * @param respool The resource pool. + * @param timeout Timeout to wait. The zero waits forewer. + */ +APU_DECLARE(void) apr_respool_timeout_set(apr_respool_t *respool, + apr_interval_time_t timeout); + +/** + * Return the number of outstanding resources. + * @param respool The resource pool. + */ +APU_DECLARE(apr_uint32_t) apr_respool_acquired_count(apr_respool_t *respool); + +/** + * Invalidate a resource in the pool - e.g. a database connection + * that returns a "lost connection" error and can't be restored. + * Use this instead of apr_respool_release if the resource is bad. + */ +APU_DECLARE(apr_status_t) apr_respool_invalidate(apr_respool_t *respool, + void *resource); + + +#ifdef __cplusplus +} +#endif + +/** @} */ + +#endif /* APR_HAS_THREADS */ + +#endif /* ! APR_RESPOOL_H */ Index: misc/apr_respool.c =================================================================== --- misc/apr_respool.c (revision 0) +++ misc/apr_respool.c (revision 0) @@ -0,0 +1,365 @@ +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "apu.h" +#include "apr_respool.h" +#include "apr_errno.h" +#include "apr_strings.h" +#include "apr_thread_mutex.h" +#include "apr_thread_cond.h" +#include "apr_ring.h" +#include "apr_hash.h" + +#if APR_HAS_THREADS + +/** + * A single resource element. + */ +struct apr_res_t { + apr_time_t freed; + void *opaque; + APR_RING_ENTRY(apr_res_t) link; + apr_pool_t *pool; /* resource specific pool, passed to constructor */ +}; +typedef struct apr_res_t apr_res_t; + +/** + * A ring of resources representing the pool of available resources. + */ +APR_RING_HEAD(apr_resring_t, apr_res_t); +typedef struct apr_resring_t apr_resring_t; + +struct apr_respool_t { + apr_pool_t *pool; /* the pool used for the resource pool */ + int ntotal; /* total number of resources managed by this pool */ + int nidle; /* number of available resources */ + int min; /* desired minimum number of available resources */ + int smax; /* soft maximum on the total number of resources */ + int hmax; /* hard maximum on the total number of resources */ + apr_interval_time_t ttl; /* TTL when we have too many resources */ + apr_interval_time_t timeout; /* Timeout for waiting on resource */ + apr_respool_constructor constructor; + void *params; /* opaque data passed to constructor */ + apr_resring_t avail; + apr_hash_t *in_use; + apr_thread_mutex_t *lock; + apr_thread_cond_t *cond; +}; + +/** + * Grab a resource from the front of the resource pool. + * Assumes: that the respool is locked. + */ +static apr_res_t *pop_resource(apr_respool_t *respool) +{ + apr_res_t *res; + res = APR_RING_FIRST(&respool->avail); + APR_RING_REMOVE(res, link); + apr_hash_set(respool->in_use, &res->opaque, sizeof(res->opaque), res); + respool->nidle--; + return res; +} + +/** + * Add a resource to the beginning of the pool, set the time at which + * it was added to the pool. + * Assumes: that the respool is locked. + */ +static void push_resource(apr_respool_t *respool, apr_res_t *res) +{ + APR_RING_INSERT_HEAD(&respool->avail, res, apr_res_t, link); + apr_hash_set(respool->in_use, &res->opaque, sizeof(res->opaque), NULL); + res->freed = apr_time_now(); + respool->nidle++; +} + +/** + * Create a new resource and return it. + * Assumes: that the respool is locked. + */ +static apr_status_t create_resource(apr_respool_t *respool, apr_res_t **ret_res) +{ + apr_status_t rv; + apr_res_t *res; + apr_pool_t *p; + + if ((rv = apr_pool_create(&p, respool->pool)) != APR_SUCCESS) { + return rv; + } + + res = apr_pcalloc(p, sizeof(*res)); + + rv = respool->constructor(&res->opaque, respool->params, p); + + if (rv != APR_SUCCESS) { + apr_pool_destroy(p); + return rv; + } + + res->pool = p; + + *ret_res = res; + return rv; +} + +/** + * Destroy a single idle resource. + * Assumes: that the respool is locked. + */ +static apr_status_t destroy_resource(apr_respool_t *respool, apr_res_t *res) +{ + apr_pool_destroy(res->pool); + return APR_SUCCESS; +} + +/** + * Perform routine maintenance on the resource pool. This call + * may instantiate new resources or expire old resources. + */ +static apr_status_t respool_maint(apr_respool_t *respool) +{ + apr_time_t now; + apr_status_t rv; + apr_res_t *res; + int created_one = 0; + + apr_thread_mutex_lock(respool->lock); + + /* Check if we need to create more resources, and if we are allowed to. */ + while (respool->nidle < respool->min && respool->ntotal < respool->hmax) { + /* Create the resource */ + rv = create_resource(respool, &res); + if (rv != APR_SUCCESS) { + apr_thread_mutex_unlock(respool->lock); + return rv; + } + /* Add it to the pool */ + push_resource(respool, res); + /* Update our counters */ + respool->ntotal++; + /* If someone is waiting on that guy, wake them up. */ + rv = apr_thread_cond_signal(respool->cond); + if (rv != APR_SUCCESS) { + apr_thread_mutex_unlock(respool->lock); + return rv; + } + created_one++; + } + + /* We don't need to see if we're over the max if we were under it before */ + if (created_one) { + apr_thread_mutex_unlock(respool->lock); + return APR_SUCCESS; + } + + /* Check if we need to expire old resources */ + now = apr_time_now(); + while (respool->nidle > respool->smax && respool->nidle > 0) { + /* Peak at the last resource in the pool */ + res = APR_RING_LAST(&respool->avail); + /* See if the oldest entry should be expired */ + if (now - res->freed < respool->ttl) { + /* If this entry is too young, none of the others + * will be ready to be expired either, so we are done. */ + break; + } + APR_RING_REMOVE(res, link); + respool->nidle--; + respool->ntotal--; + rv = destroy_resource(respool, res); + if (rv != APR_SUCCESS) { + apr_thread_mutex_unlock(respool->lock); + return rv; + } + } + + apr_thread_mutex_unlock(respool->lock); + return APR_SUCCESS; +} + +APU_DECLARE(apr_status_t) apr_respool_create(apr_respool_t **respool, + int min, int smax, int hmax, + apr_interval_time_t ttl, + apr_respool_constructor con, + void *params, + apr_pool_t *pool) +{ + apr_status_t rv; + apr_respool_t *rp; + apr_pool_t *np; + + /* Do some sanity checks so we don't thrash around in the + * maintenance routine later. */ + if (min > smax || min > hmax || smax > hmax || ttl < 0) { + return APR_EINVAL; + } + + if ((rv = apr_pool_create (&np, pool)) != APR_SUCCESS) { + return rv; + } + + rp = apr_pcalloc(np, sizeof(*rp)); + rp->pool = np; + rp->min = min; + rp->smax = smax; + rp->hmax = hmax; + rp->ttl = ttl; + rp->constructor = con; + rp->params = params; + + APR_RING_INIT(&rp->avail, apr_res_t, link); + + rp->in_use = apr_hash_make(np); + + rv = apr_thread_mutex_create(&rp->lock, APR_THREAD_MUTEX_DEFAULT, np); + if (rv != APR_SUCCESS) { + return rv; + } + rv = apr_thread_cond_create(&rp->cond, np); + if (rv != APR_SUCCESS) { + return rv; + } + + rv = respool_maint(rp); + if (rv != APR_SUCCESS) { + return rv; + } + + *respool = rp; + + return APR_SUCCESS; +} + +APU_DECLARE(apr_status_t) apr_respool_destroy(apr_respool_t *respool) +{ + apr_pool_destroy(respool->pool); + return APR_SUCCESS; +} + +APU_DECLARE(apr_status_t) apr_respool_acquire(apr_respool_t *respool, + void **resource) +{ + apr_status_t rv; + apr_res_t *res; + apr_time_t now; + + apr_thread_mutex_lock(respool->lock); + /* If there are idle resources on the available pool, use + * them right away. */ + now = apr_time_now(); + while (respool->nidle > 0) { + /* Pop off the first resource */ + res = pop_resource(respool); + if (respool->ttl && (now - res->freed >= respool->ttl)) { + /* this res is expired - kill it */ + respool->ntotal--; + rv = destroy_resource(respool, res); + if (rv != APR_SUCCESS) { + apr_thread_mutex_unlock(respool->lock); + return rv; /* FIXME: this might cause unnecessary fails */ + } + continue; + } + *resource = res->opaque; + apr_thread_mutex_unlock(respool->lock); + return APR_SUCCESS; + } + /* If we've hit our max, block until we're allowed to create + * a new one, or something becomes free. */ + while (respool->ntotal >= respool->hmax && respool->nidle <= 0) { + if (respool->timeout) { + if ((rv = apr_thread_cond_timedwait(respool->cond, + respool->lock, respool->timeout)) != APR_SUCCESS) { + apr_thread_mutex_unlock(respool->lock); + return rv; + } + } + else { + apr_thread_cond_wait(respool->cond, respool->lock); + } + } + /* If we popped out of the loop, first try to see if there + * are new resources available for immediate use. */ + if (respool->nidle > 0) { + res = pop_resource(respool); + *resource = res->opaque; + apr_thread_mutex_unlock(respool->lock); + return APR_SUCCESS; + } + /* Otherwise the reason we dropped out of the loop + * was because there is a new slot available, so create + * a resource to fill the slot and use it. */ + else { + rv = create_resource(respool, &res); + if (rv == APR_SUCCESS) { + respool->ntotal++; + *resource = res->opaque; + apr_hash_set(respool->in_use, + &res->opaque, sizeof(res->opaque), res); + } + apr_thread_mutex_unlock(respool->lock); + return rv; + } +} + +APU_DECLARE(apr_status_t) apr_respool_release(apr_respool_t *respool, + void *resource) +{ + apr_res_t *res; + + apr_thread_mutex_lock(respool->lock); + res = apr_hash_get(respool->in_use, &resource, sizeof(resource)); + push_resource(respool, res); + apr_thread_cond_signal(respool->cond); + apr_thread_mutex_unlock(respool->lock); + + return respool_maint(respool); +} + +APU_DECLARE(void) apr_respool_timeout_set(apr_respool_t *respool, + apr_interval_time_t timeout) +{ + respool->timeout = timeout; +} + +APU_DECLARE(apr_uint32_t) apr_respool_acquired_count(apr_respool_t *respool) +{ + apr_uint32_t count; + + apr_thread_mutex_lock(respool->lock); + count = respool->ntotal - respool->nidle; + apr_thread_mutex_unlock(respool->lock); + + return count; +} + +APU_DECLARE(apr_status_t) apr_respool_invalidate(apr_respool_t *respool, + void *resource) +{ + apr_status_t ret; + apr_res_t *res; + + apr_thread_mutex_lock(respool->lock); + res = apr_hash_get(respool->in_use, &resource, sizeof(resource)); + apr_hash_set(respool->in_use, &res->opaque, sizeof(res->opaque), NULL); + ret = destroy_resource(respool, res); + respool->ntotal--; + apr_thread_cond_signal(respool->cond); + apr_thread_mutex_unlock(respool->lock); + return ret; +} + +#endif /* APR_HAS_THREADS */ Index: NWGNUmakefile =================================================================== --- NWGNUmakefile (revision 679250) +++ NWGNUmakefile (working copy) @@ -255,6 +255,7 @@ $(OBJDIR)/apr_memcache.o \ $(OBJDIR)/apr_queue.o \ $(OBJDIR)/apr_reslist.o \ + $(OBJDIR)/apr_respool.o \ $(OBJDIR)/apr_rmm.o \ $(OBJDIR)/apr_sha1.o \ $(OBJDIR)/apu_version.o \ --=-sfEUa31bktRgM31cHVBI--