apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: apr_reslist semantics
Date Fri, 23 May 2008 19:28:21 GMT
On Mon, 12 May 2008 20:22:09 +0100
Nick Kew <nick@webthing.com> 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.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Mime
View raw message