apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r659802 - in /apr/apr-util/trunk: CHANGES misc/apr_reslist.c
Date Sat, 24 May 2008 13:04:01 GMT


On 05/24/2008 02:26 PM, niq@apache.org wrote:
> Author: niq
> Date: Sat May 24 05:26:43 2008
> New Revision: 659802
> 
> URL: http://svn.apache.org/viewvc?rev=659802&view=rev
> Log:
> Enforce ttl (where used) on individual resources in an apr_reslist
> PR 42841
> 
> Modified:
>     apr/apr-util/trunk/CHANGES
>     apr/apr-util/trunk/misc/apr_reslist.c
> 
> Modified: apr/apr-util/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/CHANGES?rev=659802&r1=659801&r2=659802&view=diff
> ==============================================================================
> --- apr/apr-util/trunk/CHANGES [utf-8] (original)
> +++ apr/apr-util/trunk/CHANGES [utf-8] Sat May 24 05:26:43 2008
> @@ -1,6 +1,9 @@
>                                                       -*- coding: utf-8 -*-
>  Changes with APR-util 1.4.0
>  
> +  *) Amend apr_reslist to expire resources whose idle time exceeds ttl.
> +     PR 42841 [Tom Donovan, Nick Kew]
> +
>    *) Add DTrace Probes to Hooks, making it easier to inspect APR Hook based
>       applications with DTrace. [Theo Schlossnagle <jesus omniti.com>]
>  
> 
> Modified: apr/apr-util/trunk/misc/apr_reslist.c
> URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/misc/apr_reslist.c?rev=659802&r1=659801&r2=659802&view=diff
> ==============================================================================
> --- apr/apr-util/trunk/misc/apr_reslist.c (original)
> +++ apr/apr-util/trunk/misc/apr_reslist.c Sat May 24 05:26:43 2008
> @@ -292,13 +292,24 @@
>  {
>      apr_status_t rv;
>      apr_res_t *res;
> +    apr_time_t now;
>  
>      apr_thread_mutex_lock(reslist->listlock);
>      /* If there are idle resources on the available list, use
>       * them right away. */
> -    if (reslist->nidle > 0) {
> +    now = apr_time_now();
> +    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 */

Sorry for pointing out this that late. I just thought about it again and I guess we
have a leak here. I think the following two lines are missing here (before and after
destroy_resource):

         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 */
> +            }
> +            continue;
> +        }
>          *resource = res->opaque;
>          free_container(reslist, res);
>          apr_thread_mutex_unlock(reslist->listlock);


Regards

RĂ¼diger

Mime
View raw message