On 08/06/2008 11:25 AM, mturk@apache.org wrote:
> Author: mturk
> Date: Wed Aug 6 02:25:52 2008
> New Revision: 683191
>
> URL: http://svn.apache.org/viewvc?rev=683191&view=rev
> Log:
> Do not serialize destructor calls.
>
> Modified:
> apr/apr-util/trunk/misc/apr_reslist.c
>
> Modified: apr/apr-util/trunk/misc/apr_reslist.c
> URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/misc/apr_reslist.c?rev=683191&r1=683190&r2=683191&view=diff
> ==============================================================================
> --- apr/apr-util/trunk/misc/apr_reslist.c (original)
> +++ apr/apr-util/trunk/misc/apr_reslist.c Wed Aug 6 02:25:52 2008
> @@ -452,10 +452,11 @@
> void *resource)
> {
> apr_status_t ret;
> +
> + ret = reslist->destructor(resource, reslist->params, reslist->pool);
> #if APR_HAS_THREADS
> apr_thread_mutex_lock(reslist->listlock);
> #endif
> - ret = reslist->destructor(resource, reslist->params, reslist->pool);
> reslist->ntotal--;
> #if APR_HAS_THREADS
> apr_thread_cond_signal(reslist->avail);
This looks dangerous to me. The destructor gets the reslist pool handed over
and pools are not thread save. See also:
/**
* Destroy a single idle resource.
* Assumes: that the reslist is locked.
*/
static apr_status_t destroy_resource(apr_reslist_t *reslist, apr_res_t *res)
{
return reslist->destructor(res->opaque, reslist->params, reslist->pool);
}
Regards
Rüdiger
|