On 05/23/2008 09:28 PM, Nick Kew wrote:
On Mon, 12 May 2008 20:22:09 +0100
Nick Kew <[EMAIL PROTECTED]> wrote:
In https://issues.apache.org/bugzilla/show_bug.cgi?id=42841 ,
Tom points out an issue that gives problems with MySQL
(and possibly other DBD drivers) and suggests that a change
to apr_reslist semantics would fix it. Tom also attaches
a patch implementing his proposed change.
[chop]
After far too long, I've revisited this issue and Tom's patch.
It seems to me we can get *both* semantics at once:
* Keep reslist_maint as is, preserving its existing semantics
* Check the idle time of a resource in apr_reslist_acquire,
and kill a resource if it's too old.
There are other considerations, but a minimal effective patch
looks like:
===================================================================
--- apr_reslist.c (revision 659614)
+++ apr_reslist.c (working copy)
@@ -292,13 +292,22 @@
{
apr_status_t rv;
apr_res_t *res;
+ apr_time_t now = apr_time_now();
apr_thread_mutex_lock(reslist->listlock);
/* If there are idle resources on the available list, use
* them right away. */
- if (reslist->nidle > 0) {
+ while (reslist->nidle > 0) {
/* Pop off the first resource */
res = pop_resource(reslist);
+ if (reslist->ttl && (now - res->freed >= reslist->ttl)) {
+ /* this res is expired - kill it */
+ rv = destroy_resource(reslist, res);
+ if (rv != APR_SUCCESS) {
+ apr_thread_mutex_unlock(reslist->listlock);
+ return rv; /* FIXME: this might cause unnecessary
fails */
+ }
+ continue;
+ }
*resource = res->opaque;
free_container(reslist, res);
apr_thread_mutex_unlock(reslist->listlock);
@@ -306,8 +315,7 @@
}
/* If we've hit our max, block until we're allowed to create
* a new one, or something becomes free. */
- else while (reslist->ntotal >= reslist->hmax
- && reslist->nidle <= 0) {
+ while (reslist->ntotal >= reslist->hmax && reslist->nidle <= 0) {
if (reslist->timeout) {
if ((rv = apr_thread_cond_timedwait(reslist->avail,
reslist->listlock, reslist->timeout)) != APR_SUCCESS) {
It's not perfect, but it has the advantage of neither disturbing
the API/ABI, nor breaking promises to existing apps.
From first glance this looks good.
Regards
Rüdiger
